From c28ce29a121f993ddc1192d82c5576cd8c98b703 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Tue, 9 Dec 2008 00:44:16 +0000 Subject: [PATCH] [static analyzer] Extend VLA size checking to look for undefined sizes. llvm-svn: 60734 --- .../Analysis/PathSensitive/GRExprEngine.h | 10 ++--- clang/lib/Analysis/GRExprEngine.cpp | 12 +++++- .../Analysis/GRExprEngineInternalChecks.cpp | 40 ++++++++++++++----- clang/test/Analysis/misc-ps.m | 7 +++- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h b/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h index 80165c5686a5..6f86102ae155 100644 --- a/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/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); } diff --git a/clang/lib/Analysis/GRExprEngine.cpp b/clang/lib/Analysis/GRExprEngine.cpp index fb520675734d..343ac697a302 100644 --- a/clang/lib/Analysis/GRExprEngine.cpp +++ b/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); - else ExplicitZeroSizedVLA.insert(N); + if (isFeasibleNotZero) ImplicitBadSizedVLA.insert(N); + else ExplicitBadSizedVLA.insert(N); } } diff --git a/clang/lib/Analysis/GRExprEngineInternalChecks.cpp b/clang/lib/Analysis/GRExprEngineInternalChecks.cpp index 8b484ab10c0d..d32318ca7d91 100644 --- a/clang/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/clang/lib/Analysis/GRExprEngineInternalChecks.cpp @@ -40,6 +40,7 @@ ExplodedNode* 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(), - E = Eng.ExplicitZeroSizedVLA.end(); I!=E; ++I) { - - // Generate a report for this bug. - PostStmt PS = cast((*I)->getLocation()); + I = Eng.ExplicitBadSizedVLA.begin(), + E = Eng.ExplicitBadSizedVLA.end(); I!=E; ++I) { + + // Determine whether this was a 'zero-sized' VLA or a VLA with an + // undefined size. + GRExprEngine::NodeTy* N = *I; + PostStmt PS = cast(N->getLocation()); DeclStmt *DS = cast(PS.getStmt()); VarDecl* VD = cast(*DS->decl_begin()); QualType T = Eng.getContext().getCanonicalType(VD->getType()); VariableArrayType* VT = cast(T); + Expr* SizeExpr = VT->getSizeExpr(); - RangedBugReport report(*this, *I); - report.addRange(VT->getSizeExpr()->getSourceRange()); + std::string buf; + 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); } diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m index 40322f1f0c55..be697f8f3a5e 100644 --- a/clang/test/Analysis/misc-ps.m +++ b/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.}} }