[ubsan] Add more -fsanitize-annotate-debug-info checks #141997
Conversation
This extends llvm#138577 to more UBSan checks. Note that the annotations are less detailed: they will always be __ubsan_check_singularity, rather than using the SanitizerKind (previous behavior, which is not always possible for all UBSan checks) or SanitizerHandler. This is a (minor) regression compared to llvm#128977 and llvm#139809. Updates the tests from llvm#128976, llvm#139149 and llvm#141814.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Thurston Dang (thurstond) ChangesThis extends #138577 to more UBSan checks, by changing SanitizerScope to add annotations if enabled for the specified ordinals. Note that the annotations are less detailed: they will always be Updates the tests from #128976, #139149 and #141814. Patch is 53.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141997.diff 16 Files Affected:
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 89b321090f2d8..b858770155472 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2006,10 +2006,11 @@ Value *CodeGenFunction::EmitCheckedArgForBuiltin(const Expr *E,
if (!SanOpts.has(SanitizerKind::Builtin))
return ArgValue;
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_Builtin;
+ SanitizerScope SanScope(this, {CheckOrdinal});
Value *Cond = Builder.CreateICmpNE(
ArgValue, llvm::Constant::getNullValue(ArgValue->getType()));
- EmitCheck(std::make_pair(Cond, SanitizerKind::SO_Builtin),
+ EmitCheck(std::make_pair(Cond, CheckOrdinal),
SanitizerHandler::InvalidBuiltin,
{EmitCheckSourceLocation(E->getExprLoc()),
llvm::ConstantInt::get(Builder.getInt8Ty(), Kind)},
@@ -2022,10 +2023,10 @@ Value *CodeGenFunction::EmitCheckedArgForAssume(const Expr *E) {
if (!SanOpts.has(SanitizerKind::Builtin))
return ArgValue;
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_Builtin;
+ SanitizerScope SanScope(this, {CheckOrdinal});
EmitCheck(
- std::make_pair(ArgValue, SanitizerKind::SO_Builtin),
- SanitizerHandler::InvalidBuiltin,
+ std::make_pair(ArgValue, CheckOrdinal), SanitizerHandler::InvalidBuiltin,
{EmitCheckSourceLocation(E->getExprLoc()),
llvm::ConstantInt::get(Builder.getInt8Ty(), BCK_AssumePassedFalse)},
std::nullopt);
@@ -2048,7 +2049,10 @@ static Value *EmitOverflowCheckedAbs(CodeGenFunction &CGF, const CallExpr *E,
return EmitAbs(CGF, ArgValue, true);
}
- CodeGenFunction::SanitizerScope SanScope(&CGF);
+ SmallVector<SanitizerKind::SanitizerOrdinal, 1> Ordinals;
+ if (SanitizeOverflow)
+ Ordinals.push_back(SanitizerKind::SO_SignedIntegerOverflow);
+ CodeGenFunction::SanitizerScope SanScope(&CGF, Ordinals);
Constant *Zero = Constant::getNullValue(ArgValue->getType());
Value *ResultAndOverflow = CGF.Builder.CreateBinaryIntrinsic(
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index bd920a2e3f2dd..fedabea72d600 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4156,7 +4156,7 @@ void CodeGenFunction::EmitReturnValueCheck(llvm::Value *RV) {
Handler = SanitizerHandler::NullabilityReturn;
}
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(this, {CheckKind});
// Make sure the "return" source location is valid. If we're checking a
// nullability annotation, make sure the preconditions for the check are met.
@@ -4541,7 +4541,7 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType,
Handler = SanitizerHandler::NullabilityArg;
}
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(this, {CheckKind});
llvm::Value *Cond = EmitNonNullRValueCheck(RV, ArgType);
llvm::Constant *StaticData[] = {
EmitCheckSourceLocation(ArgLoc),
@@ -5976,7 +5976,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
// attribute to insert handler calls.
if (SanOpts.hasOneOf(SanitizerKind::Address |
SanitizerKind::KernelAddress)) {
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(this, {});
llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
Builder.SetInsertPoint(CI);
auto *FnType = llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 251b059c256f6..70f885de05cf9 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -1678,7 +1678,7 @@ namespace {
static void EmitSanitizerDtorCallback(
CodeGenFunction &CGF, StringRef Name, llvm::Value *Ptr,
std::optional<CharUnits::QuantityType> PoisonSize = {}) {
- CodeGenFunction::SanitizerScope SanScope(&CGF);
+ CodeGenFunction::SanitizerScope SanScope(&CGF, {});
// Pass in void pointer and size of region as arguments to runtime
// function
SmallVector<llvm::Value *, 2> Args = {Ptr};
@@ -2885,7 +2885,7 @@ void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD,
SanitizerMask::bitPosToMask(M), TypeName))
return;
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(this, {M});
EmitSanitizerStatReport(SSK);
llvm::Metadata *MD =
@@ -2942,11 +2942,10 @@ bool CodeGenFunction::ShouldEmitVTableTypeCheckedLoad(const CXXRecordDecl *RD) {
llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad(
const CXXRecordDecl *RD, llvm::Value *VTable, llvm::Type *VTableTy,
uint64_t VTableByteOffset) {
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_CFIVCall;
+ SanitizerScope SanScope(this, {CheckOrdinal});
EmitSanitizerStatReport(llvm::SanStat_CFI_VCall);
- ApplyDebugLocation ApplyTrapDI(
- *this, SanitizerAnnotateDebugInfo(SanitizerKind::SO_CFIVCall));
llvm::Metadata *MD =
CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
@@ -2965,7 +2964,7 @@ llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad(
if (SanOpts.has(SanitizerKind::CFIVCall) &&
!getContext().getNoSanitizeList().containsType(SanitizerKind::CFIVCall,
TypeName)) {
- EmitCheck(std::make_pair(CheckResult, SanitizerKind::SO_CFIVCall),
+ EmitCheck(std::make_pair(CheckResult, CheckOrdinal),
SanitizerHandler::CFICheckFail, {}, {});
}
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index f4549ab3033b2..139cc567f101b 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -765,14 +765,15 @@ void CodeGenFunction::EmitNullabilityCheck(LValue LHS, llvm::Value *RHS,
// Check if the right hand side of the assignment is nonnull, if the left
// hand side must be nonnull.
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_NullabilityAssign;
+ SanitizerScope SanScope(this, {CheckOrdinal});
llvm::Value *IsNotNull = Builder.CreateIsNotNull(RHS);
llvm::Constant *StaticData[] = {
EmitCheckSourceLocation(Loc), EmitCheckTypeDescriptor(LHS.getType()),
llvm::ConstantInt::get(Int8Ty, 0), // The LogAlignment info is unused.
llvm::ConstantInt::get(Int8Ty, TCK_NonnullAssign)};
- EmitCheck({{IsNotNull, SanitizerKind::SO_NullabilityAssign}},
- SanitizerHandler::TypeMismatch, StaticData, RHS);
+ EmitCheck({{IsNotNull, CheckOrdinal}}, SanitizerHandler::TypeMismatch,
+ StaticData, RHS);
}
void CodeGenFunction::EmitScalarInit(const Expr *init, const ValueDecl *D,
@@ -2852,7 +2853,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, ParamValue Arg,
if (requiresReturnValueNullabilityCheck()) {
auto Nullability = Ty->getNullability();
if (Nullability && *Nullability == NullabilityKind::NonNull) {
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(this, {});
RetValNullabilityPrecondition =
Builder.CreateAnd(RetValNullabilityPrecondition,
Builder.CreateIsNotNull(Arg.getAnyValue()));
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index bae28b45afaa3..5bd782215b1b4 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -748,7 +748,9 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
if (Ty.isVolatileQualified())
return;
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(
+ this, {SanitizerKind::SO_Null, SanitizerKind::SO_ObjectSize,
+ SanitizerKind::SO_Alignment, SanitizerKind::SO_Vptr});
SmallVector<std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>, 3>
Checks;
@@ -989,7 +991,7 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction &CGF,
if (CE->getCastKind() == CK_ArrayToPointerDecay &&
!CE->getSubExpr()->isFlexibleArrayMemberLike(CGF.getContext(),
StrictFlexArraysLevel)) {
- CodeGenFunction::SanitizerScope SanScope(&CGF);
+ CodeGenFunction::SanitizerScope SanScope(&CGF, {});
IndexedType = CE->getSubExpr()->getType();
const ArrayType *AT = IndexedType->castAsArrayTypeUnsafe();
@@ -1002,7 +1004,7 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction &CGF,
}
}
- CodeGenFunction::SanitizerScope SanScope(&CGF);
+ CodeGenFunction::SanitizerScope SanScope(&CGF, {});
QualType EltTy{Base->getType()->getPointeeOrArrayElementType(), 0};
if (llvm::Value *POS = CGF.LoadPassedObjectSize(Base, EltTy)) {
@@ -1224,10 +1226,8 @@ void CodeGenFunction::EmitBoundsCheckImpl(const Expr *E, llvm::Value *Bound,
if (!Bound)
return;
- SanitizerScope SanScope(this);
-
auto CheckKind = SanitizerKind::SO_ArrayBounds;
- ApplyDebugLocation ApplyTrapDI(*this, SanitizerAnnotateDebugInfo(CheckKind));
+ SanitizerScope SanScope(this, {CheckKind});
bool IndexSigned = IndexType->isSignedIntegerOrEnumerationType();
llvm::Value *IndexVal = Builder.CreateIntCast(Index, SizeTy, IndexSigned);
@@ -1245,30 +1245,23 @@ void CodeGenFunction::EmitBoundsCheckImpl(const Expr *E, llvm::Value *Bound,
}
llvm::DILocation *CodeGenFunction::SanitizerAnnotateDebugInfo(
- SanitizerKind::SanitizerOrdinal CheckKindOrdinal) {
- std::string Label;
- switch (CheckKindOrdinal) {
-#define SANITIZER(NAME, ID) \
- case SanitizerKind::SO_##ID: \
- Label = "__ubsan_check_" NAME; \
- break;
-#include "clang/Basic/Sanitizers.def"
- default:
- llvm_unreachable("unexpected sanitizer kind");
- }
-
- // Sanitize label
- for (unsigned int i = 0; i < Label.length(); i++)
- if (!std::isalpha(Label[i]))
- Label[i] = '_';
-
+ ArrayRef<SanitizerKind::SanitizerOrdinal> Ordinals) {
llvm::DILocation *CheckDI = Builder.getCurrentDebugLocation();
- // TODO: deprecate ClArrayBoundsPseudoFn
- if (((ClArrayBoundsPseudoFn &&
- CheckKindOrdinal == SanitizerKind::SO_ArrayBounds) ||
- CGM.getCodeGenOpts().SanitizeAnnotateDebugInfo.has(CheckKindOrdinal)) &&
- CheckDI)
- CheckDI = getDebugInfo()->CreateSyntheticInlineAt(CheckDI, Label);
+
+ // TODO: the annotation could be more precise:
+ // 1) use the ordinal name if there is only one ordinal; and/or,
+ // 2) use the overarching SanitizerHandler if there are multiple ordinals
+ // (this may be confusing to users)
+ for (auto Ord : Ordinals) {
+ // TODO: deprecate ClArrayBoundsPseudoFn
+ if (((ClArrayBoundsPseudoFn && Ord == SanitizerKind::SO_ArrayBounds) ||
+ CGM.getCodeGenOpts().SanitizeAnnotateDebugInfo.has(Ord)) &&
+ CheckDI) {
+ CheckDI = getDebugInfo()->CreateSyntheticInlineAt(
+ CheckDI, "__ubsan_check_singularity");
+ break;
+ }
+ }
return CheckDI;
}
@@ -1994,8 +1987,11 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, IsBool))
return true;
+ SanitizerKind::SanitizerOrdinal Kind =
+ NeedsEnumCheck ? SanitizerKind::SO_Enum : SanitizerKind::SO_Bool;
+
auto &Ctx = getLLVMContext();
- SanitizerScope SanScope(this);
+ SanitizerScope SanScope(this, {Kind});
llvm::Value *Check;
--End;
if (!Min) {
@@ -2009,8 +2005,6 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
}
llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(Loc),
EmitCheckTypeDescriptor(Ty)};
- SanitizerKind::SanitizerOrdinal Kind =
- NeedsEnumCheck ? SanitizerKind::SO_Enum : SanitizerKind::SO_Bool;
EmitCheck(std::make_pair(Check, Kind), SanitizerHandler::LoadInvalidValue,
StaticArgs, Value);
return true;
@@ -3931,7 +3925,14 @@ void CodeGenFunction::EmitCfiCheckStub() {
// can be nullptr if the calling module has -fsanitize-trap behavior for this
// check kind; in this case __cfi_check_fail traps as well.
void CodeGenFunction::EmitCfiCheckFail() {
- SanitizerScope SanScope(this);
+ // TODO: the SanitizerKind is not yet determined for this check (and might
+ // not even be available, if Data == nullptr). However, we still want to
+ // annotate the instrumentation. We approximate this by using all the CFI
+ // kinds.
+ SanitizerScope SanScope(
+ this, {SanitizerKind::SO_CFIVCall, SanitizerKind::SO_CFINVCall,
+ SanitizerKind::SO_CFIDerivedCast,
+ SanitizerKind::SO_CFIUnrelatedCast, SanitizerKind::SO_CFIICall});
FunctionArgList Args;
ImplicitParamDecl ArgData(getContext(), getContext().VoidPtrTy,
ImplicitParamKind::Other);
@@ -3994,8 +3995,6 @@ void CodeGenFunction::EmitCfiCheckFail() {
{Addr, AllVtables}),
IntPtrTy);
- // TODO: the instructions above are not annotated with debug info. It is
- // inconvenient to do so because we have not determined SanitizerKind yet.
const std::pair<int, SanitizerKind::SanitizerOrdinal> CheckKinds[] = {
{CFITCK_VCall, SanitizerKind::SO_CFIVCall},
{CFITCK_NVCall, SanitizerKind::SO_CFINVCall},
@@ -4007,7 +4006,8 @@ void CodeGenFunction::EmitCfiCheckFail() {
int Kind = CheckKindOrdinalPair.first;
SanitizerKind::SanitizerOrdinal Ordinal = CheckKindOrdinalPair.second;
- ApplyDebugLocation ApplyTrapDI(*this, SanitizerAnnotateDebugInfo(Ordinal));
+ // TODO: we could apply SanitizerAnnotateDebugInfo(Ordinal) instead of
+ // relying on the SanitizerScope with all CFI ordinals
llvm::Value *Cond =
Builder.CreateICmpNE(CheckKind, llvm::ConstantInt::get(Int8Ty, Kind));
@@ -4030,9 +4030,10 @@ void CodeGenFunction::EmitCfiCheckFail() {
void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
if (SanOpts.has(SanitizerKind::Unreachable)) {
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_Unreachable;
+ SanitizerScope SanScope(this, {CheckOrdinal});
EmitCheck(std::make_pair(static_cast<llvm::Value *>(Builder.getFalse()),
- SanitizerKind::SO_Unreachable),
+ CheckOrdinal),
SanitizerHandler::BuiltinUnreachable,
EmitCheckSourceLocation(Loc), {});
}
@@ -6271,7 +6272,8 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType,
!isa<FunctionNoProtoType>(PointeeType)) {
if (llvm::Constant *PrefixSig =
CGM.getTargetCodeGenInfo().getUBSanFunctionSignature(CGM)) {
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_Function;
+ SanitizerScope SanScope(this, {CheckOrdinal});
auto *TypeHash = getUBSanFunctionTypeHash(PointeeType);
llvm::Type *PrefixSigType = PrefixSig->getType();
@@ -6331,7 +6333,7 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType,
Builder.CreateICmpEQ(CalleeTypeHash, TypeHash);
llvm::Constant *StaticData[] = {EmitCheckSourceLocation(E->getBeginLoc()),
EmitCheckTypeDescriptor(CalleeType)};
- EmitCheck(std::make_pair(CalleeTypeHashMatch, SanitizerKind::SO_Function),
+ EmitCheck(std::make_pair(CalleeTypeHashMatch, CheckOrdinal),
SanitizerHandler::FunctionTypeMismatch, StaticData,
{CalleePtr});
@@ -6350,10 +6352,9 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType,
// function pointer is a member of the bit set for the function type.
if (SanOpts.has(SanitizerKind::CFIICall) &&
(!TargetDecl || !isa<FunctionDecl>(TargetDecl))) {
- SanitizerScope SanScope(this);
+ auto CheckOrdinal = SanitizerKind::SO_CFIICall;
+ SanitizerScope SanScope(this, {CheckOrdinal});
EmitSanitizerStatReport(llvm::SanStat_CFI_ICall);
- ApplyDebugLocation ApplyTrapDI(
- *this, SanitizerAnnotateDebugInfo(SanitizerKind::SO_CFIICall));
llvm::Metadata *MD;
if (CGM.getCodeGenOpts().SanitizeCfiICallGeneralizePointers)
@@ -6374,10 +6375,10 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType,
EmitCheckTypeDescriptor(QualType(FnType, 0)),
};
if (CGM.getCodeGenOpts().SanitizeCfiCrossDso && CrossDsoTypeId) {
- EmitCfiSlowPathCheck(SanitizerKind::SO_CFIICall, TypeTest, CrossDsoTypeId,
- CalleePtr, StaticData);
+ EmitCfiSlowPathCheck(CheckOrdinal, TypeTest, CrossDsoTypeId, CalleePtr,
+ StaticData);
} else {
- EmitCheck(std::make_pair(TypeTest, SanitizerKind::SO_CFIICall),
+ EmitCheck(std::make_pair(TypeTest, CheckOrdinal),
SanitizerHandler::CFICheckFail, StaticData,
{CalleePtr, llvm::UndefValue::get(IntPtrTy)});
}
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 111b5805c6a94..43ac70bd81b7c 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -999,7 +999,8 @@ void ScalarExprEmitter::EmitFloatConversionCheck(
if (!isa<llvm::IntegerType>(DstTy))
return;
- CodeGenFunction::SanitizerScope SanScope(&CGF);
+ auto CheckOrdinal = SanitizerKind::SO_FloatCastOverflow;
+ CodeGenFunction::SanitizerScope SanScope(&CGF, {CheckOrdinal});
using llvm::APFloat;
using llvm::APSInt;
@@ -1056,7 +1057,7 @@ void ScalarExprEmitter::EmitFloatConversionCheck(
llvm::Constant *StaticArgs[] = {CGF.EmitCheckSourceLocation(Loc),
CGF.EmitCheckTypeDescriptor(OrigSrcType),
CGF.EmitCheckTypeDescriptor(DstType)};
- CGF.EmitCheck(std::make_pair(Check, SanitizerKind::SO_FloatCastOverflow),
+ CGF.EmitCheck(std::make_pair(Check, CheckOrdinal),
SanitizerHandler::FloatCastOverflow, StaticArgs, OrigSrc);
}
@@ -1134,18 +1135,28 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value *Src, QualType SrcType,
(!SrcSigned && DstSigned))
return;
- CodeGenFunction::SanitizerScope SanScope(&CGF);
-
std::pair<ScalarExprEmitter::ImplicitConversionCheckKind,
std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>>
- Check =
- EmitIntegerTruncationCheckHelper(Src, SrcType, Dst, DstType, Builder);
- // If the comparison result is 'i1 false', then the truncation was lossy.
+ Check;
+
+ {
+ // We don't know the check kind until we call
+ // EmitIntegerTruncationCheckHelper, but we want to annotate
+ // EmitIntegerTruncationCheckHelper's instructions too.
+ CodeGenFunction::SanitizerScope SanScope(
+ &CGF, {SanitizerKind::SO_ImplicitUnsignedIntegerTruncation,
+ SanitizerKind::SO_ImplicitSignedIntegerTruncation});
+ Check =
+ EmitIntegerTruncationCheckHelper(Src, SrcType, Dst, DstType, Builder);
+ // If the comparison result is 'i1 false', then the truncation was lossy.
+ }
// Do we care about this type of truncation?
if (!CGF.SanOpts.has(Check.second.second))
return;
+ CodeGenFunction::SanitizerScope SanScope(&CGF, {Check.second.second});
+
// Does some SSCL ignore this type?
if (CGF.getContext().isTypeIgnoredBySanitizer(
SanitizerMask::bitPosToMask(Check.second.second), DstType))
@@ -1272,7 +1283,9 @@ void ScalarExprEmitter::EmitIntegerSignChangeCheck(Value *Src, QualType SrcType,
return;
// That's it. We can't rule out any more cases with the data we have.
- CodeGenFunction::SanitizerScope SanScope(&CGF);
+ CodeGenFunction::SanitizerScope SanScope(
+ &CGF, {SanitizerKind::SO_ImplicitUnsignedIntegerTruncation,
+ SanitizerKind::SO_ImplicitSignedIntegerTruncation});
std::pair<ScalarExprEmitter::ImplicitConversionCheckKind,
std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>>
@@ -1393,7 +1406,8 @@ void CodeGenFunction::EmitBitfieldConversionCheck(Value *Src, QualType SrcType,
bool SrcSigned = SrcType->isSignedIntegerOrEnumerationType();
bool DstSigned = DstType->isSignedIntegerOrEnumerationType();
- CodeGenFunction::SanitizerScope SanScope(this);
+ CodeGenFunction::SanitizerScope SanScope(
+ this, {SanitizerKind::SO_ImplicitBitfieldConversion});
std::pair<ScalarExprEmitter::ImplicitConversionCheckKind,
std...
[truncated]
|
I've now changed the annotations to use the handler name e.g., __ubsan_check_out_of_bounds, __ubsan_check_cfi_check_fail, __ubsan_check_type_mismatch. |
| CodeGenFunction *CGF, ArrayRef<SanitizerKind::SanitizerOrdinal> Ordinals, | ||
| SanitizerHandler Handler) | ||
| : SanitizerScope(CGF) { | ||
| this->ApplyTrapDI = new ApplyDebugLocation( |
There was a problem hiding this comment.
Let's not do manual memory management. Use a unique_ptr and forward declared class, that should work because this class has a explicit destructor.
Also drop all the this
There was a problem hiding this comment.
There was a problem hiding this comment.
But why do we need heap allocation here?
There was a problem hiding this comment.
std::make_unuque<>
Done
But why do we need heap allocation here?
ApplyDebugLocation is scoped.
There was a problem hiding this comment.
I guess right answer is to avoid include, which probably achievable in other way.
I guess we don't even want to annotate traps. Now we annotate because how scoped thing work, |
Why don't we want to annotate traps? |
We annotate for profiling. Traps executed rarely, once per process. So they are not interested for us. Only conditions used for check. |
But ideally both should work. Our inline frame + message frame should work. |
Looks like we still want annotations on trap. I am not sure what is needed to make those frames to produce LLDB recognizable frame. |
While logically correct I think we might have to teach LLDB how to handle this. LLDB has a special "frame recognizer" where it looks for frames using this fake debug info mechanism (e.g. @MiB137 Can probably say more here. |
@MiB137 is better positioned to comment on this than me. It's probably possible to make LLDB work with which ever top frame we decide but we'd probably want the fake frame names be easily recognizable and stable so that LLDB's feature continues to work reliably. I personally think having the user readable message (i.e the Given that |
…bugLocation" This reverts commit ff5ffbf. Already in sanitizerscope
remove redundant SanitizerDebugLocation from EmitVTablePtrCheck
|
@Michael137 Ping |
Yea everything that @delcypher said here is accurate from the LLDB-side. Here's the logic that LLDB uses to pick which frame to display: llvm-project/lldb/source/Target/VerboseTrapFrameRecognizer.cpp Lines 22 to 50 in 9ba332f Currently we pick the frame just above the fake inlined frame (and skip over any
Agreed, for the llvm-project/lldb/source/Target/VerboseTrapFrameRecognizer.cpp Lines 145 to 146 in 9ba332f We'll want a similar prefix for any other fake frames that LLDB is supposed to recognize
Agreed |
|
Should we just rename stuff of this patch into __clang_trap_msg ? |
Actually no. _clang_trap_msg* is only for trap. Attaching _clang_trap_msg* to evaluation code probably not nice. Now we can attach _ubsan_check* to both - eval and trap. I assume _clang_trap_msg* can be as precise as _ubsan_check, so we can switch to that one from _ubsan_check for crash reports. I guess we can land this one, as we can see clear way forward with both features? |
Agreed.
That seems reasonable to me. as long as the
By "switch to" do you mean remove
SGTM. |
manually resolve conflict)
This extends #138577 to more UBSan checks, by changing SanitizerDebugLocation (formerly SanitizerScope) to add annotations if enabled for the specified ordinals.
Annotations will use the ordinal name if there is exactly one ordinal specified in the SanitizerDebugLocation; otherwise, it will use the handler name.
Updates the tests from #141814.