[compiler-rt] c5ea8e9 - Use-after-dtor detection for trivial base classes.
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 16 18:20:45 PDT 2022
Author: Evgenii Stepanov
Date: 2022-03-16T18:20:27-07:00
New Revision: c5ea8e9138931b74b60221f667da304e244dc57d
URL: https://github.com/llvm/llvm-project/commit/c5ea8e9138931b74b60221f667da304e244dc57d
DIFF: https://github.com/llvm/llvm-project/commit/c5ea8e9138931b74b60221f667da304e244dc57d.diff
LOG: Use-after-dtor detection for trivial base classes.
-fsanitize-memory-use-after-dtor detects memory access after a
subobject is destroyed but its memory is not yet deallocated.
This is done by poisoning each object memory near the end of its destructor.
Subobjects (members and base classes) do this in their respective
destructors, and the parent class does the same for its members with
trivial destructors.
Inexplicably, base classes with trivial destructors are not handled at
all. This change fixes this oversight by adding the base class poisoning logic
to the parent class destructor.
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D119300
Added:
clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp
Modified:
clang/lib/CodeGen/CGClass.cpp
compiler-rt/test/msan/dtor-base-access.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 4a49fc4a80c58..5adf7ea99391e 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -1666,6 +1666,34 @@ namespace {
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<CXXMethodDecl>(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 @@ void CodeGenFunction::EnterDtorCleanups(const CXXDestructorDecl *DD,
auto *BaseClassDecl =
cast<CXXRecordDecl>(Base.getType()->castAs<RecordType>()->getDecl());
- // Ignore trivial destructors.
- if (BaseClassDecl->hasTrivialDestructor())
- continue;
-
- EHStack.pushCleanup<CallBaseDtor>(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<SanitizeDtorTrivialBase>(NormalAndEHCleanup,
+ BaseClassDecl,
+ /*BaseIsVirtual*/ true);
+ } else {
+ EHStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup, BaseClassDecl,
+ /*BaseIsVirtual*/ true);
+ }
}
return;
@@ -1869,13 +1903,16 @@ void CodeGenFunction::EnterDtorCleanups(const CXXDestructorDecl *DD,
CXXRecordDecl *BaseClassDecl = Base.getType()->getAsCXXRecordDecl();
- // Ignore trivial destructors.
- if (BaseClassDecl->hasTrivialDestructor())
- continue;
-
- EHStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup,
- BaseClassDecl,
- /*BaseIsVirtual*/ false);
+ if (BaseClassDecl->hasTrivialDestructor()) {
+ if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+ SanOpts.has(SanitizerKind::Memory) && !BaseClassDecl->isEmpty())
+ EHStack.pushCleanup<SanitizeDtorTrivialBase>(NormalAndEHCleanup,
+ BaseClassDecl,
+ /*BaseIsVirtual*/ false);
+ } else {
+ EHStack.pushCleanup<CallBaseDtor>(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
index 0000000000000..ec560d533a145
--- /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
index bed66fb621db0..cface9919c9b2 100644
--- 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;
}
More information about the llvm-commits
mailing list