[cfe-commits] r161797 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/MemRegion.cpp lib/StaticAnalyzer/Core/RegionStore.cpp lib/StaticAnalyzer/Core/SVals.cpp test/Analysis/derived-to-base.cpp test/Analysis/dynamic-cast.cpp
Jordan Rose
jordan_rose at apple.com
Mon Aug 13 15:11:34 PDT 2012
Author: jrose
Date: Mon Aug 13 17:11:34 2012
New Revision: 161797
URL: http://llvm.org/viewvc/llvm-project?rev=161797&view=rev
Log:
[analyzer] Don't strip CXXBaseObjectRegions when checking dynamic_casts.
...and /do/ strip CXXBaseObjectRegions when casting to a virtual base class.
This allows us to enforce the invariant that a CXXBaseObjectRegion can always
provide an offset for its base region if its base region has a known class
type, by only allowing virtual bases and direct non-virtual bases to form
CXXBaseObjectRegions.
This does mean some slight problems for our modeling of dynamic_cast, which
needs to be resolved by finding a path from the current region to the class
we're trying to cast to.
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
cfe/trunk/test/Analysis/derived-to-base.cpp
cfe/trunk/test/Analysis/dynamic-cast.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Mon Aug 13 17:11:34 2012
@@ -138,7 +138,7 @@
const MemRegion *getBaseRegion() const;
- const MemRegion *StripCasts() const;
+ const MemRegion *StripCasts(bool StripBaseCasts = true) const;
bool hasGlobalsOrParametersStorage() const;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Mon Aug 13 17:11:34 2012
@@ -433,7 +433,7 @@
}
/// \brief Get the underlining region and strip casts.
- const MemRegion* stripCasts() const;
+ const MemRegion* stripCasts(bool StripBaseCasts = true) const;
template <typename REGION>
const REGION* getRegionAs() const {
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Mon Aug 13 17:11:34 2012
@@ -15,7 +15,6 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/Analysis/ProgramPoint.h"
-#include "clang/AST/CXXInheritance.h"
#include "clang/AST/ParentMap.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringExtras.h"
Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Mon Aug 13 17:11:34 2012
@@ -888,6 +888,37 @@
const CXXBaseObjectRegion *
MemRegionManager::getCXXBaseObjectRegion(const CXXRecordDecl *decl,
const MemRegion *superRegion) {
+ // Check that the base class is actually a direct base of this region.
+ if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(superRegion)) {
+ if (const CXXRecordDecl *Class = TVR->getValueType()->getAsCXXRecordDecl()){
+ if (Class->isVirtuallyDerivedFrom(decl)) {
+ // Virtual base regions should not be layered, since the layout rules
+ // are different.
+ while (const CXXBaseObjectRegion *Base =
+ dyn_cast<CXXBaseObjectRegion>(superRegion)) {
+ superRegion = Base->getSuperRegion();
+ }
+ assert(superRegion && !isa<MemSpaceRegion>(superRegion));
+
+ } else {
+ // Non-virtual bases should always be direct bases.
+#ifndef NDEBUG
+ bool FoundBase = false;
+ for (CXXRecordDecl::base_class_const_iterator I = Class->bases_begin(),
+ E = Class->bases_end();
+ I != E; ++I) {
+ if (I->getType()->getAsCXXRecordDecl() == decl) {
+ FoundBase = true;
+ break;
+ }
+ }
+
+ assert(FoundBase && "Not a direct base class of this region");
+#endif
+ }
+ }
+ }
+
return getSubRegion<CXXBaseObjectRegion>(decl, superRegion);
}
@@ -963,7 +994,7 @@
// View handling.
//===----------------------------------------------------------------------===//
-const MemRegion *MemRegion::StripCasts() const {
+const MemRegion *MemRegion::StripCasts(bool StripBaseCasts) const {
const MemRegion *R = this;
while (true) {
switch (R->getKind()) {
@@ -975,6 +1006,8 @@
break;
}
case CXXBaseObjectRegionKind:
+ if (!StripBaseCasts)
+ return R;
R = cast<CXXBaseObjectRegion>(R)->getSuperRegion();
break;
default:
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Mon Aug 13 17:11:34 2012
@@ -934,7 +934,7 @@
loc::MemRegionVal *baseRegVal = dyn_cast<loc::MemRegionVal>(&base);
if (!baseRegVal)
return UnknownVal();
- const MemRegion *BaseRegion = baseRegVal->stripCasts();
+ const MemRegion *BaseRegion = baseRegVal->stripCasts(/*StripBases=*/false);
// Assume the derived class is a pointer or a reference to a CXX record.
derivedType = derivedType->getPointeeType();
@@ -957,23 +957,23 @@
if (SRDecl == DerivedDecl)
return loc::MemRegionVal(TSR);
- // If the region type is a subclass of the derived type.
- if (!derivedType->isVoidType() && SRDecl->isDerivedFrom(DerivedDecl)) {
- // This occurs in two cases.
- // 1) We are processing an upcast.
- // 2) We are processing a downcast but we jumped directly from the
- // ancestor to a child of the cast value, so conjure the
- // appropriate region to represent value (the intermediate node).
- return loc::MemRegionVal(MRMgr.getCXXBaseObjectRegion(DerivedDecl,
- BaseRegion));
- }
+ if (!derivedType->isVoidType()) {
+ // Static upcasts are marked as DerivedToBase casts by Sema, so this will
+ // only happen when multiple or virtual inheritance is involved.
+ // FIXME: We should build the correct stack of CXXBaseObjectRegions here,
+ // instead of just punting.
+ if (SRDecl->isDerivedFrom(DerivedDecl))
+ return UnknownVal();
- // If super region is not a parent of derived class, the cast definitely
- // fails.
- if (!derivedType->isVoidType() &&
- DerivedDecl->isProvablyNotDerivedFrom(SRDecl)) {
- Failed = true;
- return UnknownVal();
+ // If super region is not a parent of derived class, the cast definitely
+ // fails.
+ // FIXME: This and the above test each require walking the entire
+ // inheritance hierarchy, and this will happen for each
+ // CXXBaseObjectRegion wrapper. We should probably be combining the two.
+ if (DerivedDecl->isProvablyNotDerivedFrom(SRDecl)) {
+ Failed = true;
+ return UnknownVal();
+ }
}
if (const CXXBaseObjectRegion *R = dyn_cast<CXXBaseObjectRegion>(TSR))
Modified: cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp Mon Aug 13 17:11:34 2012
@@ -133,9 +133,9 @@
return 0;
}
-const MemRegion *loc::MemRegionVal::stripCasts() const {
+const MemRegion *loc::MemRegionVal::stripCasts(bool StripBaseCasts) const {
const MemRegion *R = getRegion();
- return R ? R->StripCasts() : NULL;
+ return R ? R->StripCasts(StripBaseCasts) : NULL;
}
const void *nonloc::LazyCompoundVal::getStore() const {
Modified: cfe/trunk/test/Analysis/derived-to-base.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/derived-to-base.cpp?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/derived-to-base.cpp (original)
+++ cfe/trunk/test/Analysis/derived-to-base.cpp Mon Aug 13 17:11:34 2012
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store region %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_eval(bool);
class A {
protected:
@@ -22,3 +24,64 @@
x = 5;
}
};
+
+
+namespace VirtualBaseClasses {
+ class A {
+ protected:
+ int x;
+ };
+
+ class B : public virtual A {
+ public:
+ int getX() { return x; }
+ };
+
+ class C : public virtual A {
+ public:
+ void setX() { x = 42; }
+ };
+
+ class D : public B, public C {};
+ class DV : virtual public B, public C {};
+ class DV2 : public B, virtual public C {};
+
+ void test() {
+ D d;
+ d.setX();
+ clang_analyzer_eval(d.getX() == 42); // expected-warning{{TRUE}}
+
+ DV dv;
+ dv.setX();
+ clang_analyzer_eval(dv.getX() == 42); // expected-warning{{TRUE}}
+
+ DV2 dv2;
+ dv2.setX();
+ clang_analyzer_eval(dv2.getX() == 42); // expected-warning{{TRUE}}
+ }
+
+
+ // Make sure we're consistent about the offset of the A subobject within an
+ // Intermediate virtual base class.
+ class Padding1 { int unused; };
+ class Padding2 { int unused; };
+ class Intermediate : public Padding1, public A, public Padding2 {};
+
+ class BI : public virtual Intermediate {
+ public:
+ int getX() { return x; }
+ };
+
+ class CI : public virtual Intermediate {
+ public:
+ void setX() { x = 42; }
+ };
+
+ class DI : public BI, public CI {};
+
+ void testIntermediate() {
+ DI d;
+ d.setX();
+ clang_analyzer_eval(d.getX() == 42); // expected-warning{{TRUE}}
+ }
+}
Modified: cfe/trunk/test/Analysis/dynamic-cast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dynamic-cast.cpp?rev=161797&r1=161796&r2=161797&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dynamic-cast.cpp (original)
+++ cfe/trunk/test/Analysis/dynamic-cast.cpp Mon Aug 13 17:11:34 2012
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core -analyzer-ipa=none -verify %s
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=none -verify %s
+
+void clang_analyzer_eval(bool);
class A {
public:
@@ -208,7 +210,25 @@
testDynCastMostLikelyWillFail(&m);
}
+
+void testDynCastToMiddleClass () {
+ class BBB : public BB {};
+ BBB obj;
+ A &ref = obj;
+
+ // These didn't always correctly layer base regions.
+ B *ptr = dynamic_cast<B*>(&ref);
+ clang_analyzer_eval(ptr != 0); // expected-warning{{TRUE}}
+
+ // This is actually statically resolved to be a DerivedToBase cast.
+ ptr = dynamic_cast<B*>(&obj);
+ clang_analyzer_eval(ptr != 0); // expected-warning{{TRUE}}
+}
+
+
+// -----------------------------
// False positives/negatives.
+// -----------------------------
// Due to symbolic regions not being typed.
int testDynCastFalsePositive(BB *c) {
More information about the cfe-commits
mailing list