eugenis created this revision.
eugenis added reviewers: kda, vitalybuka.
eugenis requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added a subscriber: Sanitizers.
-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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119300
Files:
clang/lib/CodeGen/CGClass.cpp
clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp
compiler-rt/test/msan/dtor-base-access.cpp
Index: compiler-rt/test/msan/dtor-base-access.cpp
===================================================================
--- compiler-rt/test/msan/dtor-base-access.cpp
+++ 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;
}
Index: clang/test/CodeGenCXX/sanitize-dtor-trivial-base.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/lib/CodeGen/CGClass.cpp
===================================================================
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -1666,6 +1666,31 @@
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());
+ }
+ };
+
class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
const CXXDestructorDecl *Dtor;
@@ -1842,12 +1867,19 @@
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))
+ EHStack.pushCleanup<SanitizeDtorTrivialBase>(NormalAndEHCleanup,
+ BaseClassDecl,
+ /*BaseIsVirtual*/ true);
+ } else {
+ EHStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup, BaseClassDecl,
+ /*BaseIsVirtual*/ true);
+ }
}
return;
@@ -1870,12 +1902,16 @@
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))
+ EHStack.pushCleanup<SanitizeDtorTrivialBase>(NormalAndEHCleanup,
+ BaseClassDecl,
+ /*BaseIsVirtual*/ false);
+ } else {
+ EHStack.pushCleanup<CallBaseDtor>(NormalAndEHCleanup, BaseClassDecl,
+ /*BaseIsVirtual*/ false);
+ }
}
// Poison fields such that access after their destructors are
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
