Skip to content

[Reland][Clang][CodeGen][UBSan] Add more precise attributes to recoverable ubsan handlers #135135

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 17, 2025
Merged

[Reland][Clang][CodeGen][UBSan] Add more precise attributes to recoverable ubsan handlers #135135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de6d895
[Clang][CodeGen][UBSan] Add more precise attributes to recoverable ub…
dtcxzyw Apr 10, 2025
b6f0d59
[Clang][CodeGen] Add miscompilation reproducer
dtcxzyw Apr 10, 2025
9aba004
[Clang][CodeGen] Add memory(read) if the check value is passed by ref
dtcxzyw Apr 10, 2025
ca45b63
[Clang][CodeGen] Remove mustprogress
dtcxzyw Apr 17, 2025
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
31 changes: 25 additions & 6 deletions clang/lib/CodeGen/CGExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3486,7 +3486,8 @@ llvm::Constant *CodeGenFunction::EmitCheckTypeDescriptor(QualType T) {
return GV;
}

llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V,
bool &MayReadFromPtrToInt) {
llvm::Type *TargetTy = IntPtrTy;

if (V->getType() == TargetTy)
Expand All @@ -3512,6 +3513,7 @@ llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
Builder.CreateStore(V, Ptr);
V = Ptr.getPointer();
}
MayReadFromPtrToInt = true;
return Builder.CreatePtrToInt(V, TargetTy);
}

Expand Down Expand Up @@ -3617,7 +3619,8 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
ArrayRef<llvm::Value *> FnArgs,
SanitizerHandler CheckHandler,
CheckRecoverableKind RecoverKind, bool IsFatal,
llvm::BasicBlock *ContBB, bool NoMerge) {
llvm::BasicBlock *ContBB, bool NoMerge,
bool MayReadFromPtrToInt) {
assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
std::optional<ApplyDebugLocation> DL;
if (!CGF.Builder.getCurrentDebugLocation()) {
Expand Down Expand Up @@ -3645,6 +3648,20 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
.addAttribute(llvm::Attribute::NoUnwind);
}
B.addUWTableAttr(llvm::UWTableKind::Default);
// Add more precise attributes to recoverable ubsan handlers for better
// optimizations.
if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 && MayReturn) {
// __ubsan_handle_dynamic_type_cache_miss reads the vtable, which is also
// accessible by the current module.
if (CheckHandler != SanitizerHandler::DynamicTypeCacheMiss) {
llvm::MemoryEffects ME =
llvm::MemoryEffects::argMemOnly(llvm::ModRefInfo::Ref) |
llvm::MemoryEffects::inaccessibleMemOnly();
if (MayReadFromPtrToInt)
ME |= llvm::MemoryEffects::readOnly();
B.addMemoryAttr(ME);
}
}

llvm::FunctionCallee Fn = CGF.CGM.CreateRuntimeFunction(
FnType, FnName,
Expand Down Expand Up @@ -3741,6 +3758,7 @@ void CodeGenFunction::EmitCheck(
// representing operand values.
SmallVector<llvm::Value *, 4> Args;
SmallVector<llvm::Type *, 4> ArgTypes;
bool MayReadFromPtrToInt = false;
if (!CGM.getCodeGenOpts().SanitizeMinimalRuntime) {
Args.reserve(DynamicArgs.size() + 1);
ArgTypes.reserve(DynamicArgs.size() + 1);
Expand All @@ -3760,7 +3778,7 @@ void CodeGenFunction::EmitCheck(
}

for (size_t i = 0, n = DynamicArgs.size(); i != n; ++i) {
Args.push_back(EmitCheckValue(DynamicArgs[i]));
Args.push_back(EmitCheckValue(DynamicArgs[i], MayReadFromPtrToInt));
ArgTypes.push_back(IntPtrTy);
}
}
Expand All @@ -3772,7 +3790,8 @@ void CodeGenFunction::EmitCheck(
// Simple case: we need to generate a single handler call, either
// fatal, or non-fatal.
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
(FatalCond != nullptr), Cont, NoMerge);
(FatalCond != nullptr), Cont, NoMerge,
MayReadFromPtrToInt);
} else {
// Emit two handler calls: first one for set of unrecoverable checks,
// another one for recoverable.
Expand All @@ -3782,10 +3801,10 @@ void CodeGenFunction::EmitCheck(
Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
EmitBlock(FatalHandlerBB);
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
NonFatalHandlerBB, NoMerge);
NonFatalHandlerBB, NoMerge, MayReadFromPtrToInt);
EmitBlock(NonFatalHandlerBB);
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
Cont, NoMerge);
Cont, NoMerge, MayReadFromPtrToInt);
}

EmitBlock(Cont);
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/CodeGen/CodeGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5244,7 +5244,9 @@ class CodeGenFunction : public CodeGenTypeCache {

/// Convert a value into a format suitable for passing to a runtime
/// sanitizer handler.
llvm::Value *EmitCheckValue(llvm::Value *V);
/// If the check value is a pointer or passed by reference, set \p
/// MayReadFromPtrToInt to true.
llvm::Value *EmitCheckValue(llvm::Value *V, bool &MayReadFromPtrToInt);

/// Emit a description of a source location in a format suitable for
/// passing to a runtime sanitizer handler.
Expand Down
22 changes: 22 additions & 0 deletions clang/test/CodeGen/AArch64/ubsan-handler-pass-by-ref.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
// RUN: %clang_cc1 -triple armv7-linux-androideabi24 -emit-llvm -fsanitize=signed-integer-overflow -O3 %s -o - -fsanitize-recover=signed-integer-overflow -w | FileCheck %s --check-prefixes=RECOVER
// REQUIRES: aarch64-registered-target

#define LLONG_MIN (-__LONG_LONG_MAX__-1LL)

// RECOVER-LABEL: define dso_local noundef i32 @main(
// RECOVER-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
// RECOVER-NEXT: [[ENTRY:.*:]]
// RECOVER-NEXT: [[TMP:%.*]] = alloca i64, align 8
// RECOVER-NEXT: store i64 -9223372036854775808, ptr [[TMP]], align 8, !nosanitize [[META3:![0-9]+]]
// RECOVER-NEXT: [[TMP0:%.*]] = ptrtoint ptr [[TMP]] to i32, !nosanitize [[META3]]
// RECOVER-NEXT: call void @__ubsan_handle_negate_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i32 [[TMP0]]) #[[ATTR2:[0-9]+]], !nosanitize [[META3]]
// RECOVER-NEXT: ret i32 0
//
int main() {
__builtin_llabs(LLONG_MIN);
return 0;
}
//.
// RECOVER: [[META3]] = !{}
//.
28 changes: 14 additions & 14 deletions clang/test/CodeGen/allow-ubsan-check.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
// CHECK: [[HANDLER_DIVREM_OVERFLOW]]:
// CHECK-NEXT: [[TMP10:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
// CHECK-NEXT: [[TMP11:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
// CHECK-NEXT: tail call void @__ubsan_handle_divrem_overflow_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR6:[0-9]+]], !nosanitize [[META2]]
// CHECK-NEXT: tail call void @__ubsan_handle_divrem_overflow_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR8:[0-9]+]], !nosanitize [[META2]]
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
// CHECK: [[CONT]]:
// CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[X]], [[Y]]
Expand Down Expand Up @@ -75,7 +75,7 @@
// REC: [[HANDLER_DIVREM_OVERFLOW]]:
// REC-NEXT: [[TMP10:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
// REC-NEXT: [[TMP11:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
// REC-NEXT: tail call void @__ubsan_handle_divrem_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR6:[0-9]+]], !nosanitize [[META2]]
// REC-NEXT: tail call void @__ubsan_handle_divrem_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR8:[0-9]+]], !nosanitize [[META2]]
// REC-NEXT: br label %[[CONT]], !nosanitize [[META2]]
// REC: [[CONT]]:
// REC-NEXT: [[DIV:%.*]] = sdiv i32 [[X]], [[Y]]
Expand All @@ -86,7 +86,7 @@ int div(int x, int y) {
}

// CHECK-LABEL: define dso_local i32 @null(
// CHECK-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[TMP0:%.*]] = icmp eq ptr [[X]], null, !nosanitize [[META2]]
//
Expand All @@ -95,7 +95,7 @@ int div(int x, int y) {
// 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: tail call void @__ubsan_handle_type_mismatch_v1_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR8]], !nosanitize [[META2]]
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
// CHECK: [[CONT]]:
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
Expand All @@ -116,14 +116,14 @@ int div(int x, int y) {
// TR-NEXT: ret i32 [[TMP2]]
//
// REC-LABEL: define dso_local i32 @null(
// REC-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
// REC-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] {
// REC-NEXT: [[ENTRY:.*:]]
// 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: tail call void @__ubsan_handle_type_mismatch_v1(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR8]], !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 @@ -146,7 +146,7 @@ int null(int* x) {
// CHECK: [[HANDLER_ADD_OVERFLOW]]:
// CHECK-NEXT: [[TMP3:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
// CHECK-NEXT: [[TMP4:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
// CHECK-NEXT: tail call void @__ubsan_handle_add_overflow_abort(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR6]], !nosanitize [[META2]]
// CHECK-NEXT: tail call void @__ubsan_handle_add_overflow_abort(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR8]], !nosanitize [[META2]]
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
// CHECK: [[CONT]]:
// CHECK-NEXT: [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
Expand Down Expand Up @@ -178,7 +178,7 @@ int null(int* x) {
// REC: [[HANDLER_ADD_OVERFLOW]]:
// REC-NEXT: [[TMP3:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
// REC-NEXT: [[TMP4:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
// REC-NEXT: tail call void @__ubsan_handle_add_overflow(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR6]], !nosanitize [[META2]]
// REC-NEXT: tail call void @__ubsan_handle_add_overflow(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR8]], !nosanitize [[META2]]
// REC-NEXT: br label %[[CONT]], !nosanitize [[META2]]
// REC: [[CONT]]:
// REC-NEXT: [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
Expand All @@ -191,11 +191,11 @@ int overflow(int x, int y) {
void use(double*);

// CHECK-LABEL: define dso_local double @lbounds(
// CHECK-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR0]] {
// CHECK-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR5:[0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
// CHECK-NEXT: [[TMP0:%.*]] = zext i32 [[B]] to i64
// CHECK-NEXT: [[VLA:%.*]] = alloca double, i64 [[TMP0]], align 16
// CHECK-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR7:[0-9]+]]
// CHECK-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR9:[0-9]+]]
// CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[I]] to i64
// CHECK-NEXT: [[TMP1:%.*]] = icmp ule i64 [[TMP0]], [[IDXPROM]]
//
Expand All @@ -208,7 +208,7 @@ void use(double*);
// CHECK-NEXT: [[TMP5:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !tbaa [[TBAA9:![0-9]+]]
// CHECK-NEXT: ret double [[TMP5]]
// CHECK: [[TRAP]]:
// CHECK-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR6]], !nosanitize [[META2]]
// CHECK-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR8]], !nosanitize [[META2]]
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
//
// TR-LABEL: define dso_local double @lbounds(
Expand All @@ -231,11 +231,11 @@ void use(double*);
// TR-NEXT: unreachable, !nosanitize [[META2]]
//
// REC-LABEL: define dso_local double @lbounds(
// REC-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR0]] {
// REC-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR5:[0-9]+]] {
// REC-NEXT: [[ENTRY:.*:]]
// REC-NEXT: [[TMP0:%.*]] = zext i32 [[B]] to i64
// REC-NEXT: [[VLA:%.*]] = alloca double, i64 [[TMP0]], align 16
// REC-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR5:[0-9]+]]
// REC-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR7:[0-9]+]]
// REC-NEXT: [[IDXPROM:%.*]] = sext i32 [[I]] to i64
// REC-NEXT: [[TMP1:%.*]] = icmp ule i64 [[TMP0]], [[IDXPROM]]
// REC-NEXT: [[TMP2:%.*]] = call i1 @llvm.allow.ubsan.check(i8 71), !nosanitize [[META2]]
Expand All @@ -246,7 +246,7 @@ void use(double*);
// REC-NEXT: [[TMP5:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !tbaa [[TBAA9:![0-9]+]]
// REC-NEXT: ret double [[TMP5]]
// REC: [[TRAP]]:
// REC-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR6]], !nosanitize [[META2]]
// REC-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR8]], !nosanitize [[META2]]
// REC-NEXT: br label %[[BB4]], !nosanitize [[META2]]
//
double lbounds(int b, int i) {
Expand Down
Loading
Loading