Teach UninitializedValuesV2 to implicitly reason about C++
references by monitoring whether an access to
a variable is solely to compute it's lvalue or
to do an lvalue-to-rvalue conversion (i.e., a load).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@123777 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Analysis/UninitializedValuesV2.cpp b/lib/Analysis/UninitializedValuesV2.cpp
index c5b093b..6bb8e0d 100644
--- a/lib/Analysis/UninitializedValuesV2.cpp
+++ b/lib/Analysis/UninitializedValuesV2.cpp
@@ -19,6 +19,7 @@
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/Visitors/CFGRecStmtDeclVisitor.h"
#include "clang/Analysis/Analyses/UninitializedValuesV2.h"
+#include "clang/Analysis/Support/SaveAndRestore.h"
using namespace clang;
@@ -218,15 +219,17 @@
CFGBlockValues &vals;
const CFG &cfg;
UninitVariablesHandler *handler;
+ const DeclRefExpr *currentDR;
public:
TransferFunctions(CFGBlockValues &vals, const CFG &cfg,
UninitVariablesHandler *handler)
- : vals(vals), cfg(cfg), handler(handler) {}
+ : vals(vals), cfg(cfg), handler(handler), currentDR(0) {}
const CFG &getCFG() { return cfg; }
void reportUninit(const DeclRefExpr *ex, const VarDecl *vd);
void VisitDeclStmt(DeclStmt *ds);
+ void VisitDeclRefExpr(DeclRefExpr *dr);
void VisitUnaryOperator(UnaryOperator *uo);
void VisitBinaryOperator(BinaryOperator *bo);
void VisitCastExpr(CastExpr *ce);
@@ -249,10 +252,28 @@
vals[vd] = Initialized;
}
}
+ else if (Stmt *init = vd->getInit()) {
+ Visit(init);
+ }
}
}
}
+void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *dr) {
+ // We assume that DeclRefExprs wrapped in an lvalue-to-rvalue cast
+ // cannot be block-level expressions. Therefore, we determine if
+ // a DeclRefExpr is involved in a "load" by comparing it to the current
+ // DeclRefExpr found when analyzing the last lvalue-to-rvalue CastExpr.
+ // If a DeclRefExpr is not involved in a load, we are essentially computing
+ // its address, either for assignment to a reference or via the '&' operator.
+ // In such cases, treat the variable as being initialized, since this
+ // analysis isn't powerful enough to do alias tracking.
+ if (dr != currentDR)
+ if (const VarDecl *vd = dyn_cast<VarDecl>(dr->getDecl()))
+ if (isTrackedVar(vd))
+ vals[vd] = Initialized;
+}
+
static FindVarResult findBlockVarDecl(Expr* ex) {
if (DeclRefExpr* dr = dyn_cast<DeclRefExpr>(ex->IgnoreParenCasts()))
if (VarDecl *vd = dyn_cast<VarDecl>(dr->getDecl()))
@@ -263,55 +284,86 @@
}
void TransferFunctions::VisitBinaryOperator(clang::BinaryOperator *bo) {
- Visit(bo->getRHS());
- Visit(bo->getLHS());
if (bo->isAssignmentOp()) {
const FindVarResult &res = findBlockVarDecl(bo->getLHS());
if (const VarDecl* vd = res.getDecl()) {
+ // We assume that DeclRefExprs wrapped in a BinaryOperator "assignment"
+ // cannot be block-level expressions. Therefore, we determine if
+ // a DeclRefExpr is involved in a "load" by comparing it to the current
+ // DeclRefExpr found when analyzing the last lvalue-to-rvalue CastExpr.
+ SaveAndRestore<const DeclRefExpr*> lastDR(currentDR,
+ res.getDeclRefExpr());
+ Visit(bo->getRHS());
+ Visit(bo->getLHS());
+
llvm::BitVector::reference bit = vals[vd];
if (bit == Uninitialized) {
if (bo->getOpcode() != BO_Assign)
reportUninit(res.getDeclRefExpr(), vd);
bit = Initialized;
}
+ return;
}
}
+ Visit(bo->getRHS());
+ Visit(bo->getLHS());
}
void TransferFunctions::VisitUnaryOperator(clang::UnaryOperator *uo) {
- Visit(uo->getSubExpr());
switch (uo->getOpcode()) {
- case clang::UO_AddrOf:
- if (const VarDecl *vd = findBlockVarDecl(uo->getSubExpr()).getDecl())
- vals[vd] = Initialized;
- break;
case clang::UO_PostDec:
case clang::UO_PostInc:
case clang::UO_PreDec:
case clang::UO_PreInc: {
const FindVarResult &res = findBlockVarDecl(uo->getSubExpr());
if (const VarDecl *vd = res.getDecl()) {
+ // We assume that DeclRefExprs wrapped in a unary operator ++/--
+ // cannot be block-level expressions. Therefore, we determine if
+ // a DeclRefExpr is involved in a "load" by comparing it to the current
+ // DeclRefExpr found when analyzing the last lvalue-to-rvalue CastExpr.
+ SaveAndRestore<const DeclRefExpr*> lastDR(currentDR,
+ res.getDeclRefExpr());
+ Visit(uo->getSubExpr());
+
llvm::BitVector::reference bit = vals[vd];
if (bit == Uninitialized) {
reportUninit(res.getDeclRefExpr(), vd);
bit = Initialized;
}
+ return;
}
break;
}
default:
break;
}
+ Visit(uo->getSubExpr());
}
void TransferFunctions::VisitCastExpr(clang::CastExpr *ce) {
- Visit(ce->getSubExpr());
if (ce->getCastKind() == CK_LValueToRValue) {
const FindVarResult &res = findBlockVarDecl(ce->getSubExpr());
- if (const VarDecl *vd = res.getDecl())
- if (vals[vd] == Uninitialized)
+ if (const VarDecl *vd = res.getDecl()) {
+ // We assume that DeclRefExprs wrapped in an lvalue-to-rvalue cast
+ // cannot be block-level expressions. Therefore, we determine if
+ // a DeclRefExpr is involved in a "load" by comparing it to the current
+ // DeclRefExpr found when analyzing the last lvalue-to-rvalue CastExpr.
+ // Here we update 'currentDR' to be the one associated with this
+ // lvalue-to-rvalue cast. Then, when we analyze the DeclRefExpr, we
+ // will know that we are not computing its lvalue for other purposes
+ // than to perform a load.
+ SaveAndRestore<const DeclRefExpr*> lastDR(currentDR,
+ res.getDeclRefExpr());
+ Visit(ce->getSubExpr());
+ if (vals[vd] == Uninitialized) {
reportUninit(res.getDeclRefExpr(), vd);
- }
+ // Don't cascade warnings.
+ vals[vd] = Initialized;
+ }
+ return;
+ }
+ }
+ Visit(ce->getSubExpr());
}
//------------------------------------------------------------------------====//
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index 4463d3f..eb8590d 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -442,13 +442,9 @@
if (Diags.getDiagnosticLevel(diag::warn_var_is_uninit, D->getLocStart())
!= Diagnostic::Ignored) {
- if (!S.getLangOptions().CPlusPlus) {
- CFG *cfg = AC.getCFG();
- if (cfg) {
- UninitValsDiagReporter reporter(S);
- runUninitializedVariablesAnalysis(*cast<DeclContext>(D), *cfg,
- reporter);
- }
+ if (CFG *cfg = AC.getCFG()) {
+ UninitValsDiagReporter reporter(S);
+ runUninitializedVariablesAnalysis(*cast<DeclContext>(D), *cfg, reporter);
}
}
}
diff --git a/test/Sema/uninit-variables.c b/test/Sema/uninit-variables.c
index aabb976..58bcbb0 100644
--- a/test/Sema/uninit-variables.c
+++ b/test/Sema/uninit-variables.c
@@ -102,3 +102,12 @@
for (unsigned i = 0 ; i < 100 ; i++)
p[i] = 'a'; // no-warning
}
+
+void test17() {
+ // Don't warn multiple times about the same uninitialized variable
+ // along the same path.
+ int *x;
+ *x = 1; // expected-warning{{use of uninitialized variable 'x'}}
+ *x = 1; // no-warning
+}
+
diff --git a/test/SemaCXX/uninit-variables.cpp b/test/SemaCXX/uninit-variables.cpp
new file mode 100644
index 0000000..e971357
--- /dev/null
+++ b/test/SemaCXX/uninit-variables.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized-experimental -fsyntax-only %s -verify
+
+int test1_aux(int &x);
+int test1() {
+ int x;
+ test1_aux(x);
+ return x; // no-warning
+}
+
+int test2_aux() {
+ int x;
+ int &y = x;
+ return x; // no-warning
+}
+