NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho,
baloghadamsoftware.
Commit https://reviews.llvm.org/D51191 causes a crash when a pointer to a
completely unrelated type `UnrelatedT` (eg., opaque struct pattern) is being
casted from base class `BaseT` to derived class `DerivedT`, which results in an
ill-formed region `Derived{SymRegion{$<UnrelatedT x>}, DerivedT}`.
I guess we should prevent these from appearing somehow.
Repository:
rC Clang
https://reviews.llvm.org/D52189
Files:
lib/StaticAnalyzer/Core/Store.cpp
test/Analysis/casts.cpp
Index: test/Analysis/casts.cpp
===================================================================
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -70,5 +70,30 @@
clang_analyzer_eval(c->x); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c->y); // expected-warning{{TRUE}}
}
+} // namespace base_to_derived_double_inheritance
+
+namespace base_to_derived_opaque_class {
+class NotInt {
+public:
+ operator int() { return !x; } // no-crash
+ int x;
+};
+
+typedef struct Opaque *OpaqueRef;
+
+class Transparent {
+public:
+ int getNotInt() { return NI; }
+ NotInt NI;
+};
+
+class SubTransparent : public Transparent {};
+
+SubTransparent *castToDerived(Transparent *TRef) {
+ return (SubTransparent *)TRef;
}
+void counterValue(OpaqueRef ORef) {
+ castToDerived(reinterpret_cast<Transparent *>(ORef))->getNotInt();
+}
+} // namespace base_to_derived_opaque_class
Index: lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -375,8 +375,17 @@
MR = Uncasted;
}
+ // If we're casting a symbolic base pointer to a derived class, use
+ // CXXDerivedObjectRegion to represent the cast. If it's a pointer to an
+ // unrelated type, it must be a weird reinterpret_cast and we have to
+ // be fine with ElementRegion.
if (const auto *SR = dyn_cast<SymbolicRegion>(MR)) {
- return loc::MemRegionVal(MRMgr.getCXXDerivedObjectRegion(TargetClass, SR));
+ QualType T = SR->getSymbol()->getType();
+ const CXXRecordDecl *SourceClass = T->getPointeeCXXRecordDecl();
+ if (TargetClass->isDerivedFrom(SourceClass))
+ return loc::MemRegionVal(
+ MRMgr.getCXXDerivedObjectRegion(TargetClass, SR));
+ return loc::MemRegionVal(GetElementZeroRegion(SR, TargetType));
}
// We failed if the region we ended up with has perfect type info.
Index: test/Analysis/casts.cpp
===================================================================
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -70,5 +70,30 @@
clang_analyzer_eval(c->x); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(c->y); // expected-warning{{TRUE}}
}
+} // namespace base_to_derived_double_inheritance
+
+namespace base_to_derived_opaque_class {
+class NotInt {
+public:
+ operator int() { return !x; } // no-crash
+ int x;
+};
+
+typedef struct Opaque *OpaqueRef;
+
+class Transparent {
+public:
+ int getNotInt() { return NI; }
+ NotInt NI;
+};
+
+class SubTransparent : public Transparent {};
+
+SubTransparent *castToDerived(Transparent *TRef) {
+ return (SubTransparent *)TRef;
}
+void counterValue(OpaqueRef ORef) {
+ castToDerived(reinterpret_cast<Transparent *>(ORef))->getNotInt();
+}
+} // namespace base_to_derived_opaque_class
Index: lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -375,8 +375,17 @@
MR = Uncasted;
}
+ // If we're casting a symbolic base pointer to a derived class, use
+ // CXXDerivedObjectRegion to represent the cast. If it's a pointer to an
+ // unrelated type, it must be a weird reinterpret_cast and we have to
+ // be fine with ElementRegion.
if (const auto *SR = dyn_cast<SymbolicRegion>(MR)) {
- return loc::MemRegionVal(MRMgr.getCXXDerivedObjectRegion(TargetClass, SR));
+ QualType T = SR->getSymbol()->getType();
+ const CXXRecordDecl *SourceClass = T->getPointeeCXXRecordDecl();
+ if (TargetClass->isDerivedFrom(SourceClass))
+ return loc::MemRegionVal(
+ MRMgr.getCXXDerivedObjectRegion(TargetClass, SR));
+ return loc::MemRegionVal(GetElementZeroRegion(SR, TargetType));
}
// We failed if the region we ended up with has perfect type info.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
