[static analyzer] Extend VLA size checking to look for undefined sizes.

llvm-svn: 60734

clang/include/clang/Analysis/PathSensitive/GRExprEngine.h

@@ -139,13 +139,13 @@ public:
   ///  MUST be zero or undefined.
   ErrorNodes ExplicitBadDivides;
   
-  /// ImplicitZeroSizedVLA - Nodes in the ExplodedGraph that result from 
+  /// ImplicitBadSizedVLA - Nodes in the ExplodedGraph that result from 
   ///  constructing a zero-sized VLA where the size may be zero.
-  ErrorNodes ImplicitZeroSizedVLA;
+  ErrorNodes ImplicitBadSizedVLA;
   
-  /// ExplicitZeroSizedVLA - Nodes in the ExplodedGraph that result from 
+  /// ExplicitBadSizedVLA - Nodes in the ExplodedGraph that result from 
   ///  constructing a zero-sized VLA where the size must be zero.
-  ErrorNodes ExplicitZeroSizedVLA;
+  ErrorNodes ExplicitBadSizedVLA;
   
   /// UndefResults - Nodes in the ExplodedGraph where the operands are defined
   ///  by the result is not.  Excludes divide-by-zero errors.
@@ -453,7 +453,7 @@ protected:
   const GRState* BindLoc(const GRState* St, Loc LV, SVal V) {
     return StateMgr.BindLoc(St, LV, V);
   }
-  
+
   SVal GetSVal(const GRState* St, Stmt* Ex) {
     return StateMgr.GetSVal(St, Ex);
   }

clang/lib/Analysis/GRExprEngine.cpp

@@ -1820,6 +1820,14 @@ void GRExprEngine::VisitDeclStmt(DeclStmt* DS, NodeTy* Pred, NodeSet& Dst) {
       
       Expr* SE = VLA->getSizeExpr();
       SVal Size = GetSVal(St, SE);
+      
+      if (Size.isUndef()) {
+        if (NodeTy* N = Builder->generateNode(DS, St, Pred)) {
+          N->markAsSink();          
+          ExplicitBadSizedVLA.insert(N);
+        }
+        continue;
+      }
 
       bool isFeasibleZero = false;
       const GRState* ZeroSt =  Assume(St, Size, false, isFeasibleZero);
@@ -1830,8 +1838,8 @@ void GRExprEngine::VisitDeclStmt(DeclStmt* DS, NodeTy* Pred, NodeSet& Dst) {
       if (isFeasibleZero) {
         if (NodeTy* N = Builder->generateNode(DS, ZeroSt, Pred)) {
           N->markAsSink();          
-          if (isFeasibleNotZero) ImplicitZeroSizedVLA.insert(N);
+          if (isFeasibleNotZero) ImplicitBadSizedVLA.insert(N);
-          else ExplicitZeroSizedVLA.insert(N);
+          else ExplicitBadSizedVLA.insert(N);
         }
       }
       

clang/lib/Analysis/GRExprEngineInternalChecks.cpp

@@ -40,6 +40,7 @@ ExplodedNode<GRState>* GetNode(GRExprEngine::undef_arg_iterator I) {
 
 namespace {
 class VISIBILITY_HIDDEN BuiltinBug : public BugTypeCacheLocation {
+protected:
   const char* name;
   const char* desc;
 public:
@@ -332,26 +333,45 @@ public:
   }
 };
   
-class VISIBILITY_HIDDEN ZeroSizeVLA : public BuiltinBug {
+class VISIBILITY_HIDDEN BadSizeVLA : public BuiltinBug {
 
 public:
-  ZeroSizeVLA() : BuiltinBug("Zero-sized VLA",  
+  BadSizeVLA() : BuiltinBug("Zero-sized VLA",  
                              "VLAs with zero-size are undefined.") {}
   
   virtual void EmitBuiltinWarnings(BugReporter& BR, GRExprEngine& Eng) {
     for (GRExprEngine::ErrorNodes::iterator
-          I = Eng.ExplicitZeroSizedVLA.begin(),
+          I = Eng.ExplicitBadSizedVLA.begin(),
-          E = Eng.ExplicitZeroSizedVLA.end(); I!=E; ++I) {
+          E = Eng.ExplicitBadSizedVLA.end(); I!=E; ++I) {
-      
+
-      // Generate a report for this bug.
+      // Determine whether this was a 'zero-sized' VLA or a VLA with an
-      PostStmt PS = cast<PostStmt>((*I)->getLocation());      
+      // undefined size.
+      GRExprEngine::NodeTy* N = *I;
+      PostStmt PS = cast<PostStmt>(N->getLocation());      
       DeclStmt *DS = cast<DeclStmt>(PS.getStmt());
       VarDecl* VD = cast<VarDecl>(*DS->decl_begin());
       QualType T = Eng.getContext().getCanonicalType(VD->getType());
       VariableArrayType* VT = cast<VariableArrayType>(T);
+      Expr* SizeExpr = VT->getSizeExpr();
       
-      RangedBugReport report(*this, *I);
+      std::string buf;
-      report.addRange(VT->getSizeExpr()->getSourceRange());
+      llvm::raw_string_ostream os(buf);
+      os << "The expression used to specify the number of elements in the VLA '"
+          << VD->getNameAsString() << "' evaluates to ";
+      
+      SVal X = Eng.getStateManager().GetSVal(N->getState(), SizeExpr);
+      if (X.isUndef()) {
+        name = "Undefined size for VLA";
+        os << "an undefined or garbage value.";
+      }
+      else {
+        name = "Zero-sized VLA";
+        os << " to 0.  VLAs with no elements have undefined behavior.";
+      }
+
+      desc = os.str().c_str();
+      RangedBugReport report(*this, N);
+      report.addRange(SizeExpr->getSourceRange());
       
       // Emit the warning.
       BR.EmitWarning(report);
@@ -430,6 +450,6 @@ void GRExprEngine::RegisterInternalChecks() {
   Register(new BadMsgExprArg());
   Register(new BadReceiver());
   Register(new OutOfBoundMemoryAccess());
-  Register(new ZeroSizeVLA());
+  Register(new BadSizeVLA());
   AddCheck(new CheckAttrNonNull(), Stmt::CallExprClass); 
 }

clang/test/Analysis/misc-ps.m

@@ -79,6 +79,11 @@ void check_zero_sized_VLA(int x) {
   if (x)
     return;
 
-  int vla[x]; // expected-warning{{VLAs with zero-size are undefined}}
+  int vla[x]; // expected-warning{{VLAs with no elements have undefined behavior}}
+}
+
+void check_uninit_sized_VLA() {
+  int x;
+  int vla[x]; // expected-warning{{The expression used to specify the number of elements in the VLA 'vla' evaluates to an undefined or garbage value.}}
 }