]> granicus.if.org Git - clang/commitdiff
static analyzer: refactor checking logic for returning the address of a stack variabl...
authorTed Kremenek <kremenek@apple.com>
Fri, 6 Nov 2009 02:24:13 +0000 (02:24 +0000)
committerTed Kremenek <kremenek@apple.com>
Fri, 6 Nov 2009 02:24:13 +0000 (02:24 +0000)
value into their own respective subclasses of Checker (and put them in .cpp files where their
implementation details are hidden from GRExprEngine).

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

include/clang/Analysis/PathSensitive/Checker.h
include/clang/Analysis/PathSensitive/CheckerVisitor.def
include/clang/Analysis/PathSensitive/GRExprEngine.h
lib/Analysis/CMakeLists.txt
lib/Analysis/GRExprEngine.cpp
lib/Analysis/GRExprEngineInternalChecks.cpp
lib/Analysis/GRExprEngineInternalChecks.h [new file with mode: 0644]
lib/Analysis/ReturnStackAddressChecker.cpp [new file with mode: 0644]
lib/Analysis/ReturnUndefChecker.cpp [new file with mode: 0644]

index 4fc0a617ecf090604135eca0f7f8c179b8949504..2564a73089b7c852f2c2eee35ba5e19157fd1605 100644 (file)
@@ -76,6 +76,10 @@ public:
   BugReporter &getBugReporter() {
     return Eng.getBugReporter();
   }
+  
+  SourceManager &getSourceManager() {
+    return getBugReporter().getSourceManager();
+  }
 
   ExplodedNode *GenerateNode(const Stmt *S, bool markAsSink = false) {
     return GenerateNode(S, getState(), markAsSink);
index ff6528dae8f565abc230a34b2ee2c5948c9e4df4..3f68a7a9d5c504e9219e780a44422e92590a5fe4 100644 (file)
@@ -11,8 +11,9 @@
 //
 //===---------------------------------------------------------------------===//
 
+PREVISIT(BinaryOperator)
 PREVISIT(CallExpr)
 PREVISIT(ObjCMessageExpr)
-PREVISIT(BinaryOperator)
+PREVISIT(ReturnStmt)
 
 #undef PREVISIT
index 25e47038d72ca933bef4c82e3dda0d937d62782e..f6971374e8e80f91b075e584b27454ab85c9880e 100644 (file)
@@ -223,10 +223,6 @@ public:
      return static_cast<CHECKER*>(lookupChecker(CHECKER::getTag()));
   }
 
-  bool isRetStackAddr(const ExplodedNode* N) const {
-    return N->isSink() && RetsStackAddr.count(const_cast<ExplodedNode*>(N)) != 0;
-  }
-
   bool isUndefControlFlow(const ExplodedNode* N) const {
     return N->isSink() && UndefBranches.count(const_cast<ExplodedNode*>(N)) != 0;
   }
@@ -267,14 +263,6 @@ public:
     return N->isSink() && UndefReceivers.count(const_cast<ExplodedNode*>(N)) != 0;
   }
 
-  typedef ErrorNodes::iterator ret_stackaddr_iterator;
-  ret_stackaddr_iterator ret_stackaddr_begin() { return RetsStackAddr.begin(); }
-  ret_stackaddr_iterator ret_stackaddr_end() { return RetsStackAddr.end(); }
-
-  typedef ErrorNodes::iterator ret_undef_iterator;
-  ret_undef_iterator ret_undef_begin() { return RetsUndef.begin(); }
-  ret_undef_iterator ret_undef_end() { return RetsUndef.end(); }
-
   typedef ErrorNodes::iterator undef_branch_iterator;
   undef_branch_iterator undef_branches_begin() { return UndefBranches.begin(); }
   undef_branch_iterator undef_branches_end() { return UndefBranches.end(); }
@@ -560,8 +548,6 @@ protected:
     getTF().EvalObjCMessageExpr(Dst, *this, *Builder, ME, Pred);
   }
 
-  void EvalReturn(ExplodedNodeSet& Dst, ReturnStmt* s, ExplodedNode* Pred);
-
   const GRState* MarkBranch(const GRState* St, Stmt* Terminator,
                             bool branchTaken);
 
index cd4697f2f3246d31c067f48ac1c7f9ac2a61fb86..111f273df6c6964ff4bd1dcba65e7b5a1d8518b5 100644 (file)
@@ -36,6 +36,8 @@ add_clang_library(clangAnalysis
   PathDiagnostic.cpp
   RangeConstraintManager.cpp
   RegionStore.cpp
+  ReturnStackAddressChecker.cpp
+  ReturnUndefChecker.cpp
   SVals.cpp
   SValuator.cpp
   SimpleConstraintManager.cpp
@@ -45,8 +47,8 @@ add_clang_library(clangAnalysis
   UndefinedArgChecker.cpp
   UndefinedAssignmentChecker.cpp
   UninitializedValues.cpp
-  ValueManager.cpp
   VLASizeChecker.cpp
+  ValueManager.cpp
   )
 
 add_dependencies(clangAnalysis ClangDiagnosticAnalysis)
index 212fea3a6bcce49ae02572bd15e1218832fca4a7..1bfb49d9ef8254b6018a139f784b3ecde9bf533a 100644 (file)
@@ -2628,63 +2628,37 @@ void GRExprEngine::VisitAsmStmtHelperInputs(AsmStmt* A,
     VisitAsmStmtHelperInputs(A, I, E, *NI, Dst);
 }
 
-void GRExprEngine::EvalReturn(ExplodedNodeSet& Dst, ReturnStmt* S,
-                              ExplodedNode* Pred) {
-  assert (Builder && "GRStmtNodeBuilder must be defined.");
-
-  unsigned size = Dst.size();
-
-  SaveAndRestore<bool> OldSink(Builder->BuildSinks);
-  SaveOr OldHasGen(Builder->HasGeneratedNode);
-
-  getTF().EvalReturn(Dst, *this, *Builder, S, Pred);
-
-  // Handle the case where no nodes where generated.
-
-  if (!Builder->BuildSinks && Dst.size() == size && !Builder->HasGeneratedNode)
-    MakeNode(Dst, S, Pred, GetState(Pred));
-}
-
-void GRExprEngine::VisitReturnStmt(ReturnStmt* S, ExplodedNode* Pred,
-                                   ExplodedNodeSet& Dst) {
-
-  Expr* R = S->getRetValue();
-
-  if (!R) {
-    EvalReturn(Dst, S, Pred);
-    return;
+void GRExprEngine::VisitReturnStmt(ReturnStmt *RS, ExplodedNode *Pred,
+                                   ExplodedNodeSet &Dst) {
+  
+  ExplodedNodeSet Src;
+  if (Expr *RetE = RS->getRetValue()) {
+    Visit(RetE, Pred, Src);
   }
+  else {
+    Src.Add(Pred);
+  }
+  
+  ExplodedNodeSet CheckedSet;
+  CheckerVisit(RS, CheckedSet, Src, true);
+  
+  for (ExplodedNodeSet::iterator I = CheckedSet.begin(), E = CheckedSet.end();
+       I != E; ++I) {
 
-  ExplodedNodeSet Tmp;
-  Visit(R, Pred, Tmp);
-
-  for (ExplodedNodeSet::iterator I = Tmp.begin(), E = Tmp.end(); I != E; ++I) {
-    SVal X = (*I)->getState()->getSVal(R);
-
-    // Check if we return the address of a stack variable.
-    if (isa<loc::MemRegionVal>(X)) {
-      // Determine if the value is on the stack.
-      const MemRegion* R = cast<loc::MemRegionVal>(&X)->getRegion();
-
-      if (R && R->hasStackStorage()) {
-        // Create a special node representing the error.
-        if (ExplodedNode* N = Builder->generateNode(S, GetState(*I), *I)) {
-          N->markAsSink();
-          RetsStackAddr.insert(N);
-        }
-        continue;
-      }
-    }
-    // Check if we return an undefined value.
-    else if (X.isUndef()) {
-      if (ExplodedNode* N = Builder->generateNode(S, GetState(*I), *I)) {
-        N->markAsSink();
-        RetsUndef.insert(N);
-      }
-      continue;
-    }
-
-    EvalReturn(Dst, S, *I);
+    assert(Builder && "GRStmtNodeBuilder must be defined.");
+    
+    Pred = *I;
+    unsigned size = Dst.size();
+    
+    SaveAndRestore<bool> OldSink(Builder->BuildSinks);
+    SaveOr OldHasGen(Builder->HasGeneratedNode);
+    
+    getTF().EvalReturn(Dst, *this, *Builder, RS, Pred);
+    
+    // Handle the case where no nodes where generated.    
+    if (!Builder->BuildSinks && Dst.size() == size && 
+        !Builder->HasGeneratedNode)
+      MakeNode(Dst, RS, Pred, GetState(Pred));
   }
 }
 
index 695f0b02e59713c510a4d764bbc3a10843acbf99..4bb5d226d17e4c7de9221bdfc92f77a47527796e 100644 (file)
@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "GRExprEngineInternalChecks.h"
 #include "clang/Analysis/PathSensitive/BugReporter.h"
 #include "clang/Analysis/PathSensitive/GRExprEngine.h"
 #include "clang/Analysis/PathSensitive/CheckerVisitor.h"
@@ -290,79 +291,6 @@ public:
   }
 };
 
-class VISIBILITY_HIDDEN RetStack : public BuiltinBug {
-public:
-  RetStack(GRExprEngine* eng)
-    : BuiltinBug(eng, "Return of address to stack-allocated memory") {}
-
-  void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
-    for (GRExprEngine::ret_stackaddr_iterator I=Eng.ret_stackaddr_begin(),
-         End = Eng.ret_stackaddr_end(); I!=End; ++I) {
-
-      ExplodedNode* N = *I;
-      const Stmt *S = cast<PostStmt>(N->getLocation()).getStmt();
-      const Expr* E = cast<ReturnStmt>(S)->getRetValue();
-      assert(E && "Return expression cannot be NULL");
-
-      // Get the value associated with E.
-      loc::MemRegionVal V = cast<loc::MemRegionVal>(N->getState()->getSVal(E));
-
-      // Generate a report for this bug.
-      std::string buf;
-      llvm::raw_string_ostream os(buf);
-      SourceRange R;
-
-      // Check if the region is a compound literal.
-      if (const CompoundLiteralRegion* CR =
-            dyn_cast<CompoundLiteralRegion>(V.getRegion())) {
-
-        const CompoundLiteralExpr* CL = CR->getLiteralExpr();
-        os << "Address of stack memory associated with a compound literal "
-              "declared on line "
-            << BR.getSourceManager()
-                    .getInstantiationLineNumber(CL->getLocStart())
-            << " returned.";
-
-        R = CL->getSourceRange();
-      }
-      else if (const AllocaRegion* AR = dyn_cast<AllocaRegion>(V.getRegion())) {
-        const Expr* ARE = AR->getExpr();
-        SourceLocation L = ARE->getLocStart();
-        R = ARE->getSourceRange();
-
-        os << "Address of stack memory allocated by call to alloca() on line "
-           << BR.getSourceManager().getInstantiationLineNumber(L)
-           << " returned.";
-      }
-      else {
-        os << "Address of stack memory associated with local variable '"
-           << V.getRegion()->getString() << "' returned.";
-      }
-
-      RangedBugReport *report = new RangedBugReport(*this, os.str().c_str(), N);
-      report->addRange(E->getSourceRange());
-      if (R.isValid()) report->addRange(R);
-      BR.EmitReport(report);
-    }
-  }
-};
-
-class VISIBILITY_HIDDEN RetUndef : public BuiltinBug {
-public:
-  RetUndef(GRExprEngine* eng) : BuiltinBug(eng, "Garbage return value",
-              "Undefined or garbage value returned to caller") {}
-
-  void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
-    Emit(BR, Eng.ret_undef_begin(), Eng.ret_undef_end());
-  }
-
-  void registerInitialVisitors(BugReporterContext& BRC,
-                               const ExplodedNode* N,
-                               BuiltinBugReport *R) {
-    registerTrackNullOrUndefValue(BRC, GetRetValExpr(N), N);
-  }
-};
-
 class VISIBILITY_HIDDEN UndefBranch : public BuiltinBug {
   struct VISIBILITY_HIDDEN FindUndefExpr {
     GRStateManager& VM;
@@ -464,8 +392,6 @@ void GRExprEngine::RegisterInternalChecks() {
   // to 'FlushReports' from BugReporter.
   BR.Register(new UndefBranch(this));
   BR.Register(new UndefResult(this));
-  BR.Register(new RetStack(this));
-  BR.Register(new RetUndef(this));
   BR.Register(new BadMsgExprArg(this));
   BR.Register(new BadReceiver(this));
   BR.Register(new OutOfBoundMemoryAccess(this));
@@ -477,6 +403,8 @@ void GRExprEngine::RegisterInternalChecks() {
   // their associated BugType will get registered with the BugReporter
   // automatically.  Note that the check itself is owned by the GRExprEngine
   // object.
+  RegisterReturnStackAddressChecker(*this);
+  RegisterReturnUndefChecker(*this);
   registerCheck(new AttrNonNullChecker());
   registerCheck(new UndefinedArgChecker());
   registerCheck(new UndefinedAssignmentChecker());
diff --git a/lib/Analysis/GRExprEngineInternalChecks.h b/lib/Analysis/GRExprEngineInternalChecks.h
new file mode 100644 (file)
index 0000000..622b5ab
--- /dev/null
@@ -0,0 +1,26 @@
+//=-- GRExprEngineInternalChecks.h- Builtin GRExprEngine Checks -----*- C++ -*-=
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines functions to instantiate and register the "built-in"
+//  checks in GRExprEngine.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_GREXPRENGINE_INTERNAL_CHECKS
+#define LLVM_CLANG_GREXPRENGINE_INTERNAL_CHECKS
+
+namespace clang {
+
+class GRExprEngine;
+
+void RegisterReturnStackAddressChecker(GRExprEngine &Eng);
+void RegisterReturnUndefChecker(GRExprEngine &Eng);
+  
+} // end clang namespace
+#endif
diff --git a/lib/Analysis/ReturnStackAddressChecker.cpp b/lib/Analysis/ReturnStackAddressChecker.cpp
new file mode 100644 (file)
index 0000000..9f22b3c
--- /dev/null
@@ -0,0 +1,97 @@
+//== ReturnStackAddressChecker.cpp ------------------------------*- C++ -*--==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines ReturnStackAddressChecker, which is a path-sensitive
+// check which looks for the addresses of stack variables being returned to
+// callers.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GRExprEngineInternalChecks.h"
+#include "clang/Analysis/PathSensitive/GRExprEngine.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang;
+
+namespace {
+class VISIBILITY_HIDDEN ReturnStackAddressChecker : 
+    public CheckerVisitor<ReturnStackAddressChecker> {      
+  BuiltinBug *BT;
+public:
+    ReturnStackAddressChecker() : BT(0) {}
+    static void *getTag();
+    void PreVisitReturnStmt(CheckerContext &C, const ReturnStmt *RS);
+};
+}
+
+void clang::RegisterReturnStackAddressChecker(GRExprEngine &Eng) {
+  Eng.registerCheck(new ReturnStackAddressChecker());
+}
+
+void *ReturnStackAddressChecker::getTag() {
+  static int x = 0; return &x;
+}
+
+void ReturnStackAddressChecker::PreVisitReturnStmt(CheckerContext &C,
+                                                   const ReturnStmt *RS) {
+  
+  const Expr *RetE = RS->getRetValue();
+  if (!RetE)
+    return;
+  SVal V = C.getState()->getSVal(RetE);
+  const MemRegion *R = V.getAsRegion();
+
+  if (!R || !R->hasStackStorage())
+    return;  
+  
+  ExplodedNode *N = C.GenerateNode(RS, C.getState(), true);
+
+  if (!N)
+    return;
+  
+  if (!BT)
+    BT = new BuiltinBug("Return of address to stack-allocated memory");
+  
+  // Generate a report for this bug.
+  llvm::SmallString<100> buf;
+  llvm::raw_svector_ostream os(buf);
+  SourceRange range;
+  
+  // Check if the region is a compound literal.
+  if (const CompoundLiteralRegion* CR = dyn_cast<CompoundLiteralRegion>(R)) {    
+    const CompoundLiteralExpr* CL = CR->getLiteralExpr();
+    os << "Address of stack memory associated with a compound literal "
+          "declared on line "
+       << C.getSourceManager().getInstantiationLineNumber(CL->getLocStart())
+       << " returned to caller";    
+    range = CL->getSourceRange();
+  }
+  else if (const AllocaRegion* AR = dyn_cast<AllocaRegion>(R)) {
+    const Expr* ARE = AR->getExpr();
+    SourceLocation L = ARE->getLocStart();
+    range = ARE->getSourceRange();    
+    os << "Address of stack memory allocated by call to alloca() on line "
+       << C.getSourceManager().getInstantiationLineNumber(L)
+       << " returned to caller";
+  }
+  else {
+    os << "Address of stack memory associated with local variable '"
+        << R->getString() << "' returned.";
+  }
+
+  RangedBugReport *report = new RangedBugReport(*BT, os.str().data(), N);
+  report->addRange(RS->getSourceRange());
+  if (range.isValid())
+    report->addRange(range);
+  
+  C.EmitReport(report);
+}
diff --git a/lib/Analysis/ReturnUndefChecker.cpp b/lib/Analysis/ReturnUndefChecker.cpp
new file mode 100644 (file)
index 0000000..adde3f5
--- /dev/null
@@ -0,0 +1,68 @@
+//== ReturnUndefChecker.cpp -------------------------------------*- C++ -*--==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines ReturnUndefChecker, which is a path-sensitive
+// check which looks for undefined or garbage values being returned to the
+// caller.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GRExprEngineInternalChecks.h"
+#include "clang/Analysis/PathSensitive/GRExprEngine.h"
+#include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang;
+
+namespace {
+class VISIBILITY_HIDDEN ReturnUndefChecker : 
+    public CheckerVisitor<ReturnUndefChecker> {      
+  BuiltinBug *BT;
+public:
+    ReturnUndefChecker() : BT(0) {}
+    static void *getTag();
+    void PreVisitReturnStmt(CheckerContext &C, const ReturnStmt *RS);
+};
+}
+
+void clang::RegisterReturnUndefChecker(GRExprEngine &Eng) {
+  Eng.registerCheck(new ReturnUndefChecker());
+}
+
+void *ReturnUndefChecker::getTag() {
+  static int x = 0; return &x;
+}
+
+void ReturnUndefChecker::PreVisitReturnStmt(CheckerContext &C,
+                                            const ReturnStmt *RS) {
+  const Expr *RetE = RS->getRetValue();
+  if (!RetE)
+    return;
+  
+  if (!C.getState()->getSVal(RetE).isUndef())
+    return;
+  
+  ExplodedNode *N = C.GenerateNode(RS, C.getState(), true);
+
+  if (!N)
+    return;
+  
+  if (!BT)
+    BT = new BuiltinBug("Garbage return value",
+                        "Undefined or garbage value returned to caller");
+    
+  EnhancedBugReport *report = 
+    new EnhancedBugReport(*BT, BT->getDescription().c_str(), N);  
+
+  report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, RetE);
+
+  C.EmitReport(report);
+}