r180052 - [analyzer] Treat reinterpret_cast like a base cast in certain cases.
Jordan Rose
jordan_rose at apple.com
Mon Apr 22 14:36:50 PDT 2013
Author: jrose
Date: Mon Apr 22 16:36:49 2013
New Revision: 180052
URL: http://llvm.org/viewvc/llvm-project?rev=180052&view=rev
Log:
[analyzer] Treat reinterpret_cast like a base cast in certain cases.
The analyzer represents all pointer-to-pointer bitcasts the same way, but
this can be problematic if an implicit base cast gets layered on top of a
manual base cast (performed with reinterpret_cast instead of static_cast).
Fix this (and avoid a valid assertion) by looking through cast regions.
Using reinterpret_cast this way is only valid if the base class is at the
same offset as the derived class; this is checked by -Wreinterpret-base-class.
In the interest of performance, the analyzer doesn't repeat this check
anywhere; it will just silently do the wrong thing (use the wrong offsets
for fields of the base class) if the user code is wrong.
PR15394
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
cfe/trunk/test/Analysis/derived-to-base.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=180052&r1=180051&r2=180052&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Mon Apr 22 16:36:49 2013
@@ -289,62 +289,82 @@ SVal StoreManager::evalDerivedToBase(SVa
return loc::MemRegionVal(BaseReg);
}
-SVal StoreManager::evalDynamicCast(SVal Base, QualType DerivedType,
+/// Returns the static type of the given region, if it represents a C++ class
+/// object.
+///
+/// This handles both fully-typed regions, where the dynamic type is known, and
+/// symbolic regions, where the dynamic type is merely bounded (and even then,
+/// only ostensibly!), but does not take advantage of any dynamic type info.
+static const CXXRecordDecl *getCXXRecordType(const MemRegion *MR) {
+ if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(MR))
+ return TVR->getValueType()->getAsCXXRecordDecl();
+ if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR))
+ return SR->getSymbol()->getType()->getPointeeCXXRecordDecl();
+ return 0;
+}
+
+SVal StoreManager::evalDynamicCast(SVal Base, QualType TargetType,
bool &Failed) {
Failed = false;
- Optional<loc::MemRegionVal> BaseRegVal = Base.getAs<loc::MemRegionVal>();
- if (!BaseRegVal)
+ const MemRegion *MR = Base.getAsRegion();
+ if (!MR)
return UnknownVal();
- const MemRegion *BaseRegion = BaseRegVal->stripCasts(/*StripBases=*/false);
// Assume the derived class is a pointer or a reference to a CXX record.
- DerivedType = DerivedType->getPointeeType();
- assert(!DerivedType.isNull());
- const CXXRecordDecl *DerivedDecl = DerivedType->getAsCXXRecordDecl();
- if (!DerivedDecl && !DerivedType->isVoidType())
+ TargetType = TargetType->getPointeeType();
+ assert(!TargetType.isNull());
+ const CXXRecordDecl *TargetClass = TargetType->getAsCXXRecordDecl();
+ if (!TargetClass && !TargetType->isVoidType())
return UnknownVal();
// Drill down the CXXBaseObject chains, which represent upcasts (casts from
// derived to base).
- const MemRegion *SR = BaseRegion;
- while (const TypedRegion *TSR = dyn_cast_or_null<TypedRegion>(SR)) {
- QualType BaseType = TSR->getLocationType()->getPointeeType();
- assert(!BaseType.isNull());
- const CXXRecordDecl *SRDecl = BaseType->getAsCXXRecordDecl();
- if (!SRDecl)
- return UnknownVal();
-
+ while (const CXXRecordDecl *MRClass = getCXXRecordType(MR)) {
// If found the derived class, the cast succeeds.
- if (SRDecl == DerivedDecl)
- return loc::MemRegionVal(TSR);
+ if (MRClass == TargetClass)
+ return loc::MemRegionVal(MR);
- if (!DerivedType->isVoidType()) {
+ if (!TargetType->isVoidType()) {
// Static upcasts are marked as DerivedToBase casts by Sema, so this will
// only happen when multiple or virtual inheritance is involved.
CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true,
/*DetectVirtual=*/false);
- if (SRDecl->isDerivedFrom(DerivedDecl, Paths))
- return evalDerivedToBase(loc::MemRegionVal(TSR), Paths.front());
+ if (MRClass->isDerivedFrom(TargetClass, Paths))
+ return evalDerivedToBase(loc::MemRegionVal(MR), Paths.front());
}
- if (const CXXBaseObjectRegion *R = dyn_cast<CXXBaseObjectRegion>(TSR))
+ if (const CXXBaseObjectRegion *BaseR = dyn_cast<CXXBaseObjectRegion>(MR)) {
// Drill down the chain to get the derived classes.
- SR = R->getSuperRegion();
- else {
- // We reached the bottom of the hierarchy.
-
- // If this is a cast to void*, return the region.
- if (DerivedType->isVoidType())
- return loc::MemRegionVal(TSR);
-
- // We did not find the derived class. We we must be casting the base to
- // derived, so the cast should fail.
- Failed = true;
- return UnknownVal();
+ MR = BaseR->getSuperRegion();
+ continue;
}
+
+ // If this is a cast to void*, return the region.
+ if (TargetType->isVoidType())
+ return loc::MemRegionVal(MR);
+
+ // Strange use of reinterpret_cast can give us paths we don't reason
+ // about well, by putting in ElementRegions where we'd expect
+ // CXXBaseObjectRegions. If it's a valid reinterpret_cast (i.e. if the
+ // derived class has a zero offset from the base class), then it's safe
+ // to strip the cast; if it's invalid, -Wreinterpret-base-class should
+ // catch it. In the interest of performance, the analyzer will silently
+ // do the wrong thing in the invalid case (because offsets for subregions
+ // will be wrong).
+ const MemRegion *Uncasted = MR->StripCasts(/*IncludeBaseCasts=*/false);
+ if (Uncasted == MR) {
+ // We reached the bottom of the hierarchy and did not find the derived
+ // class. We we must be casting the base to derived, so the cast should
+ // fail.
+ break;
+ }
+
+ MR = Uncasted;
}
-
+
+ // We failed if the region we ended up with has perfect type info.
+ Failed = isa<TypedValueRegion>(MR);
return UnknownVal();
}
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=180052&r1=180051&r2=180052&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/derived-to-base.cpp (original)
+++ cfe/trunk/test/Analysis/derived-to-base.cpp Mon Apr 22 16:36:49 2013
@@ -2,6 +2,7 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -DCONSTRUCTORS=1 -analyzer-config c++-inlining=constructors -verify %s
void clang_analyzer_eval(bool);
+void clang_analyzer_checkInlined(bool);
class A {
protected:
@@ -363,3 +364,89 @@ namespace Redeclaration {
}
};
+namespace PR15394 {
+ namespace Original {
+ class Base {
+ public:
+ virtual int f() = 0;
+ int i;
+ };
+
+ class Derived1 : public Base {
+ public:
+ int j;
+ };
+
+ class Derived2 : public Derived1 {
+ public:
+ virtual int f() {
+ clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+ return i + j;
+ }
+ };
+
+ void testXXX() {
+ Derived1 *d1p = reinterpret_cast<Derived1*>(new Derived2);
+ d1p->i = 1;
+ d1p->j = 2;
+ clang_analyzer_eval(d1p->f() == 3); // expected-warning{{TRUE}}
+ }
+ }
+
+ namespace VirtualInDerived {
+ class Base {
+ public:
+ int i;
+ };
+
+ class Derived1 : public Base {
+ public:
+ virtual int f() = 0;
+ int j;
+ };
+
+ class Derived2 : public Derived1 {
+ public:
+ virtual int f() {
+ clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+ return i + j;
+ }
+ };
+
+ void test() {
+ Derived1 *d1p = reinterpret_cast<Derived1*>(new Derived2);
+ d1p->i = 1;
+ d1p->j = 2;
+ clang_analyzer_eval(d1p->f() == 3); // expected-warning{{TRUE}}
+ }
+ }
+
+ namespace NoCast {
+ class Base {
+ public:
+ int i;
+ };
+
+ class Derived1 : public Base {
+ public:
+ virtual int f() = 0;
+ int j;
+ };
+
+ class Derived2 : public Derived1 {
+ public:
+ virtual int f() {
+ clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+ return i + j;
+ }
+ };
+
+ void test() {
+ Derived1 *d1p = new Derived2;
+ d1p->i = 1;
+ d1p->j = 2;
+ clang_analyzer_eval(d1p->f() == 3); // expected-warning{{TRUE}}
+ }
+ }
+};
+
More information about the cfe-commits
mailing list