]> granicus.if.org Git - clang/commitdiff
Reduce -Wuninitialized time by 22% (on sqlite) by removing the recursive AST crawl.
authorTed Kremenek <kremenek@apple.com>
Tue, 19 Jul 2011 14:18:48 +0000 (14:18 +0000)
committerTed Kremenek <kremenek@apple.com>
Tue, 19 Jul 2011 14:18:48 +0000 (14:18 +0000)
This is accomplished by forcing the needed expressions for -Wuninitialized to always be CFGElements in the CFG.
This allows us to remove a fair amount of the code for -Wuninitialized.

Some fallout:
- AnalysisBasedWarnings.cpp now specifically toggles the CFGBuilder to create a CFG that is suitable for -Wuninitialized.  This
is a layering violation, since the logic for -Wuninitialized is in libAnalysis.  This can be fixed with the proper refactoring.
- Some of the source locations for -Wunreachable-code warnings have shifted.  While not ideal, this is okay because that analysis
already needs some serious reworking.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@135480 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Analysis/UninitializedValues.cpp
lib/Sema/AnalysisBasedWarnings.cpp
test/Sema/warn-unreachable.c
test/SemaCXX/warn-unreachable.cpp

index 1d6959d81b16d2744cb0694c7f333e547211af80..8dbade1d5f325be464386405c21c6f952ed67632 100644 (file)
@@ -336,21 +336,29 @@ public:
   const VarDecl *getDecl() const { return vd; }
 };
   
-class TransferFunctions : public CFGRecStmtVisitor<TransferFunctions> {
+class TransferFunctions : public StmtVisitor<TransferFunctions> {
   CFGBlockValues &vals;
   const CFG &cfg;
   AnalysisContext &ac;
   UninitVariablesHandler *handler;
-  const DeclRefExpr *currentDR;
-  const Expr *currentVoidCast;
   const bool flagBlockUses;
+  
+  /// The last DeclRefExpr seen when analyzing a block.  Used to
+  /// cheat when detecting cases when the address of a variable is taken.
+  DeclRefExpr *lastDR;
+  
+  /// The last lvalue-to-rvalue conversion of a variable whose value
+  /// was uninitialized.  Normally this results in a warning, but it is
+  /// possible to either silence the warning in some cases, or we
+  /// propagate the uninitialized value.
+  CastExpr *lastLoad;
 public:
   TransferFunctions(CFGBlockValues &vals, const CFG &cfg,
                     AnalysisContext &ac,
                     UninitVariablesHandler *handler,
                     bool flagBlockUses)
-    : vals(vals), cfg(cfg), ac(ac), handler(handler), currentDR(0),
-      currentVoidCast(0), flagBlockUses(flagBlockUses) {}
+    : vals(vals), cfg(cfg), ac(ac), handler(handler),
+      flagBlockUses(flagBlockUses), lastDR(0), lastLoad(0) {}
   
   const CFG &getCFG() { return cfg; }
   void reportUninit(const DeclRefExpr *ex, const VarDecl *vd,
@@ -362,15 +370,16 @@ public:
   void VisitUnaryOperator(UnaryOperator *uo);
   void VisitBinaryOperator(BinaryOperator *bo);
   void VisitCastExpr(CastExpr *ce);
-  void VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *se);
-  void VisitCXXTypeidExpr(CXXTypeidExpr *E);
-  void BlockStmt_VisitObjCForCollectionStmt(ObjCForCollectionStmt *fs);
+  void VisitObjCForCollectionStmt(ObjCForCollectionStmt *fs);
+  void Visit(Stmt *s);
   
   bool isTrackedVar(const VarDecl *vd) {
     return ::isTrackedVar(vd, cast<DeclContext>(ac.getDecl()));
   }
   
   FindVarResult findBlockVarDecl(Expr *ex);
+  
+  void ProcessUses(Stmt *s = 0);
 };
 }
 
@@ -387,11 +396,7 @@ FindVarResult TransferFunctions::findBlockVarDecl(Expr* ex) {
   return FindVarResult(0, 0);
 }
 
-void TransferFunctions::BlockStmt_VisitObjCForCollectionStmt(
-    ObjCForCollectionStmt *fs) {
-  
-  Visit(fs->getCollection());
-  
+void TransferFunctions::VisitObjCForCollectionStmt(ObjCForCollectionStmt *fs) {
   // This represents an initialization of the 'element' value.
   Stmt *element = fs->getElement();
   const VarDecl* vd = 0;
@@ -405,10 +410,6 @@ void TransferFunctions::BlockStmt_VisitObjCForCollectionStmt(
     // Initialize the value of the reference variable.
     const FindVarResult &res = findBlockVarDecl(cast<Expr>(element));
     vd = res.getDecl();
-    if (!vd) {
-      Visit(element);
-      return;
-    }
   }
   
   if (vd)
@@ -436,14 +437,20 @@ void TransferFunctions::VisitBlockExpr(BlockExpr *be) {
   }
 }
 
+void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *dr) {
+  // Record the last DeclRefExpr seen.  This is an lvalue computation.
+  // We use this value to later detect if a variable "escapes" the analysis.
+  if (const VarDecl *vd = dyn_cast<VarDecl>(dr->getDecl()))
+    if (isTrackedVar(vd))
+      lastDR = dr;
+}
+
 void TransferFunctions::VisitDeclStmt(DeclStmt *ds) {
   for (DeclStmt::decl_iterator DI = ds->decl_begin(), DE = ds->decl_end();
        DI != DE; ++DI) {
     if (VarDecl *vd = dyn_cast<VarDecl>(*DI)) {
       if (isTrackedVar(vd)) {
         if (Expr *init = vd->getInit()) {
-          Visit(init);
-
           // If the initializer consists solely of a reference to itself, we
           // explicitly mark the variable as uninitialized. This allows code
           // like the following:
@@ -454,56 +461,34 @@ void TransferFunctions::VisitDeclStmt(DeclStmt *ds) {
           // clients can detect this pattern and adjust their reporting
           // appropriately, but we need to continue to analyze subsequent uses
           // of the variable.
-          DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(init->IgnoreParenImpCasts());
-          vals[vd] = (DRE && DRE->getDecl() == vd) ? Uninitialized
-                                                   : Initialized;
+          if (init == lastLoad) {
+            DeclRefExpr *DR = cast<DeclRefExpr>(lastLoad->getSubExpr());
+            vals[vd] = (DR->getDecl() == vd) ? Uninitialized : Initialized;
+            lastLoad = 0;
+            if (lastDR == DR)
+              lastDR = 0;
+          }
+          else {
+            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;
-}
-
 void TransferFunctions::VisitBinaryOperator(clang::BinaryOperator *bo) {
   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());
-
       ValueVector::reference val = vals[vd];
       if (isUninitialized(val)) {
         if (bo->getOpcode() != BO_Assign)
           reportUninit(res.getDeclRefExpr(), vd, isAlwaysUninit(val));
         val = Initialized;
       }
-      return;
     }
   }
-  Visit(bo->getRHS());
-  Visit(bo->getLHS());
 }
 
 void TransferFunctions::VisitUnaryOperator(clang::UnaryOperator *uo) {
@@ -514,13 +499,10 @@ void TransferFunctions::VisitUnaryOperator(clang::UnaryOperator *uo) {
     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());
+        assert(res.getDeclRefExpr() == lastDR);
+        // We null out lastDR to indicate we have fully processed it
+        // and we don't want the auto-value setting in Visit().
+        lastDR = 0;
 
         ValueVector::reference val = vals[vd];
         if (isUninitialized(val)) {
@@ -528,72 +510,76 @@ void TransferFunctions::VisitUnaryOperator(clang::UnaryOperator *uo) {
           // Don't cascade warnings.
           val = Initialized;
         }
-        return;
       }
       break;
     }
     default:
       break;
   }
-  Visit(uo->getSubExpr());
 }
 
 void TransferFunctions::VisitCastExpr(clang::CastExpr *ce) {
   if (ce->getCastKind() == CK_LValueToRValue) {
     const FindVarResult &res = findBlockVarDecl(ce->getSubExpr());
     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 (currentVoidCast != ce) {
-        Value val = vals[vd];
-        if (isUninitialized(val)) {
-          reportUninit(res.getDeclRefExpr(), vd, isAlwaysUninit(val));
-          // Don't cascade warnings.
-          vals[vd] = Initialized;
-        }
+      assert(res.getDeclRefExpr() == lastDR);
+      if (isUninitialized(vals[vd])) {
+        // Record this load of an uninitialized value.  Normally this
+        // results in a warning, but we delay reporting the issue
+        // in case it is wrapped in a void cast, etc.
+        lastLoad = ce;
       }
-      return;
     }
   }
   else if (CStyleCastExpr *cse = dyn_cast<CStyleCastExpr>(ce)) {
     if (cse->getType()->isVoidType()) {
       // e.g. (void) x;
-      SaveAndRestore<const Expr *>
-        lastVoidCast(currentVoidCast, cse->getSubExpr()->IgnoreParens());
-      Visit(cse->getSubExpr());
-      return;
+      if (lastLoad == cse->getSubExpr()) {
+        // Squelch any detected load of an uninitialized value if
+        // we cast it to void.
+        lastLoad = 0;
+        lastDR = 0;
+      }
     }
   }
-  Visit(ce->getSubExpr());
 }
 
-void TransferFunctions::VisitUnaryExprOrTypeTraitExpr(
-                                          UnaryExprOrTypeTraitExpr *se) {
-  if (se->getKind() == UETT_SizeOf) {
-    if (se->getType()->isConstantSizeType())
+void TransferFunctions::Visit(clang::Stmt *s) {
+  StmtVisitor<TransferFunctions>::Visit(s);
+  ProcessUses(s);
+}
+
+void TransferFunctions::ProcessUses(Stmt *s) {
+  // This method is typically called after visiting a CFGElement statement
+  // in the CFG.  We delay processing of reporting many loads of uninitialized
+  // values until here.
+  if (lastLoad) {
+    // If we just visited the lvalue-to-rvalue cast, there is nothing
+    // left to do.
+    if (lastLoad == s)
       return;
-    // Handle VLAs.
-    Visit(se->getArgumentExpr());
+
+    // If we reach here, we have seen a load of an uninitialized value
+    // and it hasn't been casted to void or otherwise handled.  In this
+    // situation, report the incident.
+    DeclRefExpr *DR = cast<DeclRefExpr>(lastLoad->getSubExpr());
+    VarDecl *VD = cast<VarDecl>(DR->getDecl());
+    reportUninit(DR, VD, isAlwaysUninit(vals[VD]));
+    lastLoad = 0;
+    
+    // Prevent cascade of warnings.
+    vals[VD] = Initialized;
+    if (DR == lastDR) {
+      lastDR = 0;
+      return;
+    }
   }
-}
 
-void TransferFunctions::VisitCXXTypeidExpr(CXXTypeidExpr *E) {
-  // typeid(expression) is potentially evaluated when the argument is
-  // a glvalue of polymorphic type. (C++ 5.2.8p2-3)
-  if (!E->isTypeOperand() && E->Classify(ac.getASTContext()).isGLValue()) {
-    QualType SubExprTy = E->getExprOperand()->getType();
-    if (const RecordType *Record = SubExprTy->getAs<RecordType>())
-      if (cast<CXXRecordDecl>(Record->getDecl())->isPolymorphic())
-        Visit(E->getExprOperand());
+  // Any other uses of 'lastDR' involve taking an lvalue of variable.
+  // In this case, it "escapes" the analysis.
+  if (lastDR && lastDR != s) {
+    vals[cast<VarDecl>(lastDR->getDecl())] = Initialized;
+    lastDR = 0;
   }
 }
 
@@ -648,9 +634,10 @@ static bool runOnBlock(const CFGBlock *block, const CFG &cfg,
   for (CFGBlock::const_iterator I = block->begin(), E = block->end(); 
        I != E; ++I) {
     if (const CFGStmt *cs = dyn_cast<CFGStmt>(&*I)) {
-      tf.BlockStmt_Visit(cs->getStmt());
+      tf.Visit(cs->getStmt());
     }
   }
+  tf.ProcessUses();
   return vals.updateValueVectorWithScratch(block);
 }
 
index 7a14855e69cd3d0a462c2551828353b1d383374a..c0e271590dc1c1f30ea51e0f117ddd810af43c37 100644 (file)
@@ -660,6 +660,20 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
   // explosion for destrutors that can result and the compile time hit.
   AnalysisContext AC(D, 0, /*useUnoptimizedCFG=*/false, /*addehedges=*/false,
                      /*addImplicitDtors=*/true, /*addInitializers=*/true);
+  
+  // Force that certain expressions appear as CFGElements in the CFG.  This
+  // is used to speed up various analyses.
+  // FIXME: This isn't the right factoring.  This is here for initial
+  // prototyping, but we need a way for analyses to say what expressions they
+  // expect to always be CFGElements and then fill in the BuildOptions
+  // appropriately.  This is essentially a layering violation.
+  CFG::BuildOptions &buildOptions = AC.getCFGBuildOptions();
+  buildOptions.setAlwaysAdd(Stmt::BinaryOperatorClass);
+  buildOptions.setAlwaysAdd(Stmt::BlockExprClass);
+  buildOptions.setAlwaysAdd(Stmt::CStyleCastExprClass);
+  buildOptions.setAlwaysAdd(Stmt::DeclRefExprClass);
+  buildOptions.setAlwaysAdd(Stmt::ImplicitCastExprClass);
+  buildOptions.setAlwaysAdd(Stmt::UnaryOperatorClass);
 
   // Emit delayed diagnostics.
   if (!fscope->PossiblyUnreachableDiags.empty()) {
index 20e0c3172401f4077c79eb4e092cce1aed607c16..80f32cd73281f25a0089f6d33fd9a1fb3e6938e8 100644 (file)
@@ -80,8 +80,8 @@ void test2() {
     -           // expected-warning {{will never be executed}}
       halt();
   case 8:
-    i
-      +=        // expected-warning {{will never be executed}}
+    i           // expected-warning {{will never be executed}}
+      +=
       halt();
   case 9:
     halt()
@@ -93,8 +93,8 @@ void test2() {
   case 11: {
     int a[5];
     live(),
-      a[halt()
-        ];      // expected-warning {{will never be executed}}
+      a[halt()  // expected-warning {{will never be executed}}
+        ];
   }
   }
 }
index ea6755f2d6ce466992b1b2507c972088aac7024b..604a3c0da38167d7e5930790cae8ee7e2bb4256e 100644 (file)
@@ -45,8 +45,8 @@ void test3() {
     ? 
     dead() : dead();
   live(),
-    float       // expected-warning {{will never be executed}}
-      (halt());
+    float       
+      (halt()); // expected-warning {{will never be executed}}
 }
 
 void test4() {
@@ -73,6 +73,6 @@ void test6() {
     S(int i) { }
   };
   live(),
-    S            // expected-warning {{will never be executed}}
-      (halt());
+    S
+      (halt());  // expected-warning {{will never be executed}}
 }