From b63931eef6bfcf900216dc1a180b29dcaa287090 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Tue, 18 Jan 2011 21:18:58 +0000 Subject: [PATCH] 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). llvm-svn: 123777 --- clang/lib/Analysis/UninitializedValuesV2.cpp | 76 ++++++++++++++++---- clang/lib/Sema/AnalysisBasedWarnings.cpp | 10 +-- clang/test/Sema/uninit-variables.c | 9 +++ clang/test/SemaCXX/uninit-variables.cpp | 15 ++++ 4 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 clang/test/SemaCXX/uninit-variables.cpp diff --git a/clang/lib/Analysis/UninitializedValuesV2.cpp b/clang/lib/Analysis/UninitializedValuesV2.cpp index c5b093b7ade8..6bb8e0dbb155 100644 --- a/clang/lib/Analysis/UninitializedValuesV2.cpp +++ b/clang/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 @@ class TransferFunctions : public CFGRecStmtVisitor { 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 @@ void TransferFunctions::VisitDeclStmt(DeclStmt *ds) { 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(dr->getDecl())) + if (isTrackedVar(vd)) + vals[vd] = Initialized; +} + static FindVarResult findBlockVarDecl(Expr* ex) { if (DeclRefExpr* dr = dyn_cast(ex->IgnoreParenCasts())) if (VarDecl *vd = dyn_cast(dr->getDecl())) @@ -263,55 +284,86 @@ static FindVarResult findBlockVarDecl(Expr* ex) { } 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 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 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 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/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 4463d3fea53a..eb8590d60cbf 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -442,13 +442,9 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, 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(D), *cfg, - reporter); - } + if (CFG *cfg = AC.getCFG()) { + UninitValsDiagReporter reporter(S); + runUninitializedVariablesAnalysis(*cast(D), *cfg, reporter); } } } diff --git a/clang/test/Sema/uninit-variables.c b/clang/test/Sema/uninit-variables.c index aabb97634bab..58bcbb01d07f 100644 --- a/clang/test/Sema/uninit-variables.c +++ b/clang/test/Sema/uninit-variables.c @@ -102,3 +102,12 @@ void test16() { 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/clang/test/SemaCXX/uninit-variables.cpp b/clang/test/SemaCXX/uninit-variables.cpp new file mode 100644 index 000000000000..e971357ca78e --- /dev/null +++ b/clang/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 +} +