nmusgrave updated this revision to Diff 33887. nmusgrave marked 4 inline comments as done. nmusgrave added a comment.
- Update comments, consistent style for attribute checking. 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 = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev 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 @@ -1099,6 +1099,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,9 +1328,8 @@ return true; } -static bool -FieldHasTrivialDestructorBody(ASTContext &Context, - const FieldDecl *Field) +bool CodeGenModule::FieldHasTrivialDestructorBody(ASTContext &Context, + const FieldDecl *Field) { QualType FieldBaseElementType = Context.getBaseElementType(Field->getType()); @@ -1353,66 +1348,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 +1425,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 +1441,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 +1484,7 @@ llvm::Value *ShouldDeleteCondition; public: CallDtorDeleteConditional(llvm::Value *ShouldDeleteCondition) - : ShouldDeleteCondition(ShouldDeleteCondition) { + : ShouldDeleteCondition(ShouldDeleteCondition) { assert(ShouldDeleteCondition != nullptr); } @@ -1571,8 +1514,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 +1529,106 @@ flags.isForNormalCleanup() && useEHCleanupForArray); } }; + + class SanitizeDtor final : 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 (Dtor->getParent()->field_empty()) + return; + + // Prevent the current stack frame from disappearing from the stack trace. + CGF.CurFn->addFnAttr("disable-tail-calls", "true"); + + // 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; + for (Field = Dtor->getParent()->field_begin(); + Field != Dtor->getParent()->field_end(); Field++) { + + // 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 == Layout.getFieldCount() - 1) { + PoisonBlock(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); + // 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()) { + 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"); + CGF.EmitNounwindRuntimeCall(Fn, Args); + } + }; } /// \brief Emit all code that comes at the end of class's @@ -1658,6 +1701,12 @@ /*BaseIsVirtual*/ false); } + // Poison fields such that access after their destructors are + // invoked, and before the base class destructor runs, is invalid. + 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,6 +28,14 @@ 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) { @@ -129,6 +137,14 @@ llvm::GlobalValue::isWeakForLinker(Linkage)) return true; + // If sanitizing memory to check for use-after-dtor, do not emit as + // an alias, unless this class owns no members with trivial destructors. + const CXXMethodDecl *MD = + cast<CXXMethodDecl>(AliasDecl.getDecl())->getCanonicalDecl(); + if (isa<CXXDestructorDecl>(MD) && getCodeGenOpts().SanitizeMemoryUseAfterDtor + && HasFieldWithTrivialDestructor(*this, MD->getParent())) + return true; + llvm::GlobalValue::LinkageTypes TargetLinkage = getFunctionLinkage(TargetDecl);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits