r351610 - [analyzer] pr37688: Fix a crash upon evaluating a deleted destructor of a union.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri Jan 18 15:05:07 2019
New Revision: 351610

URL: http://llvm.org/viewvc/llvm-project?rev=351610&view=rev
Log:
[analyzer] pr37688: Fix a crash upon evaluating a deleted destructor of a union.

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 destructor's declaration.

rdar://problem/47362608

Differential Revision: https://reviews.llvm.org/D56899

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/test/Analysis/cfg.cpp
    cfe/trunk/test/Analysis/unions.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=351610&r1=351609&r2=351610&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Fri Jan 18 15:05:07 2019
@@ -604,6 +604,7 @@ void ExprEngine::VisitCXXDestructor(Qual
                                     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 @@ void ExprEngine::VisitCXXDestructor(Qual
   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 @@ void ExprEngine::VisitCXXDestructor(Qual
        I != E; ++I)
     defaultEvalCall(Bldr, *I, *Call, CallOpts);
 
-  ExplodedNodeSet DstPostCall;
   getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
                                              *Call, *this);
 }

Modified: cfe/trunk/test/Analysis/cfg.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg.cpp?rev=351610&r1=351609&r2=351610&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cfg.cpp (original)
+++ cfe/trunk/test/Analysis/cfg.cpp Fri Jan 18 15:05:07 2019
@@ -468,6 +468,37 @@ void test_lifetime_extended_temporaries(
 }
 
 
+// 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

Modified: cfe/trunk/test/Analysis/unions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unions.cpp?rev=351610&r1=351609&r2=351610&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/unions.cpp (original)
+++ cfe/trunk/test/Analysis/unions.cpp Fri Jan 18 15:05:07 2019
@@ -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 @@ void test() {
     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