Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-15 Thread NAKAMURA Takumi via cfe-commits
chapuni added a subscriber: chapuni.


Comment at: lib/CodeGen/CGClass.cpp:1756
@@ +1755,3 @@
+ class SanitizeDtorVTable final : public EHScopeStack::Cleanup {
+const CXXDestructorDecl *Dtor;
+

It causes a warning in -Asserts. [-Wunused-private-field]


http://reviews.llvm.org/D12712



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


Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-15 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 34851.
nmusgrave added a comment.

- Remove commented-out block.


http://reviews.llvm.org/D12712

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  test/CodeGenCXX/sanitize-dtor-vtable.cpp

Index: test/CodeGenCXX/sanitize-dtor-vtable.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-vtable.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -O0 -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 A {
+ public:
+  int x;
+  A() {}
+  virtual ~A() {}
+};
+A a;
+
+class B : virtual public A {
+ public:
+  int y;
+  B() {}
+  ~B() {}
+};
+B b;
+
+// CHECK-LABEL: define {{.*}}AD1Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}AD2Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// After invoking base dtor and dtor for virtual base, poison vtable ptr.
+// CHECK-LABEL: define {{.*}}BD1Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}BD2Ev
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: call void {{.*}}AD2Ev
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// Since no virtual bases, poison vtable ptr here.
+// CHECK-LABEL: define {{.*}}AD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
+
+// Poison members
+// CHECK-LABEL: define {{.*}}BD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-derived-class.cpp
===
--- test/CodeGenCXX/sanitize-dtor-derived-class.cpp
+++ test/CodeGenCXX/sanitize-dtor-derived-class.cpp
@@ -1,8 +1,9 @@
 // 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
+// Base dtor poisons members
+// Complete dtor poisons vtable ptr after destroying members and
+// virtual bases
 
 class Base {
  public:
@@ -28,6 +29,7 @@
 
 Derived d;
 
+// Invoke base destructor. No vtable pointer to poison.
 // CHECK-LABEL: define {{.*}}DerivedD1Ev
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: call void {{.*}}DerivedD2Ev
@@ -40,6 +42,7 @@
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Invokes base destructor, and poison vtable pointer.
 // CHECK-LABEL: define {{.*}}BaseD1Ev
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: call void {{.*}}BaseD2Ev
@@ -52,14 +55,17 @@
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison members and vtable ptr.
 // CHECK-LABEL: define {{.*}}BaseD2Ev
 // CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison members and destroy non-virtual base.
 // 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: call void @__sanitizer_dtor_callback{{.*}}i64 8
 // CHECK: ret void
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1648,11 +1648,27 @@
 }
   };
 
-  class SanitizeDtor final : public EHScopeStack::Cleanup {
+ static void EmitSanitizerDtorCallback(CodeGenFunction , llvm::Value *Ptr,
+ CharUnits::QuantityType PoisonSize) {
+   // Pass in void pointer and size of region as arguments to runtime
+   // function
+   llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, 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");
+   CGF.EmitNounwindRuntimeCall(Fn, Args);
+ }
+
+  class 

Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-11 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 34614.
nmusgrave marked 2 inline comments as done.
nmusgrave added a comment.

- Fixed testing callback emission order to account for vptr.


http://reviews.llvm.org/D12712

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  test/CodeGenCXX/sanitize-dtor-vtable.cpp

Index: test/CodeGenCXX/sanitize-dtor-vtable.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-vtable.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -O0 -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 A {
+ public:
+  int x;
+  A() {}
+  virtual ~A() {}
+};
+A a;
+
+// CHECK-LABEL: define {{.*}}AD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-derived-class.cpp
===
--- test/CodeGenCXX/sanitize-dtor-derived-class.cpp
+++ test/CodeGenCXX/sanitize-dtor-derived-class.cpp
@@ -52,14 +52,17 @@
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // CHECK-LABEL: define {{.*}}BaseD2Ev
 // CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // 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: call void @__sanitizer_dtor_callback{{.*}}i64 8
 // CHECK: ret void
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1648,11 +1648,27 @@
 }
   };
 
-  class SanitizeDtor final : public EHScopeStack::Cleanup {
+ static void EmitSanitizerDtorCallback(CodeGenFunction , llvm::Value *Ptr,
+ CharUnits::QuantityType PoisonSize) {
+   // Pass in void pointer and size of region as arguments to runtime
+   // function
+   llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, 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");
+   CGF.EmitNounwindRuntimeCall(Fn, Args);
+ }
+
+  class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
 const CXXDestructorDecl *Dtor;
 
   public:
-SanitizeDtor(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+SanitizeDtorMembers(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
 
 // Generate function call for handling object poisoning.
 // Disables tail call elimination, to prevent the current stack frame
@@ -1684,11 +1700,11 @@
   // Currently on the last field, and it must be poisoned with the
   // current block.
   if (fieldIndex == Layout.getFieldCount() - 1) {
-PoisonBlock(CGF, startIndex, Layout.getFieldCount());
+PoisonMembers(CGF, startIndex, Layout.getFieldCount());
   }
 } else if (startIndex >= 0) {
   // No longer within a block of memory to poison, so poison the block
-  PoisonBlock(CGF, startIndex, fieldIndex);
+  PoisonMembers(CGF, startIndex, fieldIndex);
   // Re-set the start index
   startIndex = -1;
 }
@@ -1701,7 +1717,7 @@
 /// start poisoning (inclusive)
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
-void PoisonBlock(CodeGenFunction , unsigned layoutStartOffset,
+void PoisonMembers(CodeGenFunction , unsigned layoutStartOffset,
  unsigned layoutEndOffset) {
   ASTContext  = CGF.getContext();
   const ASTRecordLayout  =
@@ -1732,20 +1748,30 @@
   if (PoisonSize == 0)
 return;
 
-  // Pass in void pointer and size of region as arguments to runtime
-  // function
-  llvm::Value *Args[] = {CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
- llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
+  EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize);
+}
+  };
+
+ class SanitizeDtorVTable final : public EHScopeStack::Cleanup {
+const CXXDestructorDecl *Dtor;
+
+  public:
+

Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-11 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 34617.
nmusgrave added a comment.

- Poison vtable in either complete or base dtor.


http://reviews.llvm.org/D12712

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  test/CodeGenCXX/sanitize-dtor-vtable.cpp

Index: test/CodeGenCXX/sanitize-dtor-vtable.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-vtable.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -O0 -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 A {
+ public:
+  int x;
+  A() {}
+  virtual ~A() {}
+};
+A a;
+
+// CHECK-LABEL: define {{.*}}AD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-derived-class.cpp
===
--- test/CodeGenCXX/sanitize-dtor-derived-class.cpp
+++ test/CodeGenCXX/sanitize-dtor-derived-class.cpp
@@ -1,8 +1,9 @@
 // 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
+// Base dtor poisons members
+// Complete dtor poisons vtable ptr after destroying members and
+// virtual bases
 
 class Base {
  public:
@@ -52,14 +53,17 @@
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // CHECK-LABEL: define {{.*}}BaseD2Ev
 // CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // 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: call void @__sanitizer_dtor_callback{{.*}}i64 8
 // CHECK: ret void
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1648,11 +1648,27 @@
 }
   };
 
-  class SanitizeDtor final : public EHScopeStack::Cleanup {
+ static void EmitSanitizerDtorCallback(CodeGenFunction , llvm::Value *Ptr,
+ CharUnits::QuantityType PoisonSize) {
+   // Pass in void pointer and size of region as arguments to runtime
+   // function
+   llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, 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");
+   CGF.EmitNounwindRuntimeCall(Fn, Args);
+ }
+
+  class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
 const CXXDestructorDecl *Dtor;
 
   public:
-SanitizeDtor(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+SanitizeDtorMembers(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
 
 // Generate function call for handling object poisoning.
 // Disables tail call elimination, to prevent the current stack frame
@@ -1684,11 +1700,11 @@
   // Currently on the last field, and it must be poisoned with the
   // current block.
   if (fieldIndex == Layout.getFieldCount() - 1) {
-PoisonBlock(CGF, startIndex, Layout.getFieldCount());
+PoisonMembers(CGF, startIndex, Layout.getFieldCount());
   }
 } else if (startIndex >= 0) {
   // No longer within a block of memory to poison, so poison the block
-  PoisonBlock(CGF, startIndex, fieldIndex);
+  PoisonMembers(CGF, startIndex, fieldIndex);
   // Re-set the start index
   startIndex = -1;
 }
@@ -1701,7 +1717,7 @@
 /// start poisoning (inclusive)
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
-void PoisonBlock(CodeGenFunction , unsigned layoutStartOffset,
+void PoisonMembers(CodeGenFunction , unsigned layoutStartOffset,
  unsigned layoutEndOffset) {
   ASTContext  = CGF.getContext();
   

Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-09 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 34373.
nmusgrave added a comment.

- Cleaned up impl.


http://reviews.llvm.org/D12712

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  test/CodeGenCXX/sanitize-dtor-vtable.cpp

Index: test/CodeGenCXX/sanitize-dtor-vtable.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-vtable.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -O0 -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 A {
+ public:
+  int x;
+  A() {}
+  virtual ~A() {}
+};
+A a;
+
+// CHECK-LABEL: define {{.*}}AD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-derived-class.cpp
===
--- test/CodeGenCXX/sanitize-dtor-derived-class.cpp
+++ test/CodeGenCXX/sanitize-dtor-derived-class.cpp
@@ -52,14 +52,17 @@
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // CHECK-LABEL: define {{.*}}BaseD2Ev
 // CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // 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: call void @__sanitizer_dtor_callback
 // CHECK: ret void
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1648,11 +1648,15 @@
 }
   };
 
-  class SanitizeDtor final : public EHScopeStack::Cleanup {
+
+ static void Poison(CodeGenFunction , llvm::Value *OffsetPtr,
+CharUnits::QuantityType PoisonSize);
+
+  class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
 const CXXDestructorDecl *Dtor;
 
   public:
-SanitizeDtor(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+SanitizeDtorMembers(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
 
 // Generate function call for handling object poisoning.
 // Disables tail call elimination, to prevent the current stack frame
@@ -1684,11 +1688,11 @@
   // Currently on the last field, and it must be poisoned with the
   // current block.
   if (fieldIndex == Layout.getFieldCount() - 1) {
-PoisonBlock(CGF, startIndex, Layout.getFieldCount());
+PoisonMembers(CGF, startIndex, Layout.getFieldCount());
   }
 } else if (startIndex >= 0) {
   // No longer within a block of memory to poison, so poison the block
-  PoisonBlock(CGF, startIndex, fieldIndex);
+  PoisonMembers(CGF, startIndex, fieldIndex);
   // Re-set the start index
   startIndex = -1;
 }
@@ -1701,7 +1705,7 @@
 /// start poisoning (inclusive)
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
-void PoisonBlock(CodeGenFunction , unsigned layoutStartOffset,
+void PoisonMembers(CodeGenFunction , unsigned layoutStartOffset,
  unsigned layoutEndOffset) {
   ASTContext  = CGF.getContext();
   const ASTRecordLayout  =
@@ -1732,20 +1736,46 @@
   if (PoisonSize == 0)
 return;
 
-  // Pass in void pointer and size of region as arguments to runtime
-  // function
-  llvm::Value *Args[] = {CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
- llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
+  Poison(CGF, OffsetPtr, PoisonSize);
+}
+  };
+
+ class SanitizeDtorVTable final : public EHScopeStack::Cleanup {
+const CXXDestructorDecl *Dtor;
 
-  llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
+  public:
+SanitizeDtorVTable(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+
+// Generate function call for handling vtable pointer poisoning.
+void Emit(CodeGenFunction , Flags flags) override {
+  assert(Dtor->getParent()->isDynamicClass());
+  ASTContext  = CGF.getContext();
+  // Poison vtable and vtable ptr if they exist for this class.
+  llvm::Value *VTablePtr = CGF.LoadCXXThis();
 
-  llvm::FunctionType *FnType =
-  llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
-  llvm::Value *Fn =
-  CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
-  

Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-09 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 34357.
nmusgrave added a comment.

- Fixed testing callback emission order to account for vptr. Vptr poisoned 
after all virtual and member destructors are invoked, in order to prevent a 
data race an on the virtual function invoked by a class instance. 
(https://github.com/google/sanitizers/wiki/ThreadSanitizerPopularDataRaces#data-race-on-vptr)


http://reviews.llvm.org/D12712

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  test/CodeGenCXX/sanitize-dtor-vtable.cpp

Index: test/CodeGenCXX/sanitize-dtor-vtable.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-vtable.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -O0 -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 A {
+ public:
+  int x;
+  A() {}
+  virtual ~A() {}
+};
+A a;
+
+// CHECK-LABEL: define {{.*}}AD2Ev
+// CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback{{.*}}i64 8
+// CHECK-NOT: call void @__sanitizer_dtor_callback
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-derived-class.cpp
===
--- test/CodeGenCXX/sanitize-dtor-derived-class.cpp
+++ test/CodeGenCXX/sanitize-dtor-derived-class.cpp
@@ -52,14 +52,17 @@
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // CHECK-LABEL: define {{.*}}BaseD2Ev
 // CHECK: call void @__sanitizer_dtor_callback
+// CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
+// Poison member and vtable pointer.
 // 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: call void @__sanitizer_dtor_callback
 // CHECK: ret void
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1648,11 +1648,15 @@
 }
   };
 
-  class SanitizeDtor final : public EHScopeStack::Cleanup {
+
+ static void Poison(CodeGenFunction , llvm::Value *OffsetPtr,
+CharUnits::QuantityType PoisonSize);
+
+  class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
 const CXXDestructorDecl *Dtor;
 
   public:
-SanitizeDtor(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+SanitizeDtorMembers(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
 
 // Generate function call for handling object poisoning.
 // Disables tail call elimination, to prevent the current stack frame
@@ -1668,9 +1672,21 @@
   // Prevent the current stack frame from disappearing from the stack trace.
   CGF.CurFn->addFnAttr("disable-tail-calls", "true");
 
+  ASTContext  = CGF.getContext();
+  /*
+  // Poison vtable and vtable ptr if they exist for this class.
+  if (Dtor->getParent()->isDynamicClass()) {
+llvm::Value *VTablePtr = CGF.LoadCXXThis();
+
+CharUnits::QuantityType PoisonSize =
+Context.toCharUnitsFromBits(CGF.PointerWidthInBits).getQuantity();
+// Pass in void pointer and size of region as arguments to runtime
+// function
+Poison(CGF, VTablePtr, PoisonSize);
+  }
+  */
   // Construct pointer to region to begin poisoning, and calculate poison
   // size, so that only members declared in this class are poisoned.
-  ASTContext  = CGF.getContext();
   unsigned fieldIndex = 0;
   int startIndex = -1;
   // RecordDecl::field_iterator Field;
@@ -1684,11 +1700,11 @@
   // Currently on the last field, and it must be poisoned with the
   // current block.
   if (fieldIndex == Layout.getFieldCount() - 1) {
-PoisonBlock(CGF, startIndex, Layout.getFieldCount());
+PoisonMembers(CGF, startIndex, Layout.getFieldCount());
   }
 } else if (startIndex >= 0) {
   // No longer within a block of memory to poison, so poison the block
-  PoisonBlock(CGF, startIndex, fieldIndex);
+  PoisonMembers(CGF, startIndex, fieldIndex);
   // Re-set the start index
   startIndex = -1;
 }
@@ -1701,7 +1717,7 @@
 /// start poisoning (inclusive)
 /// \param layoutEndOffset index of the ASTRecordLayout field to
 /// end poisoning (exclusive)
-void PoisonBlock(CodeGenFunction , unsigned layoutStartOffset,
+void PoisonMembers(CodeGenFunction , unsigned layoutStartOffset,
  unsigned 

Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-09 Thread Naomi Musgrave via cfe-commits
nmusgrave marked an inline comment as done.
nmusgrave added a comment.

http://reviews.llvm.org/D12712



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


Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-09 Thread Evgeniy Stepanov via cfe-commits
eugenis added inline comments.


Comment at: lib/CodeGen/CGClass.cpp:1685
@@ +1684,3 @@
+// function
+Poison(CGF, VTablePtr, PoisonSize);
+  }

Did you mean to move this chunk to the other cleanup class?
Is there a test that would fail if vptr is prematurely poisoned? There should 
be one (it's ok if it is only in compiler-rt, this could be hard to test with 
lit over IR).



http://reviews.llvm.org/D12712



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


Re: [PATCH] D12712: Implementation and testing for poisoning vtable ptr in dtor.

2015-09-08 Thread Evgeniy Stepanov via cfe-commits
eugenis added inline comments.


Comment at: lib/CodeGen/CGClass.cpp:1671
@@ -1670,1 +1670,3 @@
 
+  ASTContext  = CGF.getContext();
+  // Poison vtable and vtable ptr if they exist for this class.

You are poisoning the vtable pointer in the base destructor.
Isn't that too early?
For example, in the following case the vptr would be poisoned before ~A, right?
https://github.com/google/sanitizers/wiki/ThreadSanitizerPopularDataRaces#data-race-on-vptr



http://reviews.llvm.org/D12712



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