[clang] cb96464 - Stricter use-after-dtor detection for trivial members.
Vitaly Buka via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 16 18:20:47 PDT 2022
Author: Evgenii Stepanov
Date: 2022-03-16T18:20:27-07:00
New Revision: cb96464f12c44320150c48d639070cf0e4fd8bd2
URL: https://github.com/llvm/llvm-project/commit/cb96464f12c44320150c48d639070cf0e4fd8bd2
DIFF: https://github.com/llvm/llvm-project/commit/cb96464f12c44320150c48d639070cf0e4fd8bd2.diff
LOG: Stricter use-after-dtor detection for trivial members.
Poison trivial class members one-by-one in the reverse order of their
construction, instead of all-at-once at the very end.
For example, in the following code access to `x` from `~B` will
produce an undefined value.
struct A {
struct B b;
int x;
};
Reviewed By: kda
Differential Revision: https://reviews.llvm.org/D119600
Added:
Modified:
clang/lib/CodeGen/CGClass.cpp
clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 5adf7ea99391e..cb16019c63508 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -1694,75 +1694,28 @@ namespace {
}
};
- class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
+ class SanitizeDtorFieldRange final : public EHScopeStack::Cleanup {
const CXXDestructorDecl *Dtor;
+ unsigned StartIndex;
+ unsigned EndIndex;
public:
- SanitizeDtorMembers(const CXXDestructorDecl *Dtor) : Dtor(Dtor) {}
+ SanitizeDtorFieldRange(const CXXDestructorDecl *Dtor, unsigned StartIndex,
+ unsigned EndIndex)
+ : Dtor(Dtor), StartIndex(StartIndex), EndIndex(EndIndex) {}
// 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 (Layout.getFieldCount() == 0)
- 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();
-
- const RecordDecl *Decl = Dtor->getParent();
- auto Fields = Decl->fields();
- auto IsTrivial = [&](const FieldDecl *F) {
- return FieldHasTrivialDestructorBody(Context, F);
- };
-
- auto IsZeroSize = [&](const FieldDecl *F) {
- return F->isZeroSize(Context);
- };
-
- // Poison blocks of fields with trivial destructors making sure that block
- // begin and end do not point to zero-sized fields. They don't have
- // correct offsets so can't be used to calculate poisoning range.
- for (auto It = Fields.begin(); It != Fields.end();) {
- It = std::find_if(It, Fields.end(), [&](const FieldDecl *F) {
- return IsTrivial(F) && !IsZeroSize(F);
- });
- if (It == Fields.end())
- break;
- auto Start = It++;
- It = std::find_if(It, Fields.end(), [&](const FieldDecl *F) {
- return !IsTrivial(F) && !IsZeroSize(F);
- });
-
- PoisonMembers(CGF, (*Start)->getFieldIndex(),
- It == Fields.end() ? -1 : (*It)->getFieldIndex());
- }
- }
-
- private:
- /// \param layoutStartOffset index of the ASTRecordLayout field to
- /// start poisoning (inclusive)
- /// \param layoutEndOffset index of the ASTRecordLayout field to
- /// end poisoning (exclusive)
- void PoisonMembers(CodeGenFunction &CGF, unsigned layoutStartOffset,
- unsigned layoutEndOffset) {
- ASTContext &Context = CGF.getContext();
+ const ASTContext &Context = CGF.getContext();
const ASTRecordLayout &Layout =
Context.getASTRecordLayout(Dtor->getParent());
- // It's a first trivia field so it should be at the begining of char,
+ // It's a first trivial field so it should be at the begining of a char,
// still round up start offset just in case.
- CharUnits PoisonStart =
- Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) +
- Context.getCharWidth() - 1);
+ CharUnits PoisonStart = Context.toCharUnitsFromBits(
+ Layout.getFieldOffset(StartIndex) + Context.getCharWidth() - 1);
llvm::ConstantInt *OffsetSizePtr =
llvm::ConstantInt::get(CGF.SizeTy, PoisonStart.getQuantity());
@@ -1772,17 +1725,20 @@ namespace {
OffsetSizePtr);
CharUnits PoisonEnd;
- if (layoutEndOffset >= Layout.getFieldCount()) {
+ if (EndIndex >= Layout.getFieldCount()) {
PoisonEnd = Layout.getNonVirtualSize();
} else {
PoisonEnd =
- Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutEndOffset));
+ Context.toCharUnitsFromBits(Layout.getFieldOffset(EndIndex));
}
CharUnits PoisonSize = PoisonEnd - PoisonStart;
if (!PoisonSize.isPositive())
return;
EmitSanitizerDtorCallback(CGF, OffsetPtr, PoisonSize.getQuantity());
+
+ // Prevent the current stack frame from disappearing from the stack trace.
+ CGF.CurFn->addFnAttr("disable-tail-calls", "true");
}
};
@@ -1807,6 +1763,36 @@ namespace {
EmitSanitizerDtorCallback(CGF, VTablePtr, PoisonSize);
}
};
+
+ class SanitizeDtorCleanupBuilder {
+ ASTContext &Context;
+ EHScopeStack &EHStack;
+ const CXXDestructorDecl *DD;
+ llvm::Optional<unsigned> StartIndex;
+
+ public:
+ SanitizeDtorCleanupBuilder(ASTContext &Context, EHScopeStack &EHStack,
+ const CXXDestructorDecl *DD)
+ : Context(Context), EHStack(EHStack), DD(DD), StartIndex(llvm::None) {}
+ void PushCleanupForField(const FieldDecl *Field) {
+ if (Field->isZeroSize(Context))
+ return;
+ unsigned FieldIndex = Field->getFieldIndex();
+ if (FieldHasTrivialDestructorBody(Context, Field)) {
+ if (!StartIndex)
+ StartIndex = FieldIndex;
+ } else if (StartIndex) {
+ EHStack.pushCleanup<SanitizeDtorFieldRange>(
+ NormalAndEHCleanup, DD, StartIndex.getValue(), FieldIndex);
+ StartIndex = None;
+ }
+ }
+ void End() {
+ if (StartIndex)
+ EHStack.pushCleanup<SanitizeDtorFieldRange>(NormalAndEHCleanup, DD,
+ StartIndex.getValue(), -1);
+ }
+ };
} // end anonymous namespace
/// Emit all code that comes at the end of class's
@@ -1917,25 +1903,32 @@ void CodeGenFunction::EnterDtorCleanups(const CXXDestructorDecl *DD,
// 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<SanitizeDtorMembers>(NormalAndEHCleanup, DD);
+ bool SanitizeFields = CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+ SanOpts.has(SanitizerKind::Memory);
+ SanitizeDtorCleanupBuilder SanitizeBuilder(getContext(), EHStack, DD);
// Destroy direct fields.
for (const auto *Field : ClassDecl->fields()) {
+ if (SanitizeFields)
+ SanitizeBuilder.PushCleanupForField(Field);
+
QualType type = Field->getType();
QualType::DestructionKind dtorKind = type.isDestructedType();
- if (!dtorKind) continue;
+ if (!dtorKind)
+ continue;
// Anonymous union members do not have their destructors called.
const RecordType *RT = type->getAsUnionType();
- if (RT && RT->getDecl()->isAnonymousStructOrUnion()) continue;
+ if (RT && RT->getDecl()->isAnonymousStructOrUnion())
+ continue;
CleanupKind cleanupKind = getCleanupKind(dtorKind);
- EHStack.pushCleanup<DestroyField>(cleanupKind, Field,
- getDestroyer(dtorKind),
- cleanupKind & EHCleanup);
+ EHStack.pushCleanup<DestroyField>(
+ cleanupKind, Field, getDestroyer(dtorKind), cleanupKind & EHCleanup);
}
+
+ if (SanitizeFields)
+ SanitizeBuilder.End();
}
/// EmitCXXAggrConstructorCall - Emit a loop to call a particular
diff --git a/clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp b/clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
index 8c5cc631becc1..bd9d3493804bc 100644
--- a/clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
+++ b/clang/test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -70,8 +70,8 @@ Derived d;
// 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 {{.*}}ZN6VectorIiED1Ev
// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
// CHECK: call void {{.*}}ZN4BaseD2Ev
// CHECK: ret void
diff --git a/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp b/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
index 74ba612f99224..c7bd5ad17cb26 100644
--- a/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
+++ b/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
@@ -44,7 +44,9 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T1
// CHECK-LABEL: define {{.*}} @_ZN5empty2T16StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
+// CHECK: [[GEP:%.+]] = getelementptr i8, {{.*}}, i64 8{{$}}
+// CHECK: call void @__sanitizer_dtor_callback(i8* [[GEP]], i64 13)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T2 {
@@ -57,8 +59,11 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T2
// CHECK-LABEL: define {{.*}} @_ZN5empty2T26StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
+// CHECK: [[GEP1:%.+]] = getelementptr i8, {{.*}}, i64 16{{$}}
+// CHECK: call void @__sanitizer_dtor_callback(i8* [[GEP1]], i64 5)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: [[GEP2:%.+]] = getelementptr i8, {{.*}}, i64 0{{$}}
+// CHECK: call void @__sanitizer_dtor_callback(i8* [[GEP2]], i64 8)
// CHECK-NEXT: ret void
namespace T3 {
@@ -71,8 +76,11 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T3
// CHECK-LABEL: define {{.*}} @_ZN5empty2T36StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
+// CHECK: [[GEP1:%.+]] = getelementptr i8, {{.*}}, i64 20{{$}}
+// CHECK: call void @__sanitizer_dtor_callback(i8* [[GEP1]], i64 1)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: [[GEP2:%.+]] = getelementptr i8, {{.*}}, i64 0{{$}}
+// CHECK: call void @__sanitizer_dtor_callback(i8* [[GEP2]], i64 12)
// CHECK-NEXT: ret void
namespace T4 {
@@ -100,6 +108,7 @@ static_assert(sizeof(Struct) == 24);
} // namespace T5
// CHECK-LABEL: define {{.*}} @_ZN5empty2T56StructD2Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T6 {
@@ -114,6 +123,7 @@ static_assert(sizeof(Struct) == 24);
} // namespace T6
// CHECK-LABEL: define {{.*}} @_ZN5empty2T66StructD2Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T7 {
@@ -127,8 +137,9 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T7
// CHECK-LABEL: define {{.*}} @_ZN5empty2T76StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK-NEXT: ret void
namespace T8 {
@@ -142,8 +153,8 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T8
// CHECK-LABEL: define {{.*}} @_ZN5empty2T86StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK-NEXT: ret void
namespace T9 {
@@ -157,8 +168,8 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T9
// CHECK-LABEL: define {{.*}} @_ZN5empty2T96StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK-NEXT: ret void
namespace T10 {
@@ -172,8 +183,8 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T10
// CHECK-LABEL: define {{.*}} @_ZN5empty3T106StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK-NEXT: ret void
namespace T11 {
@@ -202,6 +213,7 @@ static_assert(sizeof(Struct) == 24);
} // namespace T12
} // namespace empty
// CHECK-LABEL: define {{.*}} @_ZN5empty3T126StructD2Ev(
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 16)
// CHECK-NEXT: ret void
@@ -217,6 +229,7 @@ static_assert(sizeof(Struct) == 24);
} // namespace T1
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T16StructD2Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T2 {
@@ -229,8 +242,8 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T2
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T26StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK-NEXT: ret void
namespace T3 {
@@ -243,8 +256,8 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T3
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T36StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK-NEXT: ret void
namespace T4 {
@@ -272,6 +285,8 @@ static_assert(sizeof(Struct) == 24);
} // namespace T5
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T56StructD2Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T6 {
@@ -286,6 +301,8 @@ static_assert(sizeof(Struct) == 24);
} // namespace T6
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T66StructD2Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T7 {
@@ -299,8 +316,10 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T7
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T76StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK-NEXT: ret void
namespace T8 {
@@ -314,8 +333,10 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T8
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T86StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T9 {
@@ -329,8 +350,10 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T9
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T96StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK-NEXT: ret void
namespace T10 {
@@ -344,8 +367,10 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T10
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial3T106StructD2Ev(
-// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
+// CHECK: call void @_ZN10NonTrivialD1Ev(
+// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
// CHECK-NEXT: ret void
namespace T11 {
@@ -359,6 +384,8 @@ struct Struct {
static_assert(sizeof(Struct) == 24);
} // namespace T11
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial3T116StructD2Ev(
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 16)
// CHECK-NEXT: ret void
@@ -374,5 +401,7 @@ static_assert(sizeof(Struct) == 24);
} // namespace T12
} // namespace empty_non_trivial
// CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial3T126StructD2Ev(
+// CHECK: call void @_ZN10NonTrivialD1Ev(
// CHECK: call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 16)
+// CHECK: call void @_ZN15EmptyNonTrivialD1Ev(
// CHECK: ret void
More information about the cfe-commits
mailing list