This is a more generalized solution to the problem presented in PR17558. The
uninitialized variable analysis was passing over variables whose address is
taken as it assumes the variable escapes its analysis.
This patch adds support to catch some of the situations where taking the
address of a variables does not actually initialize the variable.
So cases like:
int x; (void)&x; return x;
int x; int* y = &x; return x;
int x; bool b = (&x == 0); return x;
int x; bool b; b = &x; foo(b); return x;
Will give an "uninitialized" warning on the return.
While cases like
int x; int* y = &x; foo(y); return x;
int x; int *y = &x; int *z = y; return x;
Will not show an uninitialized warning.
You can view some of the discussion we have on the cfe-dev mailing list:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-January/034719.html
To copy and paste my technical description of the patch from the email:
More technical details:
1) When I encounter a "Address Of" operator, I mark the child
DeclRefExpr as being in a "Pending" state. If it remains in the
pending state, then we treat it the same way as we would an "Ignore"
and Clang will identify the variable as being uninitialized (provided
nothing else is done with the variable).
2) If the "Address Of" is assigned to a non-pointer variable, then we
mark the child DeclRefExpr as being in the "Pending" state.
3) If the "Address Of" is assigned to a pointer variable, then we mark
the child DeclRefExpr as being in the "WaitForRef" state. In this
case, we will store it for later processing and store which variable
it was assigned to. Along the way, if the pointer is used, we will
make sure to remember that it was used so we can later mark the
"Address Of" variable as being potentially initialized.
4) After the initial visits using ClassifyRefs, I do one last pass
through the stored "Address Of" expressions that were assigned to
pointers and check to see if those pointers were used and change
classifications as necessary.
http://llvm-reviews.chandlerc.com/D2623
Files:
include/clang/AST/Expr.h
lib/Analysis/UninitializedValues.cpp
test/Analysis/uninit-sometimes.cpp
test/Sema/uninit-variables.c
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -1720,6 +1720,11 @@
}
bool isArithmeticOp() const { return isArithmeticOp(getOpcode()); }
+ static bool isAddrOfOp(Opcode Op) {
+ return Op == UO_AddrOf;
+ }
+ bool isAddrOfOp() const { return isAddrOfOp(getOpcode()); }
+
/// getOpcodeStr - Turn an Opcode enum value into the punctuation char it
/// corresponds to, e.g. "sizeof" or "[pre]++"
static StringRef getOpcodeStr(Opcode Op);
Index: lib/Analysis/UninitializedValues.cpp
===================================================================
--- lib/Analysis/UninitializedValues.cpp
+++ lib/Analysis/UninitializedValues.cpp
@@ -304,6 +304,8 @@
class ClassifyRefs : public StmtVisitor<ClassifyRefs> {
public:
enum Class {
+ Pending,
+ WaitForRef,
Init,
Use,
SelfInit,
@@ -314,12 +316,25 @@
const DeclContext *DC;
llvm::DenseMap<const DeclRefExpr*, Class> Classification;
+ /// Keep track of the pointers that AddrOf operators get assigned to.
+ llvm::DenseMap<const DeclRefExpr*, llvm::SmallVector<const ValueDecl*, 3> >
+ AddrOfDependencies;
+
+ /// Keep track of all the AddrOf operators that are used for each pointer.
+ llvm::DenseMap<const ValueDecl*, llvm::SmallVector<const DeclRefExpr*, 3> >
+ PointerVarUseAssociation;
+
+ /// Keep track of the current AddrOf operator that is assigned to a pointer,
+ /// if any.
+ llvm::DenseMap<const ValueDecl*, const DeclRefExpr*> StagePointerAssociation;
+
bool isTrackedVar(const VarDecl *VD) const {
return ::isTrackedVar(VD, DC);
}
void classify(const Expr *E, Class C);
+ void ClassifyChildAddrOf(const Stmt *S, Class C, const ValueDecl* V);
public:
ClassifyRefs(AnalysisDeclContext &AC) : DC(cast<DeclContext>(AC.getDecl())) {}
@@ -330,6 +345,7 @@
void VisitCastExpr(CastExpr *CE);
void operator()(Stmt *S) { Visit(S); }
+ void DeferredPass();
Class get(const DeclRefExpr *DRE) const {
llvm::DenseMap<const DeclRefExpr*, Class>::const_iterator I
@@ -356,6 +372,35 @@
return 0;
}
+void ClassifyRefs::DeferredPass() {
+ // Must do this after the initial visit so that we know how all the pointers
+ // are used.
+ llvm::DenseMap<const DeclRefExpr*, llvm::SmallVector<const ValueDecl*, 3> >
+ ::const_iterator I = AddrOfDependencies.begin();
+ for(; I != AddrOfDependencies.end(); ++I) {
+ const DeclRefExpr *DRE = I->first;
+ llvm::SmallVector<const ValueDecl*,3>::const_iterator SVI =
+ AddrOfDependencies[DRE].begin();
+
+ bool bIsUsed = false;
+ // Check if these pointers get used in places that would be relevant for
+ // the variable whose address was taken. If so, then we can assume that
+ // the AddrOf variable gets initialized.
+ for(; SVI != AddrOfDependencies[DRE].end(); ++SVI) {
+ if (const VarDecl* VD = dyn_cast<VarDecl>(*SVI)) {
+ if (std::find(PointerVarUseAssociation[VD].begin(),
+ PointerVarUseAssociation[VD].end(),
+ DRE) != PointerVarUseAssociation[VD].end()) {
+ bIsUsed = true;
+ break;
+ }
+ }
+ }
+
+ classify(DRE, bIsUsed ? Init : Ignore);
+ }
+}
+
void ClassifyRefs::classify(const Expr *E, Class C) {
// The result of a ?: could also be an lvalue.
E = E->IgnoreParens();
@@ -368,17 +413,49 @@
}
FindVarResult Var = findVar(E, DC);
- if (const DeclRefExpr *DRE = Var.getDeclRefExpr())
+ if (const DeclRefExpr *DRE = Var.getDeclRefExpr()) {
Classification[DRE] = std::max(Classification[DRE], C);
+ if (StagePointerAssociation[DRE->getDecl()] && C > Init) {
+ PointerVarUseAssociation[DRE->getDecl()].push_back(
+ StagePointerAssociation[DRE->getDecl()]);
+ }
+ }
}
void ClassifyRefs::VisitDeclStmt(DeclStmt *DS) {
for (DeclStmt::decl_iterator DI = DS->decl_begin(), DE = DS->decl_end();
DI != DE; ++DI) {
VarDecl *VD = dyn_cast<VarDecl>(*DI);
- if (VD && isTrackedVar(VD))
+ if (VD && isTrackedVar(VD)) {
if (const DeclRefExpr *DRE = getSelfInitExpr(VD))
Classification[DRE] = SelfInit;
+
+ const QualType QT = VD->getType();
+ if (QT->isPointerType()) {
+ ClassifyChildAddrOf(VD->getInit(), WaitForRef, VD);
+ } else
+ ClassifyChildAddrOf(VD->getInit(), Pending, 0);
+ }
+ }
+}
+
+void ClassifyRefs::ClassifyChildAddrOf(const Stmt *S, Class C,
+ const ValueDecl *V) {
+ if (!S)
+ return;
+
+ for (Stmt::const_child_range CR = S->children(); CR; ++CR)
+ ClassifyChildAddrOf(*CR, C, V);
+
+ if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(S)) {
+ if (UO->isAddrOfOp()) {
+ classify(UO->getSubExpr(), C);
+ if (const DeclRefExpr* DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()))
+ if (V) {
+ AddrOfDependencies[DRE].push_back(V);
+ StagePointerAssociation[V] = DRE;
+ }
+ }
}
}
@@ -390,8 +467,17 @@
// use.
if (BO->isCompoundAssignmentOp())
classify(BO->getLHS(), Use);
- else if (BO->getOpcode() == BO_Assign)
+ else if (BO->getOpcode() == BO_Assign) {
classify(BO->getLHS(), Ignore);
+ if (BO->getLHS() &&
+ BO->getLHS()->getType()->isPointerType()) {
+ FindVarResult FV = findVar(BO->getLHS(), DC);
+ if (FV.getDeclRefExpr())
+ ClassifyChildAddrOf(BO->getRHS(), WaitForRef,
+ FV.getDeclRefExpr()->getDecl());
+ } else
+ ClassifyChildAddrOf(BO->getRHS(), Pending, 0);
+ }
}
void ClassifyRefs::VisitUnaryOperator(UnaryOperator *UO) {
@@ -399,16 +485,33 @@
// conversion.
if (UO->isIncrementDecrementOp())
classify(UO->getSubExpr(), Use);
+ else if (UO->isAddrOfOp()) {
+ // AddrOf Operators marked as "pending" so we can determine their
+ // actual use later in context.
+ // If we remain "Pending" then that means we are never assigned to
+ // another variable or used in a function and thus we can safely assume
+ // that this variable wasn't initialized here.
+ // However, if we encounter a case where the result of the AddrOf operator
+ // is assigned to a variable, further work must be done in identifying
+ // whether or not this variable is used/initialized/or if we can't tell.
+ classify(UO->getSubExpr(), Pending);
+ }
}
void ClassifyRefs::VisitCallExpr(CallExpr *CE) {
// If a value is passed by const reference to a function, we should not assume
// that it is initialized by the call, and we conservatively do not assume
// that it is used.
+ // Also, if a value is passed by pointer to a function, we assume that the
+ // function will initialize it. [Only relevant when dealing with AddrOf
+ // operators].
for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end();
- I != E; ++I)
- if ((*I)->getType().isConstQualified() && (*I)->isGLValue())
- classify(*I, Ignore);
+ I != E; ++I) {
+ if ((*I)->getType().isConstQualified() && (*I)->isGLValue())
+ classify(*I, Ignore);
+ else if ((*I)->getType()->isPointerType())
+ ClassifyChildAddrOf(*I, Init, 0);
+ }
}
void ClassifyRefs::VisitCastExpr(CastExpr *CE) {
@@ -669,10 +772,12 @@
void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *dr) {
switch (classification.get(dr)) {
case ClassifyRefs::Ignore:
+ case ClassifyRefs::Pending:
break;
case ClassifyRefs::Use:
reportUse(dr, cast<VarDecl>(dr->getDecl()));
break;
+ case ClassifyRefs::WaitForRef:
case ClassifyRefs::Init:
vals[cast<VarDecl>(dr->getDecl())] = Initialized;
break;
@@ -819,6 +924,7 @@
// Precompute which expressions are uses and which are initializations.
ClassifyRefs classification(ac);
cfg.VisitBlockStmts(classification);
+ classification.DeferredPass();
// Mark all variables uninitialized at the entry.
const CFGBlock &entry = cfg.getEntry();
Index: test/Analysis/uninit-sometimes.cpp
===================================================================
--- test/Analysis/uninit-sometimes.cpp
+++ test/Analysis/uninit-sometimes.cpp
@@ -396,7 +396,7 @@
void PR16054() {
int x; // expected-note {{variable}} expected-warning {{used uninitialized whenever function 'PR16054}}
while (x != 0) { // expected-note {{use}}
- (void)&x;
+ x = 1;
}
}
Index: test/Sema/uninit-variables.c
===================================================================
--- test/Sema/uninit-variables.c
+++ test/Sema/uninit-variables.c
@@ -531,3 +531,48 @@
}
++x; // no-warning
}
+
+void foo(int* y) {
+}
+
+int test57() {
+ int x;
+ int *y = &x;
+ foo(y);
+ return x;
+}
+
+int test58() {
+ int x;
+ int *y = &x;
+ int *z = y;
+ return x;
+}
+
+int test59() {
+ int x; // expected-note {{initialize the variable 'x' to silence this warning}}
+ (void)&x;
+ return x; // expected-warning{{variable 'x' is uninitialized when used here}}
+}
+
+int test60() {
+ int x; // expected-note {{initialize the variable 'x' to silence this warning}}
+ int* y = &x;
+ return x; // expected-warning{{variable 'x' is uninitialized when used here}}
+}
+
+int test61() {
+ int x; // expected-note {{initialize the variable 'x' to silence this warning}}
+
+ int b = (&x == 0);
+ return x; // expected-warning{{variable 'x' is uninitialized when used here}}
+}
+
+void foo2(int b) {
+}
+int test62() {
+ int x; // expected-note {{initialize the variable 'x' to silence this warning}}
+ int b = (int)&x;
+ foo2(b);
+ return x; // expected-warning{{variable 'x' is uninitialized when used here}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
