nmusgrave updated this revision to Diff 33115.
nmusgrave added a comment.
- Simplify parameters, rename function, for examining fields of class to
destroy.
http://reviews.llvm.org/D12022
Files:
lib/CodeGen/CGCXX.cpp
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template <class T>
+class Vector {
+public:
+ int size;
+ ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+ Vector<int> v;
+ int x;
+ Base() { x = 5; }
+ virtual ~Base() {}
+};
+
+struct Derived : public Base {
+ int z;
+ Derived() { z = 10; }
+ ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template <class T>
+class Vector {
+public:
+ int size;
+ ~Vector() {
+ size += 1;
+ }
+};
+
+struct Base {
+ int b1;
+ double b2;
+ Base() {
+ b1 = 5;
+ b2 = 10.989;
+ }
+ virtual ~Base() {}
+};
+
+struct VirtualBase {
+ int vb1;
+ int vb2;
+ VirtualBase() {
+ vb1 = 10;
+ vb2 = 11;
+ }
+ virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+ int d1;
+ Vector<int> v;
+ int d2;
+ Derived() {
+ d1 = 10;
+ }
+ ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,13 @@
/// are emitted lazily.
void EmitGlobal(GlobalDecl D);
+ bool
+ HasTrivialDestructorBody(ASTContext &Context,
+ const CXXRecordDecl *BaseClassDecl,
+ const CXXRecordDecl *MostDerivedClassDecl);
+ bool
+ FieldHasTrivialDestructorBody(ASTContext &Context, const FieldDecl *Field);
+
bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
bool InEveryTU);
bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
CM.finish();
}
-static bool
-FieldHasTrivialDestructorBody(ASTContext &Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext &Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext &Context,
const CXXRecordDecl *BaseClassDecl,
const CXXRecordDecl *MostDerivedClassDecl)
{
@@ -1332,10 +1328,12 @@
return true;
}
-static bool
-FieldHasTrivialDestructorBody(ASTContext &Context,
- const FieldDecl *Field)
+bool CodeGenModule::FieldHasTrivialDestructorBody(ASTContext &Context,
+ const FieldDecl *Field)
{
+ if (Field->getType()->isPointerType())
+ return true;
+
QualType FieldBaseElementType = Context.getBaseElementType(Field->getType());
const RecordType *RT = FieldBaseElementType->getAs<RecordType>();
@@ -1353,66 +1351,20 @@
/// CanSkipVTablePointerInitialization - Check whether we need to initialize
/// any vtable pointers before calling this destructor.
-static bool CanSkipVTablePointerInitialization(ASTContext &Context,
+static bool CanSkipVTablePointerInitialization(CodeGenFunction &CGF,
const CXXDestructorDecl *Dtor) {
if (!Dtor->hasTrivialBody())
return false;
// Check the fields.
const CXXRecordDecl *ClassDecl = Dtor->getParent();
for (const auto *Field : ClassDecl->fields())
- if (!FieldHasTrivialDestructorBody(Context, Field))
+ if (!CGF.CGM.FieldHasTrivialDestructorBody(CGF.getContext(), Field))
return false;
return true;
}
-// Generates function call for handling object poisoning, passing in
-// 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(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());
@@ -1476,7 +1428,7 @@
EnterDtorCleanups(Dtor, Dtor_Base);
// Initialize the vtable pointers before entering the body.
- if (!CanSkipVTablePointerInitialization(getContext(), Dtor))
+ if (!CanSkipVTablePointerInitialization(*this, Dtor))
InitializeVTablePointers(Dtor->getParent());
if (isTryBody)
@@ -1492,12 +1444,6 @@
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;
}
@@ -1541,7 +1487,7 @@
llvm::Value *ShouldDeleteCondition;
public:
CallDtorDeleteConditional(llvm::Value *ShouldDeleteCondition)
- : ShouldDeleteCondition(ShouldDeleteCondition) {
+ : ShouldDeleteCondition(ShouldDeleteCondition) {
assert(ShouldDeleteCondition != nullptr);
}
@@ -1571,8 +1517,8 @@
public:
DestroyField(const FieldDecl *field, CodeGenFunction::Destroyer *destroyer,
bool useEHCleanupForArray)
- : field(field), destroyer(destroyer),
- useEHCleanupForArray(useEHCleanupForArray) {}
+ : field(field), destroyer(destroyer),
+ useEHCleanupForArray(useEHCleanupForArray) {}
void Emit(CodeGenFunction &CGF, Flags flags) override {
// Find the address of the field.
@@ -1586,6 +1532,121 @@
flags.isForNormalCleanup() && useEHCleanupForArray);
}
};
+
+ class SanitizeDtor : public EHScopeStack::Cleanup {
+ const CXXDestructorDecl *Dtor;
+
+ public:
+ SanitizeDtor(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+
+ // Generate function call for handling object poisoning.
+ // Disables tail call elimination, to prevent the current stack frame
+ // from disappearing from the stack trace.
+ void Emit(CodeGenFunction &CGF, Flags flags) override {
+ const ASTRecordLayout &Layout =
+ CGF.getContext().getASTRecordLayout(Dtor->getParent());
+
+ // Nothing to poison
+ if (Layout.getFieldCount() == 0)
+ return;
+
+ // Iterate over fields declared in this class, and count all fields,
+ // including inherited from base classes.
+ unsigned totalFields = 0;
+ for (auto *Field : Dtor->getParent()->fields()) {
+ (void)Field;
+ totalFields += 1;
+ }
+
+ // 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;
+ unsigned inheritedFields = totalFields - Layout.getFieldCount();
+ // todo: don't use iterator for accessing fields
+ RecordDecl::field_iterator Field;
+ for (Field = Dtor->getParent()->field_begin();
+ Field != Dtor->getParent()->field_end(); Field++) {
+ // Skip inherited fields
+ if (fieldIndex < inheritedFields)
+ continue;
+
+ // Poison field if it is trivial
+ if (CGF.CGM.FieldHasTrivialDestructorBody(Context, *Field)) {
+ // Start sanitizing at this field
+ if (startIndex < 0)
+ startIndex = fieldIndex;
+
+ // Currently on the last field, and it must be poisoned with the
+ // current block.
+ if (fieldIndex - inheritedFields == Layout.getFieldCount() - 1) {
+ PoisonBlock(CGF, startIndex - inheritedFields,
+ fieldIndex - inheritedFields + 1);
+ }
+ } else if (startIndex >= 0) {
+ // No longer within a block of memory to poison, so poison the block
+ PoisonBlock(CGF, startIndex - inheritedFields,
+ fieldIndex - inheritedFields);
+ // Re-set the start index
+ startIndex = -1;
+ }
+ fieldIndex += 1;
+ }
+ }
+
+ private:
+ // layoutStartOffset: index of the ASTRecordLayout field to start poisoning
+ // (inclusive)
+ // layoutEndOffset: index of the ASTRecordLayout field to end poisoning
+ // (exclusive)
+ void PoisonBlock(CodeGenFunction &CGF, unsigned layoutStartOffset,
+ unsigned layoutEndOffset) {
+ ASTContext &Context = CGF.getContext();
+ const ASTRecordLayout &Layout =
+ Context.getASTRecordLayout(Dtor->getParent());
+ llvm::ConstantInt *OffsetSizePtr;
+ llvm::Value *OffsetPtr;
+ CharUnits::QuantityType PoisonSize;
+
+ OffsetSizePtr = llvm::ConstantInt::get(
+ CGF.SizeTy,
+ Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset))
+ .getQuantity());
+
+ OffsetPtr = CGF.Builder.CreateGEP(
+ CGF.Builder.CreateBitCast(CGF.LoadCXXThis(), CGF.Int8PtrTy),
+ OffsetSizePtr);
+
+ if (layoutEndOffset >= Layout.getFieldCount() - 1) {
+ PoisonSize = Layout.getNonVirtualSize().getQuantity() -
+ Context.toCharUnitsFromBits(
+ Layout.getFieldOffset(layoutStartOffset))
+ .getQuantity();
+ } else {
+ PoisonSize = Context.toCharUnitsFromBits(
+ Layout.getFieldOffset(layoutEndOffset) -
+ Layout.getFieldOffset(layoutStartOffset))
+ .getQuantity();
+ }
+
+ // 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");
+ // Prevent the current stack frame from disappearing from the stack trace.
+ CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+
+ CGF.EmitNounwindRuntimeCall(Fn, Args);
+ }
+ };
}
/// \brief Emit all code that comes at the end of class's
@@ -1658,6 +1719,10 @@
/*BaseIsVirtual*/ false);
}
+ if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+ SanOpts.has(SanitizerKind::Memory))
+ EHStack.pushCleanup<SanitizeDtor>(NormalAndEHCleanup, DD);
+
// Destroy direct fields.
for (const auto *Field : ClassDecl->fields()) {
QualType type = Field->getType();
Index: lib/CodeGen/CGCXX.cpp
===================================================================
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -28,9 +28,24 @@
using namespace clang;
using namespace CodeGen;
+static bool HasFieldWithTrivialDestructor(CodeGenModule &CGM,
+ const CXXRecordDecl *D) {
+ for (const auto *Field : D->fields())
+ if (CGM.FieldHasTrivialDestructorBody(CGM.getContext(), Field))
+ return true;
+ return false;
+}
+
/// Try to emit a base destructor as an alias to its primary
/// base-class destructor.
bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
+ // If sanitizing memory to check for use-after-dtor, do not emit as
+ // an alias, unless it has no fields or has only fields with non-trivial
+ // destructors.
+ if (getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+ HasFieldWithTrivialDestructor(*this, D->getParent()))
+ return true;
+
if (!getCodeGenOpts().CXXCtorDtorAliases)
return true;
@@ -113,7 +128,12 @@
bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
GlobalDecl TargetDecl,
bool InEveryTU) {
- if (!getCodeGenOpts().CXXCtorDtorAliases)
+ // If sanitizing memory to check for use-after-dtor, do not emit as
+ // an alias, unless this class owns no members.
+ if (getCodeGenOpts().SanitizeMemoryUseAfterDtor)
+ return true;
+
+ if(!getCodeGenOpts().CXXCtorDtorAliases)
return true;
// The alias will use the linkage of the referent. If we can't
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
