From 5419a3121522fe1251d52c7f1fb790d68581e549 Mon Sep 17 00:00:00 2001 From: Adam Balogh Date: Mon, 11 May 2020 15:00:42 +0200 Subject: [PATCH] [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 --- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 14 -------------- clang/test/Analysis/explain-svals.cpp | 2 +- clang/test/Analysis/temporaries.cpp | 14 +++----------- clang/unittests/StaticAnalyzer/ParamRegionTest.cpp | 22 ++++++++++++++++++++-- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index dd6f78e..cb8693f 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 different 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 0510e41..c33373f 100644 --- a/clang/test/Analysis/explain-svals.cpp +++ b/clang/test/Analysis/explain-svals.cpp @@ -98,7 +98,7 @@ public: } // 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 325b689..1c0ac38 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@-3{{TRUE}} -#endif + // expected-warning@-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@-3{{TRUE}} -#else - // expected-warning@-5{{UNKNOWN}} -#endif + // expected-warning@-2{{TRUE}} #else - // expected-warning@-8{{UNKNOWN}} + // expected-warning@-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 9a666f7..c58af3f 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(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(Reg)); else assert(isa(Reg)); + checkForSameParamRegions(MRMgr, SFC, P); } } else if (const auto *CD = dyn_cast(D)) { for (const auto *P : CD->parameters()) { @@ -37,6 +47,7 @@ class ParamRegionTestConsumer : public ExprEngineConsumer { assert(isa(Reg)); else assert(isa(Reg)); + checkForSameParamRegions(MRMgr, SFC, P); } } else if (const auto *MD = dyn_cast(D)) { for (const auto *P : MD->parameters()) { @@ -45,6 +56,7 @@ class ParamRegionTestConsumer : public ExprEngineConsumer { assert(isa(Reg)); else assert(isa(Reg)); + checkForSameParamRegions(MRMgr, SFC, P); } } } @@ -71,7 +83,10 @@ public: TEST(ParamRegion, ParamRegionTest) { EXPECT_TRUE( tooling::runToolOnCode(std::make_unique(), - 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(), R"(@interface O -- 2.7.4