[clang] 5419a31 - [Analyzer] Allow creation of stack frame for functions without definition

Adam Balogh via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 9 03:07:25 PDT 2020


Author: Adam Balogh
Date: 2020-06-09T12:08:57+02:00
New Revision: 5419a3121522fe1251d52c7f1fb790d68581e549

URL: https://github.com/llvm/llvm-project/commit/5419a3121522fe1251d52c7f1fb790d68581e549
DIFF: https://github.com/llvm/llvm-project/commit/5419a3121522fe1251d52c7f1fb790d68581e549.diff

LOG: [Analyzer] Allow creation of stack frame for functions without definition

Retrieving the parameter location of functions was disabled because it
may causes crashes due to the fact that functions may have multiple
declarations and without definition it is difficult to ensure that
always the same declration is used. Now parameters are stored in
`ParamRegions` which are independent of the declaration of the function,
therefore the same parameters always have the same regions,
independently of the function declaration used actually. This allows us
to remove the limitation described above.

Differential Revision: https://reviews.llvm.org/D80286

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/CallEvent.cpp
    clang/test/Analysis/explain-svals.cpp
    clang/test/Analysis/temporaries.cpp
    clang/unittests/StaticAnalyzer/ParamRegionTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index dd6f78e7143f..cb8693f3c34c 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -172,23 +172,9 @@ AnalysisDeclContext *CallEvent::getCalleeAnalysisDeclContext() const {
   if (!D)
     return nullptr;
 
-  // TODO: For now we skip functions without definitions, even if we have
-  // our own getDecl(), because it's hard to find out which re-declaration
-  // is going to be used, and usually clients don't really care about this
-  // situation because there's a loss of precision anyway because we cannot
-  // inline the call.
-  RuntimeDefinition RD = getRuntimeDefinition();
-  if (!RD.getDecl())
-    return nullptr;
-
   AnalysisDeclContext *ADC =
       LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
 
-  // TODO: For now we skip virtual functions, because this also rises
-  // the problem of which decl to use, but now it's across 
diff erent classes.
-  if (RD.mayHaveOtherDefinitions() || RD.getDecl() != ADC->getDecl())
-    return nullptr;
-
   return ADC;
 }
 

diff  --git a/clang/test/Analysis/explain-svals.cpp b/clang/test/Analysis/explain-svals.cpp
index 0510e4156f51..c33373f00de2 100644
--- a/clang/test/Analysis/explain-svals.cpp
+++ b/clang/test/Analysis/explain-svals.cpp
@@ -98,7 +98,7 @@ class C {
 } // end of anonymous namespace
 
 void test_6() {
-  clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of temporary object constructed at statement 'conjure_S\(\)'$}}}}
+  clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of 1st parameter of function 'clang_analyzer_explain\(\)'$}}}}
   clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}}
 }
 

diff  --git a/clang/test/Analysis/temporaries.cpp b/clang/test/Analysis/temporaries.cpp
index 325b689c0deb..1c0ac38160c2 100644
--- a/clang/test/Analysis/temporaries.cpp
+++ b/clang/test/Analysis/temporaries.cpp
@@ -890,12 +890,9 @@ class C {
 public:
   ~C() {
     glob = 1;
-    // FIXME: Why is destructor not inlined in C++17
     clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS
-#if __cplusplus < 201703L
-    // expected-warning at -3{{TRUE}}
-#endif
+    // expected-warning at -2{{TRUE}}
 #endif
   }
 };
@@ -914,16 +911,11 @@ void test(int coin) {
   // temporaries returned from functions, so we took the wrong branch.
   coin && is(get()); // no-crash
   if (coin) {
-    // FIXME: Why is destructor not inlined in C++17
     clang_analyzer_eval(glob);
 #ifdef TEMPORARY_DTORS
-#if __cplusplus < 201703L
-    // expected-warning at -3{{TRUE}}
-#else
-    // expected-warning at -5{{UNKNOWN}}
-#endif
+    // expected-warning at -2{{TRUE}}
 #else
-    // expected-warning at -8{{UNKNOWN}}
+    // expected-warning at -4{{UNKNOWN}}
 #endif
   } else {
     // The destructor is not called on this branch.

diff  --git a/clang/unittests/StaticAnalyzer/ParamRegionTest.cpp b/clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
index 9a666f7fb615..c58af3f851c4 100644
--- a/clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
@@ -16,6 +16,15 @@ namespace ento {
 namespace {
 
 class ParamRegionTestConsumer : public ExprEngineConsumer {
+  void checkForSameParamRegions(MemRegionManager &MRMgr,
+                                const StackFrameContext *SFC,
+                                const ParmVarDecl *PVD) {
+    for (const auto *D2: PVD->redecls()) {
+      const auto *PVD2 = cast<ParmVarDecl>(D2);
+      assert(MRMgr.getVarRegion(PVD, SFC) == MRMgr.getVarRegion(PVD2, SFC));
+    }
+  }
+
   void performTest(const Decl *D) {
     StoreManager &StMgr = Eng.getStoreManager();
     MemRegionManager &MRMgr = StMgr.getRegionManager();
@@ -29,6 +38,7 @@ class ParamRegionTestConsumer : public ExprEngineConsumer {
           assert(isa<NonParamVarRegion>(Reg));
         else
           assert(isa<ParamVarRegion>(Reg));
+        checkForSameParamRegions(MRMgr, SFC, P);
       }
     } else if (const auto *CD = dyn_cast<CXXConstructorDecl>(D)) {
       for (const auto *P : CD->parameters()) {
@@ -37,6 +47,7 @@ class ParamRegionTestConsumer : public ExprEngineConsumer {
           assert(isa<NonParamVarRegion>(Reg));
         else
           assert(isa<ParamVarRegion>(Reg));
+        checkForSameParamRegions(MRMgr, SFC, P);
       }
     } else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
       for (const auto *P : MD->parameters()) {
@@ -45,6 +56,7 @@ class ParamRegionTestConsumer : public ExprEngineConsumer {
           assert(isa<NonParamVarRegion>(Reg));
         else
           assert(isa<ParamVarRegion>(Reg));
+        checkForSameParamRegions(MRMgr, SFC, P);
       }
     }
   }
@@ -71,7 +83,10 @@ class ParamRegionTestAction : public ASTFrontendAction {
 TEST(ParamRegion, ParamRegionTest) {
   EXPECT_TRUE(
       tooling::runToolOnCode(std::make_unique<ParamRegionTestAction>(),
-                             R"(void foo(int n) {
+                             R"(void foo(int n);
+                                void baz(int p);
+
+                                void foo(int n) {
                                   auto lambda = [n](int m) {
                                     return n + m;
                                   };
@@ -90,7 +105,10 @@ TEST(ParamRegion, ParamRegionTest) {
 
                                 void baz(int p) {
                                   S s(p);
-                                })"));
+                                }
+
+                                void bar(int l);
+                                void baz(int p);)"));
   EXPECT_TRUE(
       tooling::runToolOnCode(std::make_unique<ParamRegionTestAction>(),
                              R"(@interface O


        


More information about the cfe-commits mailing list