Skip to content

Split TypeMismatch UBSan handler into 4 handlers #151243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions clang/lib/CodeGen/CGDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -767,12 +767,11 @@ 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.
auto CheckOrdinal = SanitizerKind::SO_NullabilityAssign;
auto CheckHandler = SanitizerHandler::TypeMismatch;
auto CheckHandler = SanitizerHandler::NullPointerUseWithNullability;
SanitizerDebugLocation SanScope(this, {CheckOrdinal}, CheckHandler);
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, CheckOrdinal}}, CheckHandler, StaticData, RHS);
}
Expand Down
59 changes: 35 additions & 24 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,18 +766,13 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
llvm::BasicBlock *Done = nullptr;
bool DoneViaNullSanitize = false;

llvm::Value *True = llvm::ConstantInt::getTrue(getLLVMContext());

{
auto CheckHandler = SanitizerHandler::TypeMismatch;
SanitizerDebugLocation SanScope(this,
{SanitizerKind::SO_Null,
SanitizerKind::SO_ObjectSize,
SanitizerKind::SO_Alignment},
auto CheckHandler = SanitizerHandler::NullPointerUse;
SanitizerDebugLocation SanScope(this, {SanitizerKind::SO_Null},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: assign the SanitizerKind to a variable, so that it can be reused between the SanScope and EmitCheck.
(Prior to this change, that wasn't possible because the SanScope had multiple SanitizerKinds but the EmitCheck only had one.)

CheckHandler);

SmallVector<std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>, 3>
Checks;

llvm::Value *True = llvm::ConstantInt::getTrue(getLLVMContext());
bool AllowNullPointers = isNullPointerAllowed(TCK);
if ((SanOpts.has(SanitizerKind::Null) || AllowNullPointers) &&
!IsGuaranteedNonNull) {
Expand All @@ -799,10 +794,20 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
Builder.CreateCondBr(IsNonNull, Rest, Done);
EmitBlock(Rest);
} else {
Checks.push_back(std::make_pair(IsNonNull, SanitizerKind::SO_Null));
llvm::Constant *StaticData[] = {EmitCheckSourceLocation(Loc),
EmitCheckTypeDescriptor(Ty),
llvm::ConstantInt::get(Int8Ty, TCK)};
EmitCheck({{IsNonNull, SanitizerKind::SO_Null}}, CheckHandler,
StaticData, Ptr);
}
}
}
}

{
auto CheckHandler = SanitizerHandler::InsufficientObjectSize;
SanitizerDebugLocation SanScope(this, {SanitizerKind::SO_ObjectSize},
CheckHandler);

if (SanOpts.has(SanitizerKind::ObjectSize) &&
!SkippedChecks.has(SanitizerKind::ObjectSize) &&
Expand All @@ -827,10 +832,19 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
llvm::Value *Dynamic = Builder.getFalse();
llvm::Value *LargeEnough = Builder.CreateICmpUGE(
Builder.CreateCall(F, {Ptr, Min, NullIsUnknown, Dynamic}), Size);
Checks.push_back(
std::make_pair(LargeEnough, SanitizerKind::SO_ObjectSize));
llvm::Constant *StaticData[] = {EmitCheckSourceLocation(Loc),
EmitCheckTypeDescriptor(Ty),
llvm::ConstantInt::get(Int8Ty, TCK)};
EmitCheck({{LargeEnough, SanitizerKind::SO_ObjectSize}}, CheckHandler,
StaticData, Ptr);
}
}
}

{
auto CheckHandler = SanitizerHandler::MisalignedPointerUse;
SanitizerDebugLocation SanScope(this, {SanitizerKind::SO_Alignment},
CheckHandler);

llvm::MaybeAlign AlignVal;
llvm::Value *PtrAsInt = nullptr;
Expand All @@ -851,19 +865,16 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
PtrAsInt, llvm::ConstantInt::get(IntPtrTy, AlignVal->value() - 1));
llvm::Value *Aligned =
Builder.CreateICmpEQ(Align, llvm::ConstantInt::get(IntPtrTy, 0));
if (Aligned != True)
Checks.push_back(
std::make_pair(Aligned, SanitizerKind::SO_Alignment));
if (Aligned != True) {
llvm::Constant *StaticData[] = {
EmitCheckSourceLocation(Loc), EmitCheckTypeDescriptor(Ty),
llvm::ConstantInt::get(Int8Ty, llvm::Log2(*AlignVal)),
llvm::ConstantInt::get(Int8Ty, TCK)};
EmitCheck({{Aligned, SanitizerKind::SO_Alignment}}, CheckHandler,
StaticData, PtrAsInt ? PtrAsInt : Ptr);
}
}
}

if (Checks.size() > 0) {
llvm::Constant *StaticData[] = {
EmitCheckSourceLocation(Loc), EmitCheckTypeDescriptor(Ty),
llvm::ConstantInt::get(Int8Ty, AlignVal ? llvm::Log2(*AlignVal) : 1),
llvm::ConstantInt::get(Int8Ty, TCK)};
EmitCheck(Checks, CheckHandler, StaticData, PtrAsInt ? PtrAsInt : Ptr);
}
}

// If possible, check that the vptr indicates that there is a subobject of
Expand Down Expand Up @@ -950,7 +961,7 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
SanitizerDebugLocation SanScope(
this,
{DoneViaNullSanitize ? SanitizerKind::SO_Null : SanitizerKind::SO_Vptr},
DoneViaNullSanitize ? SanitizerHandler::TypeMismatch
DoneViaNullSanitize ? SanitizerHandler::NullPointerUse
: SanitizerHandler::DynamicTypeCacheMiss);
Builder.CreateBr(Done);
EmitBlock(Done);
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/CodeGen/SanitizerHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@
SANITIZER_CHECK( \
VLABoundNotPositive, vla_bound_not_positive, 0, \
"Variable length array bound evaluates to non-positive value") \
SANITIZER_CHECK(NullPointerUse, null_pointer_use, 0, "Null pointer usage") \
SANITIZER_CHECK( \
NullPointerUseWithNullability, null_pointer_use_with_nullability, 0, \
"Assigning null to a lvalue which is annotated with _Nonnull") \
SANITIZER_CHECK(MisalignedPointerUse, misaligned_pointer_use, 0, \
"Misaligned pointer use") \
SANITIZER_CHECK(InsufficientObjectSize, insufficient_object_size, 0, \
"Insufficient object size") \
SANITIZER_CHECK(BoundsSafety, bounds_safety, 0, \
"") // BoundsSafety Msg is empty because it is not considered
// part of UBSan; therefore, no trap reason is emitted for
Expand Down
24 changes: 7 additions & 17 deletions clang/test/CodeGen/allow-ubsan-check.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,9 @@
// CHECK-NEXT: [[TMP1:%.*]] = icmp ne i32 [[X]], -2147483648, !nosanitize [[META2]]
// CHECK-NEXT: [[TMP2:%.*]] = icmp ne i32 [[Y]], -1, !nosanitize [[META2]]
// CHECK-NEXT: [[OR:%.*]] = or i1 [[TMP1]], [[TMP2]], !nosanitize [[META2]]
//
// 27 == SO_IntegerDivideByZero
// CHECK-NEXT: [[TMP3:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 27), !nosanitize [[META2]]
// CHECK-NEXT: [[TMP4:%.*]] = xor i1 [[TMP3]], true, !nosanitize [[META2]]
// CHECK-NEXT: [[TMP5:%.*]] = or i1 [[TMP0]], [[TMP4]], !nosanitize [[META2]]
//
// 41 == SO_SignedIntegerOverflow
// CHECK-NEXT: [[TMP6:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 41), !nosanitize [[META2]]
// CHECK-NEXT: [[TMP7:%.*]] = xor i1 [[TMP6]], true, !nosanitize [[META2]]
// CHECK-NEXT: [[TMP8:%.*]] = or i1 [[OR]], [[TMP7]], !nosanitize [[META2]]
Expand Down Expand Up @@ -89,13 +85,11 @@ int div(int x, int y) {
// CHECK-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[TMP0:%.*]] = icmp eq ptr [[X]], null, !nosanitize [[META2]]
//
// 29 == SO_Null
// CHECK-NEXT: [[TMP1:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 29), !nosanitize [[META2]]
// CHECK-NEXT: [[DOTNOT1:%.*]] = and i1 [[TMP0]], [[TMP1]]
// CHECK-NEXT: br i1 [[DOTNOT1]], label %[[HANDLER_TYPE_MISMATCH:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
// CHECK: [[HANDLER_TYPE_MISMATCH]]:
// CHECK-NEXT: tail call void @__ubsan_handle_type_mismatch_v1_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR6]], !nosanitize [[META2]]
// CHECK-NEXT: br i1 [[DOTNOT1]], label %[[HANDLER_NULL_POINTER_USE:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
// CHECK: [[HANDLER_NULL_POINTER_USE]]:
// CHECK-NEXT: tail call void @__ubsan_handle_null_pointer_use_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR6]], !nosanitize [[META2]]
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
// CHECK: [[CONT]]:
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
Expand All @@ -109,7 +103,7 @@ int div(int x, int y) {
// TR-NEXT: [[DOTNOT1:%.*]] = and i1 [[TMP0]], [[TMP1]]
// TR-NEXT: br i1 [[DOTNOT1]], label %[[TRAP:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
// TR: [[TRAP]]:
// TR-NEXT: tail call void @llvm.ubsantrap(i8 22) #[[ATTR5]], !nosanitize [[META2]]
// TR-NEXT: tail call void @llvm.ubsantrap(i8 25) #[[ATTR5]], !nosanitize [[META2]]
// TR-NEXT: unreachable, !nosanitize [[META2]]
// TR: [[CONT]]:
// TR-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
Expand All @@ -121,9 +115,9 @@ int div(int x, int y) {
// REC-NEXT: [[TMP0:%.*]] = icmp eq ptr [[X]], null, !nosanitize [[META2]]
// REC-NEXT: [[TMP1:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 29), !nosanitize [[META2]]
// REC-NEXT: [[DOTNOT1:%.*]] = and i1 [[TMP0]], [[TMP1]]
// REC-NEXT: br i1 [[DOTNOT1]], label %[[HANDLER_TYPE_MISMATCH:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
// REC: [[HANDLER_TYPE_MISMATCH]]:
// REC-NEXT: tail call void @__ubsan_handle_type_mismatch_v1(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR6]], !nosanitize [[META2]]
// REC-NEXT: br i1 [[DOTNOT1]], label %[[HANDLER_NULL_POINTER_USE:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
// REC: [[HANDLER_NULL_POINTER_USE]]:
// REC-NEXT: tail call void @__ubsan_handle_null_pointer_use(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR6]], !nosanitize [[META2]]
// REC-NEXT: br label %[[CONT]], !nosanitize [[META2]]
// REC: [[CONT]]:
// REC-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
Expand All @@ -138,8 +132,6 @@ int null(int* x) {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[TMP0:%.*]] = tail call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[X]], i32 [[Y]]), !nosanitize [[META2]]
// CHECK-NEXT: [[TMP1:%.*]] = extractvalue { i32, i1 } [[TMP0]], 1, !nosanitize [[META2]]
//
// 41 == SO_SignedIntegerOverflow
// CHECK-NEXT: [[TMP2:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 41), !nosanitize [[META2]]
// CHECK-NEXT: [[DOTDEMORGAN:%.*]] = and i1 [[TMP1]], [[TMP2]]
// CHECK-NEXT: br i1 [[DOTDEMORGAN]], label %[[HANDLER_ADD_OVERFLOW:.*]], label %[[CONT:.*]], !prof [[PROF4]], !nosanitize [[META2]]
Expand Down Expand Up @@ -198,8 +190,6 @@ void use(double*);
// CHECK-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR7:[0-9]+]]
// CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[I]] to i64
// CHECK-NEXT: [[TMP1:%.*]] = icmp ule i64 [[TMP0]], [[IDXPROM]]
//
// 71 == SO_LocalBounds
// CHECK-NEXT: [[TMP2:%.*]] = call i1 @llvm.allow.ubsan.check(i8 71), !nosanitize [[META2]]
// CHECK-NEXT: [[TMP3:%.*]] = and i1 [[TMP1]], [[TMP2]], !nosanitize [[META2]]
// CHECK-NEXT: br i1 [[TMP3]], label %[[TRAP:.*]], label %[[BB4:.*]]
Expand Down
12 changes: 6 additions & 6 deletions clang/test/CodeGen/attr-counted-by.c
Original file line number Diff line number Diff line change
Expand Up @@ -1223,12 +1223,12 @@ int test12_a, test12_b;
// SANITIZE-WITH-ATTR-NEXT: store i32 [[TMP2]], ptr @test12_b, align 4, !tbaa [[TBAA4]]
// SANITIZE-WITH-ATTR-NEXT: [[DOTCOUNTED_BY_LOAD:%.*]] = load i32, ptr @test12_foo, align 4
// SANITIZE-WITH-ATTR-NEXT: [[DOTNOT:%.*]] = icmp eq i32 [[DOTCOUNTED_BY_LOAD]], 0
// SANITIZE-WITH-ATTR-NEXT: br i1 [[DOTNOT]], label [[HANDLER_OUT_OF_BOUNDS4:%.*]], label [[HANDLER_TYPE_MISMATCH6:%.*]], !prof [[PROF8]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: br i1 [[DOTNOT]], label [[HANDLER_OUT_OF_BOUNDS4:%.*]], label [[HANDLER_INSUFFICIENT_OBJECT_SIZE6:%.*]], !prof [[PROF8]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds4:
// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB24:[0-9]+]], i64 0) #[[ATTR8]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.type_mismatch6:
// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_type_mismatch_v1_abort(ptr nonnull @[[GLOB25:[0-9]+]], i64 ptrtoint (ptr getelementptr inbounds nuw (i8, ptr @test12_foo, i64 4) to i64)) #[[ATTR8]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.insufficient_object_size6:
// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_insufficient_object_size_abort(ptr nonnull @[[GLOB25:[0-9]+]], i64 ptrtoint (ptr getelementptr inbounds nuw (i8, ptr @test12_foo, i64 4) to i64)) #[[ATTR8]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
//
// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local noundef i32 @test12(
Expand Down Expand Up @@ -1265,12 +1265,12 @@ int test12_a, test12_b;
// SANITIZE-WITHOUT-ATTR-NEXT: store i32 [[TMP2]], ptr @test12_b, align 4, !tbaa [[TBAA2]]
// SANITIZE-WITHOUT-ATTR-NEXT: [[DOTCOUNTED_BY_LOAD:%.*]] = load i32, ptr @test12_foo, align 4
// SANITIZE-WITHOUT-ATTR-NEXT: [[DOTNOT:%.*]] = icmp eq i32 [[DOTCOUNTED_BY_LOAD]], 0
// SANITIZE-WITHOUT-ATTR-NEXT: br i1 [[DOTNOT]], label [[HANDLER_OUT_OF_BOUNDS4:%.*]], label [[HANDLER_TYPE_MISMATCH6:%.*]], !prof [[PROF10:![0-9]+]], !nosanitize [[META9]]
// SANITIZE-WITHOUT-ATTR-NEXT: br i1 [[DOTNOT]], label [[HANDLER_OUT_OF_BOUNDS4:%.*]], label [[HANDLER_INSUFFICIENT_OBJECT_SIZE6:%.*]], !prof [[PROF10:![0-9]+]], !nosanitize [[META9]]
// SANITIZE-WITHOUT-ATTR: handler.out_of_bounds4:
// SANITIZE-WITHOUT-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB4:[0-9]+]], i64 0) #[[ATTR8]], !nosanitize [[META9]]
// SANITIZE-WITHOUT-ATTR-NEXT: unreachable, !nosanitize [[META9]]
// SANITIZE-WITHOUT-ATTR: handler.type_mismatch6:
// SANITIZE-WITHOUT-ATTR-NEXT: tail call void @__ubsan_handle_type_mismatch_v1_abort(ptr nonnull @[[GLOB5:[0-9]+]], i64 ptrtoint (ptr getelementptr inbounds nuw (i8, ptr @test12_foo, i64 4) to i64)) #[[ATTR8]], !nosanitize [[META9]]
// SANITIZE-WITHOUT-ATTR: handler.insufficient_object_size6:
// SANITIZE-WITHOUT-ATTR-NEXT: tail call void @__ubsan_handle_insufficient_object_size_abort(ptr nonnull @[[GLOB5:[0-9]+]], i64 ptrtoint (ptr getelementptr inbounds nuw (i8, ptr @test12_foo, i64 4) to i64)) #[[ATTR8]], !nosanitize [[META9]]
// SANITIZE-WITHOUT-ATTR-NEXT: unreachable, !nosanitize [[META9]]
//
// NO-SANITIZE-WITHOUT-ATTR-LABEL: define dso_local noundef i32 @test12(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ void *f(C *c) {
// CHECK-SANITIZE-NEXT: %[[PTRTOINT:.*]] = ptrtoint ptr %[[C_RELOAD]] to i64, !nosanitize
// CHECK-SANITIZE-NEXT: %[[MASKEDPTR:.*]] = and i64 %[[PTRTOINT]], 3, !nosanitize
// CHECK-SANITIZE-NEXT: %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0, !nosanitize
// CHECK-SANITIZE-NEXT: br i1 %[[MASKCOND]], label %[[CONT:[^,]+]], label %[[HANDLER_TYPE_MISMATCH:[^,]+]]
// CHECK-SANITIZE: [[HANDLER_TYPE_MISMATCH]]:
// CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_type_mismatch_v1_abort(
// CHECK-SANITIZE-RECOVER-NEXT: call void @__ubsan_handle_type_mismatch_v1(
// CHECK-SANITIZE-NEXT: br i1 %[[MASKCOND]], label %[[CONT:[^,]+]], label %[[HANDLER_TYPE_MISALIGNED_POINTER_USE:[^,]+]]
// CHECK-SANITIZE: [[HANDLER_TYPE_MISALIGNED_POINTER_USE]]:
// CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_misaligned_pointer_use_abort(
// CHECK-SANITIZE-RECOVER-NEXT: call void @__ubsan_handle_misaligned_pointer_use(
// CHECK-SANITIZE-TRAP-NEXT: call void @llvm.ubsantrap(
// CHECK-SANITIZE-UNREACHABLE-NEXT: unreachable, !nosanitize
// CHECK-SANITIZE: [[CONT]]:
Expand Down
Loading