NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added a subscriber: rnkovacs.
When modeling implicit copy/move-constructor or copy/move-assignment operator
of an empty class, don't do anything. The previous behavior was to take the
value of the empty source object (which is always `UnknownVal` for empty
classes) and assign it to the empty target object. This causes problems when
the target object is a field or a base class because, due to how `RegionStore`
forgets binding sizes, such `UnknownVal` would overwrite any existing store
binding at the respective offset.
While testing temporary constructors and destructors, this resulted in numerous
leak false positives when the raw pointer value in `unique_ptr` started
disappearing from the program state when the zero-size `deleter` part of the
smart pointer was trivially-copied at its offset. Note that
`performTrivialCopy` doesn't cause pointer escape, because it normally doesn't
need to.
https://reviews.llvm.org/D43714
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/ctor.mm
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -729,3 +729,23 @@
S s;
}
}
+
+namespace EmptyBaseAssign {
+struct B1 {};
+struct B2 { int x; };
+struct D: public B1, public B2 {
+const D &operator=(const D &d) {
+ *((B2 *)this) = d;
+ *((B1 *)this) = d;
+ return *this;
+}
+};
+
+void test() {
+ D d1;
+ d1.x = 1;
+ D d2;
+ d2 = d1;
+ clang_analyzer_eval(d2.x == 1); // expected-warning{{TRUE}}
+}
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -42,19 +42,30 @@
const CallEvent &Call) {
SVal ThisVal;
bool AlwaysReturnsLValue;
+ const CXXRecordDecl *ThisRD = nullptr;
if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
assert(Ctor->getDecl()->isTrivial());
assert(Ctor->getDecl()->isCopyOrMoveConstructor());
ThisVal = Ctor->getCXXThisVal();
+ ThisRD = Ctor->getDecl()->getParent();
AlwaysReturnsLValue = false;
} else {
assert(cast<CXXMethodDecl>(Call.getDecl())->isTrivial());
assert(cast<CXXMethodDecl>(Call.getDecl())->getOverloadedOperator() ==
OO_Equal);
ThisVal = cast<CXXInstanceCall>(Call).getCXXThisVal();
+ ThisRD = cast<CXXMethodDecl>(Call.getDecl())->getParent();
AlwaysReturnsLValue = true;
}
+ assert(ThisRD);
+ if (ThisRD->isEmpty()) {
+ // Do nothing for empty classes. Otherwise it'd retrieve an UnknownVal
+ // and bind it and RegionStore would think that the actual value
+ // in this region at this offset is unknown.
+ return;
+ }
+
const LocationContext *LCtx = Pred->getLocationContext();
ExplodedNodeSet Dst;
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -729,3 +729,23 @@
S s;
}
}
+
+namespace EmptyBaseAssign {
+struct B1 {};
+struct B2 { int x; };
+struct D: public B1, public B2 {
+const D &operator=(const D &d) {
+ *((B2 *)this) = d;
+ *((B1 *)this) = d;
+ return *this;
+}
+};
+
+void test() {
+ D d1;
+ d1.x = 1;
+ D d2;
+ d2 = d1;
+ clang_analyzer_eval(d2.x == 1); // expected-warning{{TRUE}}
+}
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -42,19 +42,30 @@
const CallEvent &Call) {
SVal ThisVal;
bool AlwaysReturnsLValue;
+ const CXXRecordDecl *ThisRD = nullptr;
if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
assert(Ctor->getDecl()->isTrivial());
assert(Ctor->getDecl()->isCopyOrMoveConstructor());
ThisVal = Ctor->getCXXThisVal();
+ ThisRD = Ctor->getDecl()->getParent();
AlwaysReturnsLValue = false;
} else {
assert(cast<CXXMethodDecl>(Call.getDecl())->isTrivial());
assert(cast<CXXMethodDecl>(Call.getDecl())->getOverloadedOperator() ==
OO_Equal);
ThisVal = cast<CXXInstanceCall>(Call).getCXXThisVal();
+ ThisRD = cast<CXXMethodDecl>(Call.getDecl())->getParent();
AlwaysReturnsLValue = true;
}
+ assert(ThisRD);
+ if (ThisRD->isEmpty()) {
+ // Do nothing for empty classes. Otherwise it'd retrieve an UnknownVal
+ // and bind it and RegionStore would think that the actual value
+ // in this region at this offset is unknown.
+ return;
+ }
+
const LocationContext *LCtx = Pred->getLocationContext();
ExplodedNodeSet Dst;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
