Add a checker check if a global variable holds a local variable's address after

the function call is left where the local variable is declared.


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

lib/Checker/CMakeLists.txt

@@ -57,6 +57,7 @@ add_clang_library(clangChecker
   ReturnUndefChecker.cpp
   SimpleConstraintManager.cpp
   SimpleSValuator.cpp
+  StackAddrLeakChecker.cpp
   Store.cpp
   SVals.cpp
   SValuator.cpp

lib/Checker/GRExprEngineExperimentalChecks.cpp

@@ -38,4 +38,5 @@ void clang::RegisterExperimentalInternalChecks(GRExprEngine &Eng) {
   RegisterCastToStructChecker(Eng);
   RegisterCastSizeChecker(Eng);
   RegisterArrayBoundChecker(Eng);
+  RegisterStackAddrLeakChecker(Eng);
 }

lib/Checker/GRExprEngineInternalChecks.h

@@ -36,6 +36,7 @@ void RegisterPointerSubChecker(GRExprEngine &Eng);
 void RegisterReturnPointerRangeChecker(GRExprEngine &Eng);
 void RegisterReturnStackAddressChecker(GRExprEngine &Eng);
 void RegisterReturnUndefChecker(GRExprEngine &Eng);
+void RegisterStackAddrLeakChecker(GRExprEngine &Eng);
 void RegisterUndefBranchChecker(GRExprEngine &Eng);
 void RegisterUndefCapturedBlockVarChecker(GRExprEngine &Eng);
 void RegisterUndefResultChecker(GRExprEngine &Eng);

lib/Checker/StackAddrLeakChecker.cpp

+//=== StackAddrLeakChecker.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 stack address leak checker, which checks if an invalid 
+// stack address is stored into a global or heap location. See CERT DCL30-C.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GRExprEngineInternalChecks.h"
+#include "clang/Checker/BugReporter/BugType.h"
+#include "clang/Checker/PathSensitive/Checker.h"
+#include "clang/Checker/PathSensitive/GRState.h"
+
+using namespace clang;
+
+namespace {
+class StackAddrLeakChecker : public Checker {
+  BuiltinBug *BT_stackleak;
+
+public:
+  StackAddrLeakChecker() : BT_stackleak(0) {}
+  static void *getTag() {
+    static int x;
+    return &x;
+  }
+
+  void EvalEndPath(GREndPathNodeBuilder &B, void *tag, GRExprEngine &Eng);
+};
+}
+
+void clang::RegisterStackAddrLeakChecker(GRExprEngine &Eng) {
+  Eng.registerCheck(new StackAddrLeakChecker());
+}
+
+void StackAddrLeakChecker::EvalEndPath(GREndPathNodeBuilder &B, void *tag,
+                                       GRExprEngine &Eng) {
+  SaveAndRestore<bool> OldHasGen(B.HasGeneratedNode);
+  const GRState *state = B.getState();
+  TranslationUnitDecl *TU = Eng.getContext().getTranslationUnitDecl();
+
+  // Check each global variable if it contains a MemRegionVal of a stack
+  // variable declared in the function we are leaving.
+  for (DeclContext::decl_iterator I = TU->decls_begin(), E = TU->decls_end();
+       I != E; ++I) {
+    if (VarDecl *VD = dyn_cast<VarDecl>(*I)) {
+      const LocationContext *LCtx = B.getPredecessor()->getLocationContext();
+      SVal L = state->getLValue(VD, LCtx);
+      SVal V = state->getSVal(cast<Loc>(L));
+      if (loc::MemRegionVal *RV = dyn_cast<loc::MemRegionVal>(&V)) {
+        const MemRegion *R = RV->getRegion();
+        // Strip fields or elements to get the variable region.
+        R = R->getBaseRegion();
+        if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
+          const VarDecl *VD = VR->getDecl();
+          const DeclContext *DC = VD->getDeclContext();
+          // Get the function where the variable is declared.
+          if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DC)) {
+            // Check if the function is the function we are leaving.
+            if (FD == LCtx->getDecl()) {
+              // The variable is declared in the function scope which we are 
+              // leaving. Keeping this variable's address in a global variable
+              // is dangerous.
+              // FIXME: Currently VarRegion does not carry context information.
+              // So we cannot tell if the local variable instance is in the
+              // current stack frame. This may produce false positive in 
+              // recursive function call context. But that's a rare case.
+
+              // FIXME: better warning location.
+
+              ExplodedNode *N = B.generateNode(state, tag, B.getPredecessor());
+              if (N) {
+                if (!BT_stackleak)
+                  BT_stackleak = new BuiltinBug("Stack address leak",
+                    "Stack address was saved into a global variable. "
+                    "This is dangerous because the address will become invalid "
+                    "after returning from the function.");
+                BugReport *R = new BugReport(*BT_stackleak, 
+                                             BT_stackleak->getDescription(), N);
+                Eng.getBugReporter().EmitReport(R);
+              }
+            }
+          }            
+        }
+      }
+    }
+  }
+}
+

test/Analysis/stackaddrleak.c

+// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-store region -analyzer-experimental-internal-checks -verify %s
+
+char const *p;
+
+void f0() {
+  char const str[] = "This will change";
+  p = str; // expected-warning {{Stack address was saved into a global variable.}}
+}
+
+void f1() {
+  char const str[] = "This will change";
+  p = str; 
+  p = 0; // no-warning
+}