This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc5ea8e913893: Use-after-dtor detection for trivial base
classes. (authored by eugenis, committed by vitalybuka).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119300/new/
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,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<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 @@
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 @@
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
