Poison only class members in dtor, simpler test cases.

[nmusgrave]

Summary

Poison only class members in destructor, as opposed to poisoning inherited
members as well. Verify class members poisoned only once, before invoking
base class destructors.
Simplified test cases to focus on one feature at time.
Resolves issues from rL244819 (rolled back in revert rL244820).
Closed in rL244933

[eugenis]

test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp

14–15

In the spirit of simplifying test cases, let's make this one purely about the attribute.

1. Remove the no-attribute case, it is tested somewhere else already.
2. No need to verify that No_SanD2Ev calls ~Vector. Check that: a. Destructors of No_San do not call sanitizer callback b. All destructors of Vector call sanitizer callback

[nmusgrave]

• test examines member dtor

[nmusgrave]

• removed comment

[nmusgrave]

• test marked as failing, since impl for nontrivial members pending

[eugenis]

test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp

28–29

Now this does not check the sanitizer callback.
No need to check that D1 calls D2 - it;s not the point of this test case.
Just CHECK-NOT the sanitizer callback here and below.

[nmusgrave]

• test: non-trivial member modified to ensure sanitizing callback will be emitted

[eugenis]

LGTM

[nmusgrave]

Simplify test comment

lib/CodeGen/CGClass.cpp

 // references to 'this' and its size as arguments.
 // Disables tail call elimination, to prevent the current stack frame from
 // disappearing from the stack trace.
 static void EmitDtorSanitizerCallback(CodeGenFunction &CGF,
                                       const CXXDestructorDecl *Dtor) {
   const ASTRecordLayout &Layout =
       CGF.getContext().getASTRecordLayout(Dtor->getParent());
 
+  // Nothing to poison
+  if(Layout.getFieldCount() == 0)
+    return;
+
+  // Construct pointer to region to begin poisoning, and calculate poison
+  // size, so that only members declared in this class are poisoned.
+  llvm::Value *OffsetPtr;
+  CharUnits::QuantityType PoisonSize;
+  ASTContext &Context = CGF.getContext();
+
+  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
+      CGF.SizeTy, Context.toCharUnitsFromBits(Layout.getFieldOffset(0)).
+      getQuantity());
+
+  OffsetPtr = CGF.Builder.CreateGEP(CGF.Builder.CreateBitCast(
+      CGF.LoadCXXThis(), CGF.Int8PtrTy), OffsetSizePtr);
+
+  PoisonSize = Layout.getSize().getQuantity() -
+      Context.toCharUnitsFromBits(Layout.getFieldOffset(0)).getQuantity();
+
   llvm::Value *Args[] = {
-      CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.VoidPtrTy),
-      llvm::ConstantInt::get(CGF.SizeTy, Layout.getSize().getQuantity())};
+    CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
+    llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
+
   llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
 
   llvm::FunctionType *FnType =
       llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
   llvm::Value *Fn =
       CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
 
+  // Disables tail call elimination, to prevent the current stack frame from
+  // disappearing from the stack trace.
   CGF.CurFn->addFnAttr("disable-tail-calls", "true");
   CGF.EmitNounwindRuntimeCall(Fn, Args);
 }
 
 /// EmitDestructorBody - Emits the body of the current destructor.
 void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   const CXXDestructorDecl *Dtor = cast<CXXDestructorDecl>(CurGD.getDecl());
   CXXDtorType DtorType = CurGD.getDtorType();
case Dtor_Base:
     else {
       assert(Dtor->isImplicit() && "bodyless dtor not implicit");
       // nothing to do besides what's in the epilogue
     }
     // -fapple-kext must inline any call to this dtor into
     // the caller's body.
     if (getLangOpts().AppleKext)
       CurFn->addFnAttr(llvm::Attribute::AlwaysInline);
+
+    // Insert memory-poisoning instrumentation, before final clean ups,
+    // to ensure this class's members are protected from invalid access.
+    if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor
+        && SanOpts.has(SanitizerKind::Memory))
+      EmitDtorSanitizerCallback(*this, Dtor);
+
     break;
   }
 
   // Jump out through the epilogue cleanups.
   DtorEpilogue.ForceCleanup();
 
   // Exit the try if applicable.
   if (isTryBody)
     ExitCXXTryStmt(*cast<CXXTryStmt>(Body), true);
-
-  // Insert memory-poisoning instrumentation.
-  if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor
-      && SanOpts.has(SanitizerKind::Memory))
-    EmitDtorSanitizerCallback(*this, Dtor);
 }
 
 void CodeGenFunction::emitImplicitAssignmentOperatorBody(FunctionArgList &Args) {
   const CXXMethodDecl *AssignOp = cast<CXXMethodDecl>(CurGD.getDecl());
   const Stmt *RootS = AssignOp->getBody();
   assert(isa<CompoundStmt>(RootS) &&
          "Body of an implicit assignment operator should be compound stmt.");
   const CompoundStmt *RootCS = cast<CompoundStmt>(RootS);

test/CodeGenCXX/sanitize-dtor-callback.cpp

 // Test -fsanitize-memory-use-after-dtor
 // RUN: %clang_cc1 -fsanitize=memory -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
 
+// Sanitizing dtor is emitted in dtor for every class
+
 struct Simple {
+  int x;
   ~Simple() {}
 };
 Simple s;
 // Simple internal member is poisoned by compiler-generated dtor
 // CHECK-LABEL: define {{.*}}SimpleD1Ev
-// CHECK: call void @__sanitizer_dtor_callback
-// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}SimpleD2Ev
 // CHECK: ret void
 
 struct Inlined {
+  int y;
   inline ~Inlined() {}
 };
 Inlined i;
 // Simple internal member is poisoned by compiler-generated dtor
 // CHECK-LABEL: define {{.*}}InlinedD1Ev
-// CHECK: call void @__sanitizer_dtor_callback
-// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}InlinedD2Ev
 // CHECK: ret void
 
 struct Defaulted_Trivial {
   ~Defaulted_Trivial() = default;
 };
 void create_def_trivial() {
   Defaulted_Trivial def_trivial;
 }
 // The compiler is explicitly signalled to handle object cleanup.
 // No complex member attributes. Compiler destroys inline, so
 // no destructor defined.
 // CHECK-LABEL: define {{.*}}create_def_trivial
 // CHECK-NOT: call {{.*}}Defaulted_Trivial
-// CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 struct Defaulted_Non_Trivial {
   Simple s;
   ~Defaulted_Non_Trivial() = default;
 };
 Defaulted_Non_Trivial def_non_trivial;
 // Explicitly compiler-generated dtor poisons object.
 // By including a Simple member in the struct, the compiler is
 // forced to generate a non-trivial destructor.
 // CHECK-LABEL: define {{.*}}Defaulted_Non_TrivialD1Ev
-// CHECK: call void @__sanitizer_dtor_callback
-// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}Defaulted_Non_TrivialD2
 // CHECK: ret void
 
 
 // Note: ordering is important. In the emitted bytecode, these
 // second dtors defined after the first. Explicitly checked here
 // to confirm that all invoked dtors have member poisoning
 // instrumentation inserted.
 // CHECK-LABEL: define {{.*}}SimpleD2Ev

test/CodeGenCXX/sanitize-dtor-derived-class.cpp

+// RUN: %clang_cc1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+// Only the last dtor of a class invokes the sanitizing callback
+// Sanitizing callback emited prior to base class dtor invocations
+
+class Base {
+ public:
+  int x;
+  Base() {
+    x = 5;
+  }
+  virtual ~Base() {
+    x += 1;
+  }
+};
+
+class Derived : public Base {
+ public:
+  int y;
+  Derived() {
+    y = 10;
+  }
+  ~Derived() {
+    y += 1;
+  }
+};
+
+Derived d;
+
+// CHECK-LABEL: define {{.*}}DerivedD1Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}DerivedD2Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}DerivedD0Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}DerivedD1Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}BaseD1Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}BaseD2Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}BaseD0Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}BaseD1Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}BaseD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}DerivedD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}BaseD2Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void

test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp

 // Test -fsanitize-memory-use-after-dtor
 // RUN: %clang_cc1 -fsanitize=memory -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -DATTRIBUTE -fsanitize=memory -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-ATTR
+
+// The no_sanitize_memory attribute, when applied to a destructor,
+// represses emission of sanitizing callback
 
 template <class T> class Vector {
  public:
   ~Vector() {}
 };
 
 struct No_San {
   Vector<int> v;
   No_San() { }
-#ifdef ATTRIBUTE
-  __attribute__((no_sanitize_memory)) ~No_San() = default;
-#else
+  //__attribute__((no_sanitize_memory))
   ~No_San() = default;
-#endif
 };
 
 int main() {
   No_San *ns = new No_San();
   ns->~No_San();
   return 0;
 }
 
 // Repressing the sanitization attribute results in no msan
 // instrumentation of the destructor
 // CHECK: define {{.*}}No_SanD1Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
 // CHECK: call void {{.*}}No_SanD2Ev
-// CHECK: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
-// CHECK-ATTR: define {{.*}}No_SanD1Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
-// CHECK-ATTR: call void {{.*}}No_SanD2Ev
-// CHECK-ATTR-NOT: call void @__sanitizer_dtor_callback
-// CHECK-ATTR: ret void
-
-
 // CHECK: define {{.*}}No_SanD2Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
-// CHECK: call void {{.*}}Vector
-// CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}VectorIiED1Ev
 // CHECK: ret void
 
-// CHECK-ATTR: define {{.*}}No_SanD2Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
-// CHECK-ATTR: call void {{.*}}Vector
-// CHECK-ATTR-NOT: call void @__sanitizer_dtor_callback
-// CHECK-ATTR: ret void
+// CHECK: define {{.*}}VectorIiED1Ev
+// CHECK: call void {{.*}}VectorIiED2Ev
+// CHECK: ret void
+
+// CHECK: define {{.*}}VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback
+// CHECK: ret void
 
 // When attribute is repressed, the destructor does not emit any tail calls
-// CHECK: attributes [[ATTRIBUTE]] = {{.*}} sanitize_memory
-// CHECK-ATTR-NOT: attributes [[ATTRIBUTE]] = {{.*}} sanitize_memory
+// CHECK-NOT: attributes [[ATTRIBUTE]] = {{.*}} sanitize_memory