From bd06c417e6c717cbe33b566d7bbaf27fb47e763a Mon Sep 17 00:00:00 2001 From: Valeriy Savchenko Date: Tue, 28 Apr 2020 12:21:39 +0300 Subject: [PATCH] [analyzer] Allow bindings of the CompoundLiteralRegion Summary: CompoundLiteralRegions have been properly modeled before, but 'getBindingForElement` was not changed to accommodate this change properly. rdar://problem/46144644 Differential Revision: https://reviews.llvm.org/D78990 --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 4 - clang/test/Analysis/compound-literals.c | 17 ++- .../Analysis/retain-release-compound-literal.m | 25 ++++ clang/unittests/StaticAnalyzer/StoreTest.cpp | 142 ++++++++++++++------- 4 files changed, 137 insertions(+), 51 deletions(-) create mode 100644 clang/test/Analysis/retain-release-compound-literal.m diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 2a55c99..57fde32 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -1628,10 +1628,6 @@ RegionStoreManager::findLazyBinding(RegionBindingsConstRef B, SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B, const ElementRegion* R) { - // We do not currently model bindings of the CompoundLiteralregion. - if (isa(R->getBaseRegion())) - return UnknownVal(); - // Check if the region has a binding. if (const Optional &V = B.getDirectBinding(R)) return *V; diff --git a/clang/test/Analysis/compound-literals.c b/clang/test/Analysis/compound-literals.c index f8b9121..42e6a55 100644 --- a/clang/test/Analysis/compound-literals.c +++ b/clang/test/Analysis/compound-literals.c @@ -1,4 +1,7 @@ -// RUN: %clang_cc1 -triple=i386-apple-darwin10 -analyze -analyzer-checker=debug.ExprInspection -verify %s +// RUN: %clang_cc1 -triple=i386-apple-darwin10 -verify %s -analyze \ +// RUN: -analyzer-checker=debug.ExprInspection + +#define NULL 0 void clang_analyzer_eval(int); // pr28449: Used to crash. @@ -6,3 +9,15 @@ void foo(void) { static const unsigned short array[] = (const unsigned short[]){0x0F00}; clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{TRUE}} } + +// check that we propagate info through compound literal regions +void bar() { + int *integers = (int[]){1, 2, 3}; + clang_analyzer_eval(integers[0] == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(integers[1] == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(integers[2] == 3); // expected-warning{{TRUE}} + + int **pointers = (int *[]){&integers[0], NULL}; + clang_analyzer_eval(pointers[0] == NULL); // expected-warning{{FALSE}} + clang_analyzer_eval(pointers[1] == NULL); // expected-warning{{TRUE}} +} diff --git a/clang/test/Analysis/retain-release-compound-literal.m b/clang/test/Analysis/retain-release-compound-literal.m new file mode 100644 index 0000000..29a1253 --- /dev/null +++ b/clang/test/Analysis/retain-release-compound-literal.m @@ -0,0 +1,25 @@ +// RUN: %clang_analyze_cc1 -verify -Wno-objc-root-class %s \ +// RUN: -analyzer-checker=core,osx.cocoa.RetainCount + +#define NULL 0 +#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained)) +#define CF_CONSUMED __attribute__((cf_consumed)) + +void clang_analyzer_eval(int); + +typedef const void *CFTypeRef; + +extern CFTypeRef CFCreate() CF_RETURNS_RETAINED; +extern CFTypeRef CFRetain(CFTypeRef cf); +extern void CFRelease(CFTypeRef cf); + +void bar(CFTypeRef *v) {} + +void test1() { + CFTypeRef *values = (CFTypeRef[]){ + CFCreate(), // no-warning + CFCreate(), // expected-warning{{leak}} + CFCreate()}; // no-warning + CFRelease(values[0]); + CFRelease(values[2]); +} diff --git a/clang/unittests/StaticAnalyzer/StoreTest.cpp b/clang/unittests/StaticAnalyzer/StoreTest.cpp index c8b930b..17b64ce 100644 --- a/clang/unittests/StaticAnalyzer/StoreTest.cpp +++ b/clang/unittests/StaticAnalyzer/StoreTest.cpp @@ -15,89 +15,139 @@ namespace clang { namespace ento { namespace { +class StoreTestConsumer : public ExprEngineConsumer { +public: + StoreTestConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {} + + bool HandleTopLevelDecl(DeclGroupRef DG) override { + for (const auto *D : DG) + performTest(D); + return true; + } + +private: + virtual void performTest(const Decl *D) = 0; +}; + +template class TestAction : public ASTFrontendAction { +public: + std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler, + StringRef File) override { + return std::make_unique(Compiler); + } +}; + // Test that we can put a value into an int-type variable and load it // back from that variable. Test what happens if default bindings are used. -class VariableBindConsumer : public ExprEngineConsumer { - void performTest(const Decl *D) { - StoreManager &StMgr = Eng.getStoreManager(); - SValBuilder &SVB = Eng.getSValBuilder(); - MemRegionManager &MRMgr = StMgr.getRegionManager(); - const ASTContext &ACtx = Eng.getContext(); +class VariableBindConsumer : public StoreTestConsumer { + void performTest(const Decl *D) override { + StoreManager &SManager = Eng.getStoreManager(); + SValBuilder &Builder = Eng.getSValBuilder(); + MemRegionManager &MRManager = SManager.getRegionManager(); + const ASTContext &ASTCtxt = Eng.getContext(); const auto *VDX0 = findDeclByName(D, "x0"); const auto *VDY0 = findDeclByName(D, "y0"); const auto *VDZ0 = findDeclByName(D, "z0"); const auto *VDX1 = findDeclByName(D, "x1"); const auto *VDY1 = findDeclByName(D, "y1"); - assert(VDX0 && VDY0 && VDZ0 && VDX1 && VDY1); + + ASSERT_TRUE(VDX0 && VDY0 && VDZ0 && VDX1 && VDY1); const StackFrameContext *SFC = Eng.getAnalysisDeclContextManager().getStackFrame(D); - Loc LX0 = loc::MemRegionVal(MRMgr.getVarRegion(VDX0, SFC)); - Loc LY0 = loc::MemRegionVal(MRMgr.getVarRegion(VDY0, SFC)); - Loc LZ0 = loc::MemRegionVal(MRMgr.getVarRegion(VDZ0, SFC)); - Loc LX1 = loc::MemRegionVal(MRMgr.getVarRegion(VDX1, SFC)); - Loc LY1 = loc::MemRegionVal(MRMgr.getVarRegion(VDY1, SFC)); + Loc LX0 = loc::MemRegionVal(MRManager.getVarRegion(VDX0, SFC)); + Loc LY0 = loc::MemRegionVal(MRManager.getVarRegion(VDY0, SFC)); + Loc LZ0 = loc::MemRegionVal(MRManager.getVarRegion(VDZ0, SFC)); + Loc LX1 = loc::MemRegionVal(MRManager.getVarRegion(VDX1, SFC)); + Loc LY1 = loc::MemRegionVal(MRManager.getVarRegion(VDY1, SFC)); - Store StInit = StMgr.getInitialStore(SFC).getStore(); - SVal Zero = SVB.makeZeroVal(ACtx.IntTy); - SVal One = SVB.makeIntVal(1, ACtx.IntTy); - SVal NarrowZero = SVB.makeZeroVal(ACtx.CharTy); + Store StInit = SManager.getInitialStore(SFC).getStore(); + SVal Zero = Builder.makeZeroVal(ASTCtxt.IntTy); + SVal One = Builder.makeIntVal(1, ASTCtxt.IntTy); + SVal NarrowZero = Builder.makeZeroVal(ASTCtxt.CharTy); // Bind(Zero) - Store StX0 = - StMgr.Bind(StInit, LX0, Zero).getStore(); - ASSERT_EQ(Zero, StMgr.getBinding(StX0, LX0, ACtx.IntTy)); + Store StX0 = SManager.Bind(StInit, LX0, Zero).getStore(); + EXPECT_EQ(Zero, SManager.getBinding(StX0, LX0, ASTCtxt.IntTy)); // BindDefaultInitial(Zero) Store StY0 = - StMgr.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore(); - ASSERT_EQ(Zero, StMgr.getBinding(StY0, LY0, ACtx.IntTy)); - ASSERT_EQ(Zero, *StMgr.getDefaultBinding(StY0, LY0.getAsRegion())); + SManager.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore(); + EXPECT_EQ(Zero, SManager.getBinding(StY0, LY0, ASTCtxt.IntTy)); + EXPECT_EQ(Zero, *SManager.getDefaultBinding(StY0, LY0.getAsRegion())); // BindDefaultZero() - Store StZ0 = - StMgr.BindDefaultZero(StInit, LZ0.getAsRegion()).getStore(); + Store StZ0 = SManager.BindDefaultZero(StInit, LZ0.getAsRegion()).getStore(); // BindDefaultZero wipes the region with '0 S8b', not with out Zero. // Direct load, however, does give us back the object of the type // that we specify for loading. - ASSERT_EQ(Zero, StMgr.getBinding(StZ0, LZ0, ACtx.IntTy)); - ASSERT_EQ(NarrowZero, *StMgr.getDefaultBinding(StZ0, LZ0.getAsRegion())); + EXPECT_EQ(Zero, SManager.getBinding(StZ0, LZ0, ASTCtxt.IntTy)); + EXPECT_EQ(NarrowZero, *SManager.getDefaultBinding(StZ0, LZ0.getAsRegion())); // Bind(One) - Store StX1 = - StMgr.Bind(StInit, LX1, One).getStore(); - ASSERT_EQ(One, StMgr.getBinding(StX1, LX1, ACtx.IntTy)); + Store StX1 = SManager.Bind(StInit, LX1, One).getStore(); + EXPECT_EQ(One, SManager.getBinding(StX1, LX1, ASTCtxt.IntTy)); // BindDefaultInitial(One) Store StY1 = - StMgr.BindDefaultInitial(StInit, LY1.getAsRegion(), One).getStore(); - ASSERT_EQ(One, StMgr.getBinding(StY1, LY1, ACtx.IntTy)); - ASSERT_EQ(One, *StMgr.getDefaultBinding(StY1, LY1.getAsRegion())); + SManager.BindDefaultInitial(StInit, LY1.getAsRegion(), One).getStore(); + EXPECT_EQ(One, SManager.getBinding(StY1, LY1, ASTCtxt.IntTy)); + EXPECT_EQ(One, *SManager.getDefaultBinding(StY1, LY1.getAsRegion())); } public: - VariableBindConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {} + using StoreTestConsumer::StoreTestConsumer; +}; - bool HandleTopLevelDecl(DeclGroupRef DG) override { - for (const auto *D : DG) - performTest(D); - return true; +TEST(Store, VariableBind) { + EXPECT_TRUE(tooling::runToolOnCode( + std::make_unique>(), + "void foo() { int x0, y0, z0, x1, y1; }")); +} + +class LiteralCompoundConsumer : public StoreTestConsumer { + void performTest(const Decl *D) override { + StoreManager &SManager = Eng.getStoreManager(); + SValBuilder &Builder = Eng.getSValBuilder(); + MemRegionManager &MRManager = SManager.getRegionManager(); + ASTContext &ASTCtxt = Eng.getContext(); + + using namespace ast_matchers; + + const auto *CL = findNode(D, compoundLiteralExpr()); + + const StackFrameContext *SFC = + Eng.getAnalysisDeclContextManager().getStackFrame(D); + + QualType Int = ASTCtxt.IntTy; + + // Get region for 'test' + const SubRegion *CLRegion = MRManager.getCompoundLiteralRegion(CL, SFC); + + // Get value for 'test[0]' + NonLoc Zero = Builder.makeIntVal(0, false); + loc::MemRegionVal ZeroElement( + MRManager.getElementRegion(ASTCtxt.IntTy, Zero, CLRegion, ASTCtxt)); + + Store StInit = SManager.getInitialStore(SFC).getStore(); + // Let's bind constant 1 to 'test[0]' + SVal One = Builder.makeIntVal(1, Int); + Store StX = SManager.Bind(StInit, ZeroElement, One).getStore(); + + // And make sure that we can read this binding back as it was + EXPECT_EQ(One, SManager.getBinding(StX, ZeroElement, Int)); } -}; -class VariableBindAction : public ASTFrontendAction { public: - std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler, - StringRef File) override { - return std::make_unique(Compiler); - } + using StoreTestConsumer::StoreTestConsumer; }; -TEST(Store, VariableBind) { - EXPECT_TRUE(tooling::runToolOnCode(std::make_unique(), - "void foo() { int x0, y0, z0, x1, y1; }")); +TEST(Store, LiteralCompound) { + EXPECT_TRUE(tooling::runToolOnCode( + std::make_unique>(), + "void foo() { int *test = (int[]){ 1, 2, 3 }; }", "input.c")); } } // namespace -- 2.7.4