diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -1666,6 +1666,34 @@ CGF.EmitNounwindRuntimeCall(Fn, Args); } + /// Poison base class with a trivial destructor. + struct SanitizeDtorTrivialBase final : EHScopeStack::Cleanup { + const CXXRecordDecl *BaseClass; + bool BaseIsVirtual; + SanitizeDtorTrivialBase(const CXXRecordDecl *Base, bool BaseIsVirtual) + : BaseClass(Base), BaseIsVirtual(BaseIsVirtual) {} + + void Emit(CodeGenFunction &CGF, Flags flags) override { + const CXXRecordDecl *DerivedClass = + cast(CGF.CurCodeDecl)->getParent(); + + Address Addr = CGF.GetAddressOfDirectBaseInCompleteClass( + CGF.LoadCXXThisAddress(), DerivedClass, BaseClass, BaseIsVirtual); + + const ASTRecordLayout &BaseLayout = + CGF.getContext().getASTRecordLayout(BaseClass); + CharUnits BaseSize = BaseLayout.getSize(); + + if (!BaseSize.isPositive()) + return; + + EmitSanitizerDtorCallback(CGF, Addr.getPointer(), BaseSize.getQuantity()); + + // Prevent the current stack frame from disappearing from the stack trace. + CGF.CurFn->addFnAttr("disable-tail-calls", "true"); + } + }; + class SanitizeDtorMembers final : public EHScopeStack::Cleanup { const CXXDestructorDecl *Dtor; @@ -1841,13 +1869,19 @@ auto *BaseClassDecl = cast(Base.getType()->castAs()->getDecl()); - // Ignore trivial destructors. - if (BaseClassDecl->hasTrivialDestructor()) - continue; - - EHStack.pushCleanup(NormalAndEHCleanup, - BaseClassDecl, - /*BaseIsVirtual*/ true); + if (BaseClassDecl->hasTrivialDestructor()) { + // Under SanitizeMemoryUseAfterDtor, poison the trivial base class + // memory. For non-trival base classes the same is done in the class + // destructor. + if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor && + SanOpts.has(SanitizerKind::Memory) && !BaseClassDecl->isEmpty()) + EHStack.pushCleanup(NormalAndEHCleanup, + BaseClassDecl, + /*BaseIsVirtual*/ true); + } else { + EHStack.pushCleanup(NormalAndEHCleanup, BaseClassDecl, + /*BaseIsVirtual*/ true); + } } return; @@ -1869,13 +1903,16 @@ CXXRecordDecl *BaseClassDecl = Base.getType()->getAsCXXRecordDecl(); - // Ignore trivial destructors. - if (BaseClassDecl->hasTrivialDestructor()) - continue; - - EHStack.pushCleanup(NormalAndEHCleanup, - BaseClassDecl, - /*BaseIsVirtual*/ false); + if (BaseClassDecl->hasTrivialDestructor()) { + if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor && + SanOpts.has(SanitizerKind::Memory) && !BaseClassDecl->isEmpty()) + EHStack.pushCleanup(NormalAndEHCleanup, + BaseClassDecl, + /*BaseIsVirtual*/ false); + } else { + EHStack.pushCleanup(NormalAndEHCleanup, BaseClassDecl, + /*BaseIsVirtual*/ false); + } } // Poison fields such that access after their destructors are diff --git a/clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp b/clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-passes -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-passes -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s + +// Base class has trivial dtor => complete dtor poisons base class memory directly. + +class Base { +public: + int x[4]; +}; + +class Derived : public Base { +public: + int y; + ~Derived() { + } +}; + +Derived d; + +// Poison members, then poison the trivial base class. +// CHECK-LABEL: define {{.*}}DerivedD2Ev +// CHECK: %[[GEP:[0-9a-z]+]] = getelementptr i8, i8* {{.*}}, i64 16 +// CHECK: call void @__sanitizer_dtor_callback{{.*}}%[[GEP]], i64 4 +// CHECK: call void @__sanitizer_dtor_callback{{.*}}, i64 16 +// CHECK: ret void diff --git a/compiler-rt/test/msan/dtor-base-access.cpp b/compiler-rt/test/msan/dtor-base-access.cpp --- a/compiler-rt/test/msan/dtor-base-access.cpp +++ b/compiler-rt/test/msan/dtor-base-access.cpp @@ -9,41 +9,62 @@ class Base { public: - int *x_ptr; - Base(int *y_ptr) { - // store value of subclass member - x_ptr = y_ptr; - } - virtual ~Base(); + int b; + Base() { b = 1; } + ~Base(); }; -class Derived : public Base { - public: - int y; - Derived():Base(&y) { - y = 10; - } +class TrivialBaseBefore { +public: + int tb0; + TrivialBaseBefore() { tb0 = 1; } +}; + +class TrivialBaseAfter { +public: + int tb1; + TrivialBaseAfter() { tb1 = 1; } +}; + +class Derived : public TrivialBaseBefore, public Base, public TrivialBaseAfter { +public: + int d; + Derived() { d = 1; } ~Derived(); }; +Derived *g; + Base::~Base() { - // ok access its own member - assert(__msan_test_shadow(&this->x_ptr, sizeof(this->x_ptr)) == -1); - // bad access subclass member - assert(__msan_test_shadow(this->x_ptr, sizeof(*this->x_ptr)) != -1); + // ok to access its own members and earlier bases + assert(__msan_test_shadow(&g->tb0, sizeof(g->tb0)) == -1); + assert(__msan_test_shadow(&g->b, sizeof(g->b)) == -1); + // not ok to access others + assert(__msan_test_shadow(&g->tb1, sizeof(g->tb1)) == 0); + assert(__msan_test_shadow(&g->d, sizeof(g->d)) == 0); } Derived::~Derived() { - // ok to access its own members - assert(__msan_test_shadow(&this->y, sizeof(this->y)) == -1); - // ok access base class members - assert(__msan_test_shadow(&this->x_ptr, sizeof(this->x_ptr)) == -1); + // ok to access everything + assert(__msan_test_shadow(&g->tb0, sizeof(g->tb0)) == -1); + assert(__msan_test_shadow(&g->b, sizeof(g->b)) == -1); + assert(__msan_test_shadow(&g->tb1, sizeof(g->tb1)) == -1); + assert(__msan_test_shadow(&g->d, sizeof(g->d)) == -1); } int main() { - Derived *d = new Derived(); - assert(__msan_test_shadow(&d->x_ptr, sizeof(d->x_ptr)) == -1); - d->~Derived(); - assert(__msan_test_shadow(&d->x_ptr, sizeof(d->x_ptr)) != -1); + g = new Derived(); + // ok to access everything + assert(__msan_test_shadow(&g->tb0, sizeof(g->tb0)) == -1); + assert(__msan_test_shadow(&g->b, sizeof(g->b)) == -1); + assert(__msan_test_shadow(&g->tb1, sizeof(g->tb1)) == -1); + assert(__msan_test_shadow(&g->d, sizeof(g->d)) == -1); + + g->~Derived(); + // not ok to access everything + assert(__msan_test_shadow(&g->tb0, sizeof(g->tb0)) == 0); + assert(__msan_test_shadow(&g->b, sizeof(g->b)) == 0); + assert(__msan_test_shadow(&g->tb1, sizeof(g->tb1)) == 0); + assert(__msan_test_shadow(&g->d, sizeof(g->d)) == 0); return 0; }