Author: nmusgrave
Date: Wed Aug 12 16:37:40 2015
New Revision: 244819

URL: http://llvm.org/viewvc/llvm-project?rev=244819&view=rev
Log:
Implement poisoning of only class members in dtor, as opposed to also poisoning 
fields inherited from base classes.
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

Added:
    cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
Modified:
    cfe/trunk/lib/CodeGen/CGClass.cpp
    cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp
    cfe/trunk/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=244819&r1=244818&r2=244819&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Aug 12 16:37:40 2015
@@ -1376,9 +1376,30 @@ static void EmitDtorSanitizerCallback(Co
   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(Co
   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
     // 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
   // 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) {

Modified: cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp?rev=244819&r1=244818&r2=244819&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp Wed Aug 12 16:37:40 
2015
@@ -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
 

Added: cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp?rev=244819&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp Wed Aug 12 
16:37:40 2015
@@ -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

Modified: cfe/trunk/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp?rev=244819&r1=244818&r2=244819&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/sanitize-dtor-fn-attribute.cpp Wed Aug 12 
16:37:40 2015
@@ -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


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to