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 &CGF, 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 &Context = 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 &Context = 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 &CGF, unsigned layoutStartOffset,
+ void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
unsigned layoutEndOffset) {
ASTContext &Context = CGF.getContext();
const ASTRecordLayout &Layout =
@@ -1732,10 +1748,16 @@
if (PoisonSize == 0)
return;
+ Poison(CGF, OffsetPtr, PoisonSize);
+ }
+ /*
+ void Poison(CodeGenFunction &CGF, llvm::Value *OffsetPtr,
+ CharUnits::QuantityType PoisonSize) {
// 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)};
+ llvm::Value *Args[] = {
+ CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
+ llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
@@ -1745,7 +1767,45 @@
CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
CGF.EmitNounwindRuntimeCall(Fn, Args);
}
+ */
};
+
+ class SanitizeDtorVTable final : public EHScopeStack::Cleanup {
+ const CXXDestructorDecl *Dtor;
+
+ public:
+ SanitizeDtorVTable(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+
+ // Generate function call for handling vtable pointer poisoning.
+ void Emit(CodeGenFunction &CGF, Flags flags) override {
+ assert(Dtor->getParent()->isDynamicClass());
+ ASTContext &Context = CGF.getContext();
+ // Poison vtable and vtable ptr if they exist for this class.
+ 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);
+ }
+ };
+
+ static void Poison(CodeGenFunction &CGF, llvm::Value *OffsetPtr,
+ CharUnits::QuantityType PoisonSize) {
+ // 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)};
+
+ 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);
+ }
}
/// \brief Emit all code that comes at the end of class's
@@ -1800,6 +1860,11 @@
}
assert(DtorType == Dtor_Base);
+ // Poison the vtable pointer such that access after the base
+ // and member destructors are invoked is invalid.
+ if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+ SanOpts.has(SanitizerKind::Memory) && DD->getParent()->isDynamicClass())
+ EHStack.pushCleanup<SanitizeDtorVTable>(NormalAndEHCleanup, DD);
// Destroy non-virtual bases.
for (const auto &Base : ClassDecl->bases()) {
@@ -1822,7 +1887,7 @@
// invoked, and before the base class destructor runs, is invalid.
if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
SanOpts.has(SanitizerKind::Memory))
- EHStack.pushCleanup<SanitizeDtor>(NormalAndEHCleanup, DD);
+ EHStack.pushCleanup<SanitizeDtorMembers>(NormalAndEHCleanup, DD);
// Destroy direct fields.
for (const auto *Field : ClassDecl->fields()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
