Implement poisoning of only class members in dtor, as opposed to also poisoning field...

Verify emitted code for derived class with virtual destructor sanitizes its members only once.
Changed emission order for dtor callback, so only the last dtor for a class emits the sanitizing callback, while ensuring that class members are poisoned before base class destructors are invoked.
Skip poisoning of members, if class has no fields.
Removed patch file containing extraneous changes.

Summary: Poisoning applied to only class members, and before dtors for base class invoked

Reviewers: eugenis, kcc

Differential Revision: http://reviews.llvm.org/D11951

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@244819 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp
index 740cd4ab4ebae75e5e9935ceddaf3fcaeff4fcb9..fea72f6818e0312643f1bfd8a8a2a0632845a41f 100644 (file)
--- a/lib/CodeGen/CGClass.cpp
+++ b/lib/CodeGen/CGClass.cpp
@@ -1376,9 +1376,30 @@ static void EmitDtorSanitizerCallback(CodeGenFunction &CGF,
   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 =
@@ -1386,6 +1407,8 @@ static void EmitDtorSanitizerCallback(CodeGenFunction &CGF,
   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);
 }
@@ -1468,6 +1491,13 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
     // 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;
   }
 
@@ -1477,11 +1507,6 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) {
   // 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) {

diff --git a/test/CodeGenCXX/sanitize-dtor-callback.cpp b/test/CodeGenCXX/sanitize-dtor-callback.cpp
index a3ecbcbdd8b0d021c3104abae11261c79fd3d184..078efe88cacd6fa42495f443901078ce148714ce 100644 (file)
--- a/test/CodeGenCXX/sanitize-dtor-callback.cpp
+++ b/test/CodeGenCXX/sanitize-dtor-callback.cpp
@@ -7,7 +7,8 @@ struct 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-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
@@ -17,7 +18,8 @@ struct 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-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
@@ -44,7 +46,8 @@ Defaulted_Non_Trivial def_non_trivial;
 // 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-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 

diff --git a/test/CodeGenCXX/sanitize-dtor-derived-class.cpp b/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
new file mode 100644 (file)
index 0000000..2084d53
--- /dev/null
+++ b/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
@@ -0,0 +1,62 @@
+// 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
+
+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

diff --git a/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp b/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp
index 8a9e477cf1b37f294b8e102dfabffe4466f575c2..8d190e09f342f38404c08e3f1c1b04add517e9df 100644 (file)
--- a/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp
+++ b/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp
@@ -26,22 +26,27 @@ int main() {
 // Repressing the sanitization attribute results in no msan
 // instrumentation of the destructor
 // CHECK: define {{.*}}No_SanD1Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
+// CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: call void {{.*}}No_SanD2Ev
-// CHECK: call void @__sanitizer_dtor_callback
+// CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-ATTR: define {{.*}}No_SanD1Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
+// CHECK-ATTR-NOT: call void @__sanitizer_dtor_callback
 // 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-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}Vector
+// CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-ATTR: define {{.*}}No_SanD2Ev{{.*}} [[ATTRIBUTE:#[0-9]+]]
+// CHECK-ATTR-NOT: call void @__sanitizer_dtor_callback
 // CHECK-ATTR: call void {{.*}}Vector
 // CHECK-ATTR-NOT: call void @__sanitizer_dtor_callback
 // CHECK-ATTR: ret void