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
This commit is contained in:
Evgenii Stepanov 2022-03-15 18:43:38 -07:00 committed by Vitaly Buka
parent c5ea8e9138
commit cb96464f12
3 changed files with 105 additions and 83 deletions

View File

@ -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

View File

@ -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

View File

@ -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