Skip to content

Commit 909a9fe

Browse files
dtcxzywdtcxzyw
authored
[Reland][Clang][CodeGen][UBSan] Add more precise attributes to recoverable ubsan handlers (#135135)
This patch relands #130990. If the check value is passed by reference, add `memory(read)`. Original PR description: This patch adds `memory(argmem: read, inaccessiblemem: readwrite)` to **recoverable** ubsan handlers in order to unblock some memory/loop optimizations. It provides an average of 3% performance improvement on llvm-test-suite (except for 49 test failures due to ubsan diagnostics).
1 parent 2007dcf commit 909a9fe

File tree

6 files changed

+280
-137
lines changed

6 files changed

+280
-137
lines changed

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3486,7 +3486,8 @@ llvm::Constant *CodeGenFunction::EmitCheckTypeDescriptor(QualType T) {
34863486
return GV;
34873487
}
34883488

3489-
llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
3489+
llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V,
3490+
bool &MayReadFromPtrToInt) {
34903491
llvm::Type *TargetTy = IntPtrTy;
34913492

34923493
if (V->getType() == TargetTy)
@@ -3512,6 +3513,7 @@ llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
35123513
Builder.CreateStore(V, Ptr);
35133514
V = Ptr.getPointer();
35143515
}
3516+
MayReadFromPtrToInt = true;
35153517
return Builder.CreatePtrToInt(V, TargetTy);
35163518
}
35173519

@@ -3617,7 +3619,8 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
36173619
ArrayRef<llvm::Value *> FnArgs,
36183620
SanitizerHandler CheckHandler,
36193621
CheckRecoverableKind RecoverKind, bool IsFatal,
3620-
llvm::BasicBlock *ContBB, bool NoMerge) {
3622+
llvm::BasicBlock *ContBB, bool NoMerge,
3623+
bool MayReadFromPtrToInt) {
36213624
assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
36223625
std::optional<ApplyDebugLocation> DL;
36233626
if (!CGF.Builder.getCurrentDebugLocation()) {
@@ -3645,6 +3648,20 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
36453648
.addAttribute(llvm::Attribute::NoUnwind);
36463649
}
36473650
B.addUWTableAttr(llvm::UWTableKind::Default);
3651+
// Add more precise attributes to recoverable ubsan handlers for better
3652+
// optimizations.
3653+
if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 && MayReturn) {
3654+
// __ubsan_handle_dynamic_type_cache_miss reads the vtable, which is also
3655+
// accessible by the current module.
3656+
if (CheckHandler != SanitizerHandler::DynamicTypeCacheMiss) {
3657+
llvm::MemoryEffects ME =
3658+
llvm::MemoryEffects::argMemOnly(llvm::ModRefInfo::Ref) |
3659+
llvm::MemoryEffects::inaccessibleMemOnly();
3660+
if (MayReadFromPtrToInt)
3661+
ME |= llvm::MemoryEffects::readOnly();
3662+
B.addMemoryAttr(ME);
3663+
}
3664+
}
36483665

36493666
llvm::FunctionCallee Fn = CGF.CGM.CreateRuntimeFunction(
36503667
FnType, FnName,
@@ -3741,6 +3758,7 @@ void CodeGenFunction::EmitCheck(
37413758
// representing operand values.
37423759
SmallVector<llvm::Value *, 4> Args;
37433760
SmallVector<llvm::Type *, 4> ArgTypes;
3761+
bool MayReadFromPtrToInt = false;
37443762
if (!CGM.getCodeGenOpts().SanitizeMinimalRuntime) {
37453763
Args.reserve(DynamicArgs.size() + 1);
37463764
ArgTypes.reserve(DynamicArgs.size() + 1);
@@ -3760,7 +3778,7 @@ void CodeGenFunction::EmitCheck(
37603778
}
37613779

37623780
for (size_t i = 0, n = DynamicArgs.size(); i != n; ++i) {
3763-
Args.push_back(EmitCheckValue(DynamicArgs[i]));
3781+
Args.push_back(EmitCheckValue(DynamicArgs[i], MayReadFromPtrToInt));
37643782
ArgTypes.push_back(IntPtrTy);
37653783
}
37663784
}
@@ -3772,7 +3790,8 @@ void CodeGenFunction::EmitCheck(
37723790
// Simple case: we need to generate a single handler call, either
37733791
// fatal, or non-fatal.
37743792
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
3775-
(FatalCond != nullptr), Cont, NoMerge);
3793+
(FatalCond != nullptr), Cont, NoMerge,
3794+
MayReadFromPtrToInt);
37763795
} else {
37773796
// Emit two handler calls: first one for set of unrecoverable checks,
37783797
// another one for recoverable.
@@ -3782,10 +3801,10 @@ void CodeGenFunction::EmitCheck(
37823801
Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
37833802
EmitBlock(FatalHandlerBB);
37843803
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
3785-
NonFatalHandlerBB, NoMerge);
3804+
NonFatalHandlerBB, NoMerge, MayReadFromPtrToInt);
37863805
EmitBlock(NonFatalHandlerBB);
37873806
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
3788-
Cont, NoMerge);
3807+
Cont, NoMerge, MayReadFromPtrToInt);
37893808
}
37903809

37913810
EmitBlock(Cont);

‎clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5244,7 +5244,9 @@ class CodeGenFunction : public CodeGenTypeCache {
52445244

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

52495251
/// Emit a description of a source location in a format suitable for
52505252
/// passing to a runtime sanitizer handler.

‎clang/test/CodeGen/AArch64/ubsan-handler-pass-by-ref.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
2+
// 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
3+
// REQUIRES: aarch64-registered-target
4+
5+
#define LLONG_MIN (-__LONG_LONG_MAX__-1LL)
6+
7+
// RECOVER-LABEL: define dso_local noundef i32 @main(
8+
// RECOVER-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
9+
// RECOVER-NEXT: [[ENTRY:.*:]]
10+
// RECOVER-NEXT: [[TMP:%.*]] = alloca i64, align 8
11+
// RECOVER-NEXT: store i64 -9223372036854775808, ptr [[TMP]], align 8, !nosanitize [[META3:![0-9]+]]
12+
// RECOVER-NEXT: [[TMP0:%.*]] = ptrtoint ptr [[TMP]] to i32, !nosanitize [[META3]]
13+
// RECOVER-NEXT: call void @__ubsan_handle_negate_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i32 [[TMP0]]) #[[ATTR2:[0-9]+]], !nosanitize [[META3]]
14+
// RECOVER-NEXT: ret i32 0
15+
//
16+
int main() {
17+
__builtin_llabs(LLONG_MIN);
18+
return 0;
19+
}
20+
//.
21+
// RECOVER: [[META3]] = !{}
22+
//.

‎clang/test/CodeGen/allow-ubsan-check.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
// CHECK: [[HANDLER_DIVREM_OVERFLOW]]:
3030
// CHECK-NEXT: [[TMP10:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
3131
// CHECK-NEXT: [[TMP11:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
32-
// CHECK-NEXT: tail call void @__ubsan_handle_divrem_overflow_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR6:[0-9]+]], !nosanitize [[META2]]
32+
// CHECK-NEXT: tail call void @__ubsan_handle_divrem_overflow_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR8:[0-9]+]], !nosanitize [[META2]]
3333
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
3434
// CHECK: [[CONT]]:
3535
// CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[X]], [[Y]]
@@ -75,7 +75,7 @@
7575
// REC: [[HANDLER_DIVREM_OVERFLOW]]:
7676
// REC-NEXT: [[TMP10:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
7777
// REC-NEXT: [[TMP11:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
78-
// REC-NEXT: tail call void @__ubsan_handle_divrem_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR6:[0-9]+]], !nosanitize [[META2]]
78+
// REC-NEXT: tail call void @__ubsan_handle_divrem_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[TMP10]], i64 [[TMP11]]) #[[ATTR8:[0-9]+]], !nosanitize [[META2]]
7979
// REC-NEXT: br label %[[CONT]], !nosanitize [[META2]]
8080
// REC: [[CONT]]:
8181
// REC-NEXT: [[DIV:%.*]] = sdiv i32 [[X]], [[Y]]
@@ -86,7 +86,7 @@ int div(int x, int y) {
8686
}
8787

8888
// CHECK-LABEL: define dso_local i32 @null(
89-
// CHECK-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
89+
// CHECK-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] {
9090
// CHECK-NEXT: [[ENTRY:.*:]]
9191
// CHECK-NEXT: [[TMP0:%.*]] = icmp eq ptr [[X]], null, !nosanitize [[META2]]
9292
//
@@ -95,7 +95,7 @@ int div(int x, int y) {
9595
// CHECK-NEXT: [[DOTNOT1:%.*]] = and i1 [[TMP0]], [[TMP1]]
9696
// CHECK-NEXT: br i1 [[DOTNOT1]], label %[[HANDLER_TYPE_MISMATCH:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
9797
// CHECK: [[HANDLER_TYPE_MISMATCH]]:
98-
// CHECK-NEXT: tail call void @__ubsan_handle_type_mismatch_v1_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR6]], !nosanitize [[META2]]
98+
// CHECK-NEXT: tail call void @__ubsan_handle_type_mismatch_v1_abort(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR8]], !nosanitize [[META2]]
9999
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
100100
// CHECK: [[CONT]]:
101101
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
@@ -116,14 +116,14 @@ int div(int x, int y) {
116116
// TR-NEXT: ret i32 [[TMP2]]
117117
//
118118
// REC-LABEL: define dso_local i32 @null(
119-
// REC-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
119+
// REC-SAME: ptr noundef readonly captures(address_is_null) [[X:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] {
120120
// REC-NEXT: [[ENTRY:.*:]]
121121
// REC-NEXT: [[TMP0:%.*]] = icmp eq ptr [[X]], null, !nosanitize [[META2]]
122122
// REC-NEXT: [[TMP1:%.*]] = tail call i1 @llvm.allow.ubsan.check(i8 29), !nosanitize [[META2]]
123123
// REC-NEXT: [[DOTNOT1:%.*]] = and i1 [[TMP0]], [[TMP1]]
124124
// REC-NEXT: br i1 [[DOTNOT1]], label %[[HANDLER_TYPE_MISMATCH:.*]], label %[[CONT:.*]], !prof [[PROF4:![0-9]+]], !nosanitize [[META2]]
125125
// REC: [[HANDLER_TYPE_MISMATCH]]:
126-
// REC-NEXT: tail call void @__ubsan_handle_type_mismatch_v1(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR6]], !nosanitize [[META2]]
126+
// REC-NEXT: tail call void @__ubsan_handle_type_mismatch_v1(ptr nonnull @[[GLOB2:[0-9]+]], i64 0) #[[ATTR8]], !nosanitize [[META2]]
127127
// REC-NEXT: br label %[[CONT]], !nosanitize [[META2]]
128128
// REC: [[CONT]]:
129129
// REC-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[TBAA5:![0-9]+]]
@@ -146,7 +146,7 @@ int null(int* x) {
146146
// CHECK: [[HANDLER_ADD_OVERFLOW]]:
147147
// CHECK-NEXT: [[TMP3:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
148148
// CHECK-NEXT: [[TMP4:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
149-
// CHECK-NEXT: tail call void @__ubsan_handle_add_overflow_abort(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR6]], !nosanitize [[META2]]
149+
// CHECK-NEXT: tail call void @__ubsan_handle_add_overflow_abort(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR8]], !nosanitize [[META2]]
150150
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
151151
// CHECK: [[CONT]]:
152152
// CHECK-NEXT: [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
@@ -178,7 +178,7 @@ int null(int* x) {
178178
// REC: [[HANDLER_ADD_OVERFLOW]]:
179179
// REC-NEXT: [[TMP3:%.*]] = zext i32 [[X]] to i64, !nosanitize [[META2]]
180180
// REC-NEXT: [[TMP4:%.*]] = zext i32 [[Y]] to i64, !nosanitize [[META2]]
181-
// REC-NEXT: tail call void @__ubsan_handle_add_overflow(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR6]], !nosanitize [[META2]]
181+
// REC-NEXT: tail call void @__ubsan_handle_add_overflow(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[TMP3]], i64 [[TMP4]]) #[[ATTR8]], !nosanitize [[META2]]
182182
// REC-NEXT: br label %[[CONT]], !nosanitize [[META2]]
183183
// REC: [[CONT]]:
184184
// REC-NEXT: [[TMP5:%.*]] = extractvalue { i32, i1 } [[TMP0]], 0, !nosanitize [[META2]]
@@ -191,11 +191,11 @@ int overflow(int x, int y) {
191191
void use(double*);
192192

193193
// CHECK-LABEL: define dso_local double @lbounds(
194-
// CHECK-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR0]] {
194+
// CHECK-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR5:[0-9]+]] {
195195
// CHECK-NEXT: [[ENTRY:.*:]]
196196
// CHECK-NEXT: [[TMP0:%.*]] = zext i32 [[B]] to i64
197197
// CHECK-NEXT: [[VLA:%.*]] = alloca double, i64 [[TMP0]], align 16
198-
// CHECK-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR7:[0-9]+]]
198+
// CHECK-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR9:[0-9]+]]
199199
// CHECK-NEXT: [[IDXPROM:%.*]] = sext i32 [[I]] to i64
200200
// CHECK-NEXT: [[TMP1:%.*]] = icmp ule i64 [[TMP0]], [[IDXPROM]]
201201
//
@@ -208,7 +208,7 @@ void use(double*);
208208
// CHECK-NEXT: [[TMP5:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !tbaa [[TBAA9:![0-9]+]]
209209
// CHECK-NEXT: ret double [[TMP5]]
210210
// CHECK: [[TRAP]]:
211-
// CHECK-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR6]], !nosanitize [[META2]]
211+
// CHECK-NEXT: call void @__ubsan_handle_local_out_of_bounds_abort() #[[ATTR8]], !nosanitize [[META2]]
212212
// CHECK-NEXT: unreachable, !nosanitize [[META2]]
213213
//
214214
// TR-LABEL: define dso_local double @lbounds(
@@ -231,11 +231,11 @@ void use(double*);
231231
// TR-NEXT: unreachable, !nosanitize [[META2]]
232232
//
233233
// REC-LABEL: define dso_local double @lbounds(
234-
// REC-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR0]] {
234+
// REC-SAME: i32 noundef [[B:%.*]], i32 noundef [[I:%.*]]) local_unnamed_addr #[[ATTR5:[0-9]+]] {
235235
// REC-NEXT: [[ENTRY:.*:]]
236236
// REC-NEXT: [[TMP0:%.*]] = zext i32 [[B]] to i64
237237
// REC-NEXT: [[VLA:%.*]] = alloca double, i64 [[TMP0]], align 16
238-
// REC-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR5:[0-9]+]]
238+
// REC-NEXT: call void @use(ptr noundef nonnull [[VLA]]) #[[ATTR7:[0-9]+]]
239239
// REC-NEXT: [[IDXPROM:%.*]] = sext i32 [[I]] to i64
240240
// REC-NEXT: [[TMP1:%.*]] = icmp ule i64 [[TMP0]], [[IDXPROM]]
241241
// REC-NEXT: [[TMP2:%.*]] = call i1 @llvm.allow.ubsan.check(i8 71), !nosanitize [[META2]]
@@ -246,7 +246,7 @@ void use(double*);
246246
// REC-NEXT: [[TMP5:%.*]] = load double, ptr [[ARRAYIDX]], align 8, !tbaa [[TBAA9:![0-9]+]]
247247
// REC-NEXT: ret double [[TMP5]]
248248
// REC: [[TRAP]]:
249-
// REC-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR6]], !nosanitize [[META2]]
249+
// REC-NEXT: call void @__ubsan_handle_local_out_of_bounds() #[[ATTR8]], !nosanitize [[META2]]
250250
// REC-NEXT: br label %[[BB4]], !nosanitize [[META2]]
251251
//
252252
double lbounds(int b, int i) {

0 commit comments

Comments
 (0)