From 111aa9a28bd89fe0209157c1537717aa244c6a38 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 19 Feb 2013 20:28:33 +0000 Subject: [PATCH] [analyzer] Don't accidentally strip off base object regions for lazy bindings. If a base object is at a 0 offset, RegionStoreManager may find a lazy binding for the entire object, then try to attach a FieldRegion or grandparent CXXBaseObjectRegion on top of that (skipping the intermediate region). We now preserve as many layers of base object regions necessary to make the types match. llvm-svn: 175556 --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 64 ++++++---- clang/test/Analysis/derived-to-base.cpp | 169 ++++++++++++++++++++++++++ 2 files changed, 212 insertions(+), 21 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 6559e11..f75ab02 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -497,8 +497,11 @@ public: // Part of public interface to class. const TypedValueRegion *R, QualType Ty); - /// Get the state and region whose binding this region R corresponds to. - std::pair + /// Get the state and region whose binding this region \p R corresponds to. + /// + /// If there is no lazy binding for \p R, the returned value will have a null + /// \c second. Note that a null pointer can represents a valid Store. + std::pair getLazyBinding(RegionBindingsConstRef B, const MemRegion *R, const MemRegion *originalRegion); @@ -1256,49 +1259,68 @@ std::pair RegionStoreManager::getLazyBinding(RegionBindingsConstRef B, const MemRegion *R, const MemRegion *originalRegion) { + typedef std::pair StoreRegionPair; + if (originalRegion != R) { if (Optional OV = B.getDefaultBinding(R)) { if (const nonloc::LazyCompoundVal *V = - dyn_cast(OV.getPointer())) + dyn_cast(OV.getPointer())) return std::make_pair(V->getStore(), V->getRegion()); } } if (const ElementRegion *ER = dyn_cast(R)) { - const std::pair &X = - getLazyBinding(B, ER->getSuperRegion(), originalRegion); + StoreRegionPair X = getLazyBinding(B, ER->getSuperRegion(), originalRegion); if (X.second) return std::make_pair(X.first, MRMgr.getElementRegionWithSuper(ER, X.second)); } else if (const FieldRegion *FR = dyn_cast(R)) { - const std::pair &X = - getLazyBinding(B, FR->getSuperRegion(), originalRegion); + StoreRegionPair X = getLazyBinding(B, FR->getSuperRegion(), originalRegion); if (X.second) { return std::make_pair(X.first, MRMgr.getFieldRegionWithSuper(FR, X.second)); } - } - // C++ base object region is another kind of region that we should blast - // through to look for lazy compound value. It is like a field region. - else if (const CXXBaseObjectRegion *baseReg = - dyn_cast(R)) { - const std::pair &X = - getLazyBinding(B, baseReg->getSuperRegion(), originalRegion); + } else if (const CXXBaseObjectRegion *BaseReg = + dyn_cast(R)) { + // C++ base object region is another kind of region that we should blast + // through to look for lazy compound value. It is like a field region. + StoreRegionPair Result = getLazyBinding(B, BaseReg->getSuperRegion(), + originalRegion); - if (X.second) { - return std::make_pair(X.first, - MRMgr.getCXXBaseObjectRegionWithSuper(baseReg, - X.second)); + if (Result.second) { + // Make sure the types match up. + const CXXRecordDecl *LazyD = 0; + if (const TypedValueRegion *LazyR = + dyn_cast(Result.second)) { + LazyD = LazyR->getValueType()->getAsCXXRecordDecl(); + if (LazyD) + LazyD = LazyD->getCanonicalDecl(); + } + + typedef SmallVector BaseListTy; + BaseListTy BaseRegions; + do { + BaseRegions.push_back(BaseReg); + BaseReg = dyn_cast(BaseReg->getSuperRegion()); + } while (BaseReg && LazyD != BaseReg->getDecl()->getCanonicalDecl()); + + // Layer the base regions on the result in least-to-most derived order. + for (BaseListTy::const_reverse_iterator I = BaseRegions.rbegin(), + E = BaseRegions.rend(); + I != E; ++I) { + Result.second = MRMgr.getCXXBaseObjectRegionWithSuper(*I, + Result.second); + } } + + return Result; } - // The NULL MemRegion indicates an non-existent lazy binding. A NULL Store is - // possible for a valid lazy binding. - return std::make_pair((Store) 0, (const MemRegion *) 0); + return StoreRegionPair(); } SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B, diff --git a/clang/test/Analysis/derived-to-base.cpp b/clang/test/Analysis/derived-to-base.cpp index 57689d1..80dbc17 100644 --- a/clang/test/Analysis/derived-to-base.cpp +++ b/clang/test/Analysis/derived-to-base.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -DCONSTRUCTORS=1 -analyzer-config c++-inlining=constructors -verify %s void clang_analyzer_eval(bool); @@ -135,3 +136,171 @@ namespace DynamicMultipleInheritanceUpcast { clang_analyzer_eval(testCast(&d)); // expected-warning{{TRUE}} } } + +namespace LazyBindings { + struct Base { + int x; + }; + + struct Derived : public Base { + int y; + }; + + struct DoubleDerived : public Derived { + int z; + }; + + int getX(const Base &obj) { + return obj.x; + } + + int getY(const Derived &obj) { + return obj.y; + } + + void testDerived() { + Derived d; + d.x = 1; + d.y = 2; + clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}} + + Base b(d); + clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}} + + Derived d2(d); + clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}} + } + + void testDoubleDerived() { + DoubleDerived d; + d.x = 1; + d.y = 2; + clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}} + + Base b(d); + clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}} + + Derived d2(d); + clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}} + + DoubleDerived d3(d); + clang_analyzer_eval(getX(d3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d3) == 2); // expected-warning{{TRUE}} + } + + namespace WithOffset { + struct Offset { + int padding; + }; + + struct OffsetDerived : private Offset, public Base { + int y; + }; + + struct DoubleOffsetDerived : public OffsetDerived { + int z; + }; + + int getY(const OffsetDerived &obj) { + return obj.y; + } + + void testDerived() { + OffsetDerived d; + d.x = 1; + d.y = 2; + clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}} + + Base b(d); + clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}} + + OffsetDerived d2(d); + clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}} + } + + void testDoubleDerived() { + DoubleOffsetDerived d; + d.x = 1; + d.y = 2; + clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}} + + Base b(d); + clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}} + + OffsetDerived d2(d); + clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}} + + DoubleOffsetDerived d3(d); + clang_analyzer_eval(getX(d3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d3) == 2); // expected-warning{{TRUE}} + } + } + + namespace WithVTable { + struct DerivedVTBL : public Base { + int y; + virtual void method(); + }; + + struct DoubleDerivedVTBL : public DerivedVTBL { + int z; + }; + + int getY(const DerivedVTBL &obj) { + return obj.y; + } + + int getZ(const DoubleDerivedVTBL &obj) { + return obj.z; + } + + void testDerived() { + DerivedVTBL d; + d.x = 1; + d.y = 2; + clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}} + + Base b(d); + clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}} + +#if CONSTRUCTORS + DerivedVTBL d2(d); + clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}} +#endif + } + +#if CONSTRUCTORS + void testDoubleDerived() { + DoubleDerivedVTBL d; + d.x = 1; + d.y = 2; + d.z = 3; + clang_analyzer_eval(getX(d) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d) == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(getZ(d) == 3); // expected-warning{{TRUE}} + + Base b(d); + clang_analyzer_eval(getX(b) == 1); // expected-warning{{TRUE}} + + DerivedVTBL d2(d); + clang_analyzer_eval(getX(d2) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d2) == 2); // expected-warning{{TRUE}} + + DoubleDerivedVTBL d3(d); + clang_analyzer_eval(getX(d3) == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(getY(d3) == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(getZ(d3) == 3); // expected-warning{{TRUE}} + } +#endif + } +} -- 2.7.4