NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov,
rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, szepet.
This is a slow quick fix for https://bugs.llvm.org/show_bug.cgi?id=37688 - add
a defensive check against an invalid destructor in the CFG.
Unions with fields with destructors have their own destructor implicitly
deleted. Due to a bug in the CFG we're still trying to evaluate them at the end
of the object's lifetime and crash because we are unable to find the
declaration.
Add a FIXME test for the CFG that demonstrates the bug and a normal test that
demonstrates that we've fixed the crash and are even trying to continue the
analysis beyond that point.
I've no idea why did this require a noexcept specification. I also don't know
why does it require that many levels of structure nestedness; it might be that
Clang doesn't emit an error in this case but in fact the code is invalid - an
error about attempting to rely upon a deleted destructor is emitted in many
simpler cases.
Repository:
rC Clang
https://reviews.llvm.org/D56899
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/cfg.cpp
test/Analysis/unions.cpp
Index: test/Analysis/unions.cpp
===================================================================
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -1,6 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection %s -analyzer-config eagerly-assume=false -verify
extern void clang_analyzer_eval(bool);
+extern void clang_analyzer_warnIfReached();
extern "C" char *strdup(const char *s);
namespace PR14054_reduced {
@@ -121,3 +122,22 @@
y = 1 / y; // no-warning
}
} // end namespace assume_union_contents
+
+namespace pr37688_deleted_union_destructor {
+struct S { ~S(); };
+struct A {
+ ~A() noexcept {}
+ union {
+ struct {
+ S s;
+ } ss;
+ };
+};
+void foo() {
+ A a;
+} // no-crash
+void bar() {
+ foo();
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+} // end namespace pr37688_deleted_union_destructor
Index: test/Analysis/cfg.cpp
===================================================================
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -468,6 +468,37 @@
}
+// FIXME: The destructor for 'a' shouldn't be there because it's deleted
+// in the union.
+// CHECK-LABEL: void foo()
+// CHECK: [B2 (ENTRY)]
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B1]
+// WARNINGS-NEXT: 1: (CXXConstructExpr, struct pr37688_deleted_union_destructor::A)
+// ANALYZER-NEXT: 1: (CXXConstructExpr, [B1.2], struct pr37688_deleted_union_destructor::A)
+// CHECK-NEXT: 2: pr37688_deleted_union_destructor::A a;
+// CHECK-NEXT: 3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT: Preds (1): B2
+// CHECK-NEXT: Succs (1): B0
+// CHECK: [B0 (EXIT)]
+// CHECK-NEXT: Preds (1): B1
+
+namespace pr37688_deleted_union_destructor {
+struct S { ~S(); };
+struct A {
+ ~A() noexcept {}
+ union {
+ struct {
+ S s;
+ } ss;
+ };
+};
+void foo() {
+ A a;
+}
+} // end namespace pr37688_deleted_union_destructor
+
+
// CHECK-LABEL: template<> int *PR18472<int>()
// CHECK: [B2 (ENTRY)]
// CHECK-NEXT: Succs (1): B1
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -604,6 +604,7 @@
ExplodedNode *Pred,
ExplodedNodeSet &Dst,
const EvalCallOptions &CallOpts) {
+ assert(S && "A destructor without a trigger!");
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
@@ -611,6 +612,19 @@
assert(RecordDecl && "Only CXXRecordDecls should have destructors");
const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor();
+ // FIXME: There should always be a Decl, otherwise the destructor call
+ // shouldn't have been added to the CFG in the first place.
+ if (!DtorDecl) {
+ // Skip the invalid destructor. We cannot simply return because
+ // it would interrupt the analysis instead.
+ static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor");
+ // FIXME: PostImplicitCall with a null decl may crash elsewhere anyway.
+ PostImplicitCall PP(/*Decl=*/nullptr, S->getEndLoc(), LCtx, &T);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(PP, Pred->getState(), Pred);
+ return;
+ }
+
CallEventManager &CEMgr = getStateManager().getCallEventManager();
CallEventRef<CXXDestructorCall> Call =
CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
@@ -629,7 +643,6 @@
I != E; ++I)
defaultEvalCall(Bldr, *I, *Call, CallOpts);
- ExplodedNodeSet DstPostCall;
getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
*Call, *this);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
