[Reland][Clang][CodeGen][UBSan] Add more precise attributes to recoverable ubsan handlers #135135
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looking at the issue description from #130990, the problem is that the code is passing I'm not sure how #135141 is connected to this. |
Oh, sorry, somehow didn't read the current version of the patch; I see you fixed this. Still not sure how #135141 is related. |
We add a new parameter |
dtcxzyw
added a commit
that referenced
this pull request
Apr 12, 2025
…#135141) `EmitCheckValue` is called inside `EmitCheck`: https://github.com/llvm/llvm-project/blob/b122956390a6877536927c2b073a0b99f8b9704f/clang/lib/CodeGen/CGExpr.cpp#L3739 The outside calls are redundant because `EmitCheckValue(EmitCheckValue(V))` always returns `EmitCheckValue(V)`. Required by #135135.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 12, 2025
…calls. NFCI (#135141) `EmitCheckValue` is called inside `EmitCheck`: https://github.com/llvm/llvm-project/blob/b122956390a6877536927c2b073a0b99f8b9704f/clang/lib/CodeGen/CGExpr.cpp#L3739 The outside calls are redundant because `EmitCheckValue(EmitCheckValue(V))` always returns `EmitCheckValue(V)`. Required by llvm/llvm-project#135135.
dtcxzyw
force-pushed
the
ubsan-handler-attr-v2
branch
from
f89b114 to
0bf78cc
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Yingwei Zheng (dtcxzyw) ChangesThis patch relands #130990. Original PR description: This patch adds Patch is 94.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135135.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5f028f6d8c6ac..fe8ccb713230e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3456,7 +3456,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)
@@ -3482,6 +3483,7 @@ llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
Builder.CreateStore(V, Ptr);
V = Ptr.getPointer();
}
+ MayReadFromPtrToInt = true;
return Builder.CreatePtrToInt(V, TargetTy);
}
@@ -3587,7 +3589,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()) {
@@ -3615,6 +3618,23 @@ 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);
+ }
+ // If the handler does not return, it must interact with the environment in
+ // an observable way.
+ B.addAttribute(llvm::Attribute::MustProgress);
+ }
llvm::FunctionCallee Fn = CGF.CGM.CreateRuntimeFunction(
FnType, FnName,
@@ -3711,6 +3731,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);
@@ -3730,7 +3751,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);
}
}
@@ -3742,7 +3763,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.
@@ -3752,10 +3774,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);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index cdddc69effb86..ac7b2c03bc929 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5224,7 +5224,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.
diff --git a/clang/test/CodeGen/AArch64/ubsan-handler-pass-by-ref.c b/clang/test/CodeGen/AArch64/ubsan-handler-pass-by-ref.c
new file mode 100644
index 0000000000000..a8ff941a124a4
--- /dev/null
+++ b/clang/test/CodeGen/AArch64/ubsan-handler-pass-by-ref.c
@@ -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]] = !{}
+//.
diff --git a/clang/test/CodeGen/allow-ubsan-check.c b/clang/test/CodeGen/allow-ubsan-check.c
index e225fb63f08eb..6358425aa40be 100644
--- a/clang/test/CodeGen/allow-ubsan-check.c
+++ b/clang/test/CodeGen/allow-ubsan-check.c
@@ -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]]
@@ -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]]
@@ -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]]
//
@@ -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]+]]
@@ -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]+]]
@@ -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]]
@@ -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]]
@@ -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]]
//
@@ -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(
@@ -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]]
@@ -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) {
diff --git a/clang/test/CodeGen/attr-counted-by.c b/clang/test/CodeGen/attr-counted-by.c
index dfdf06587f0e2..8e062629464dd 100644
--- a/clang/test/CodeGen/attr-counted-by.c
+++ b/clang/test/CodeGen/attr-counted-by.c
@@ -68,7 +68,7 @@ struct anon_struct {
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = icmp ult i64 [[IDXPROM]], [[TMP0]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT3:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3:![0-9]+]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[IDXPROM]]) #[[ATTR8:[0-9]+]], !nosanitize [[META2]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB1:[0-9]+]], i64 [[IDXPROM]]) #[[ATTR12:[0-9]+]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: cont3:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAY:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 12
@@ -116,7 +116,7 @@ void test1(struct annotated *p, int index, int val) {
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = icmp ult i64 [[INDEX]], [[TMP0]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT6:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[INDEX]]) #[[ATTR8]], !nosanitize [[META2]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB3:[0-9]+]], i64 [[INDEX]]) #[[ATTR12]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: cont6:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAY:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 12
@@ -159,7 +159,7 @@ void test2(struct annotated *p, size_t index) {
}
// SANITIZE-WITH-ATTR-LABEL: define dso_local range(i64 -8589934592, 8589934589) i64 @test2_bdos(
-// SANITIZE-WITH-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// SANITIZE-WITH-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_GEP:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 8
// SANITIZE-WITH-ATTR-NEXT: [[COUNTED_BY_LOAD:%.*]] = load i32, ptr [[COUNTED_BY_GEP]], align 4
@@ -181,7 +181,7 @@ void test2(struct annotated *p, size_t index) {
// NO-SANITIZE-WITH-ATTR-NEXT: ret i64 [[TMP1]]
//
// SANITIZE-WITHOUT-ATTR-LABEL: define dso_local i64 @test2_bdos(
-// SANITIZE-WITHOUT-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// SANITIZE-WITHOUT-ATTR-SAME: ptr noundef [[P:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
// SANITIZE-WITHOUT-ATTR-NEXT: entry:
// SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -1
//
@@ -203,7 +203,7 @@ size_t test2_bdos(struct annotated *p) {
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = icmp ult i64 [[INDEX]], [[TMP0]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT3:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB4:[0-9]+]], i64 [[INDEX]]) #[[ATTR8]], !nosanitize [[META2]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB4:[0-9]+]], i64 [[INDEX]]) #[[ATTR12]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: cont3:
// SANITIZE-WITH-ATTR-NEXT: [[ARRAY:%.*]] = getelementptr inbounds nuw i8, ptr [[P]], i64 12
@@ -242,7 +242,7 @@ void test3(struct annotated *p, size_t index) {
}
// SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @test3_bdos(
-// SANITIZE-WITH-ATTR-SAME: ptr noundef readnone [[P:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
+// SANITIZE-WITH-ATTR-SAME: ptr noundef readnone [[P:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] {
// SANITIZE-WITH-ATTR-NEXT: entry:
// SANITIZE-WITH-ATTR-NEXT: ret i64 -1
//
@@ -252,7 +252,7 @@ void test3(struct annotated *p, size_t index) {
// NO-SANITIZE-WITH-ATTR-NEXT: ret i64 -1
//
// SANITIZE-WITHOUT-ATTR-LABEL: define dso_local i64 @test3_bdos(
-// SANITIZE-WITHOUT-ATTR-SAME: ptr noundef readnone [[P:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
+// SANITIZE-WITHOUT-ATTR-SAME: ptr noundef readnone [[P:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] {
// SANITIZE-WITHOUT-ATTR-NEXT: entry:
// SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -1
//
@@ -275,7 +275,7 @@ size_t test3_bdos(struct annotated *p) {
// SANITIZE-WITH-ATTR-NEXT: [[TMP1:%.*]] = icmp ugt i32 [[DOTCOUNTED_BY_LOAD]], 2
// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP1]], label [[CONT1:%.*]], label [[HANDLER_OUT_OF_BOUNDS:%.*]], !prof [[PROF3]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: handler.out_of_bounds:
-// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB5:[0-9]+]], i64 3) #[[ATTR8]], !nosanitize [[META2]]
+// SANITIZE-WITH-ATTR-NEXT: tail call void @__ubsan_handle_out_of_bounds_abort(ptr nonnull @[[GLOB5:[0-9]+]], i64 3) #[[ATTR12]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: unreachable, !nosanitize [[META2]]
// SANITIZE-WITH-ATTR: cont1:
// SANITIZE-WITH-ATTR-NEXT: [[FLEXIBLE_ARRAY_MEMBER_SIZE:%.*]] = shl i32 [[DOTCOUNTED_BY_LOAD]], 2
@@ -283,7 +283,7 @@ size_t test3_bdos(struct annotated *p) {
// SANITIZE-WITH-ATTR-NEXT: [[TMP2:%.*]] = icmp ult i64 [[IDXPROM]], [[TMP0]], !nosanitize [[META2]]
// SANITIZE-WITH-ATTR-NEXT: br i1 [[TMP2...
[truncated]
|
clang/lib/CodeGen/CGExpr.cpp
Outdated
| } | ||
| // If the handler does not return, it must interact with the environment in | ||
| // an observable way. | ||
| B.addAttribute(llvm::Attribute::MustProgress); |
There was a problem hiding this comment.
I know this isn't new... but I don't think this "mustprogress" marking actually does anything useful. "mustprogress" is more a characteristic of function definitions, not calls.
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…llvm#135141) `EmitCheckValue` is called inside `EmitCheck`: https://github.com/llvm/llvm-project/blob/b122956390a6877536927c2b073a0b99f8b9704f/clang/lib/CodeGen/CGExpr.cpp#L3739 The outside calls are redundant because `EmitCheckValue(EmitCheckValue(V))` always returns `EmitCheckValue(V)`. Required by llvm#135135.
…san handlers (llvm#130990) This patch adds `memory(argmem: read, inaccessiblemem: readwrite) mustprogress` 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). Closes llvm#130093.
dtcxzyw
force-pushed
the
ubsan-handler-attr-v2
branch
from
0bf78cc to
ca45b63
Compare
|
This change broke a couple of ubsan tests on i686 on Windows (i.e. a 32 bit environment): https://github.com/mstorsjo/llvm-mingw/actions/runs/14527591706/job/40770945907 The failing tests are https://github.com/llvm/llvm-project/blob/llvmorg-20.1.3/compiler-rt/test/ubsan/TestCases/Misc/abs.cpp and https://github.com/llvm/llvm-project/blob/llvmorg-20.1.3/compiler-rt/test/ubsan/TestCases/ImplicitConversion/bitfield-conversion.c. The diff in ubsan output from @@ -6,7 +6,7 @@
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:19:18
compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:20:8: runtime error: negation of -2147483648 cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:20:8
-compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:24:19: runtime error: negation of -9223372036854775808 cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
+compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:24:19: runtime error: negation of 14963666063463584 cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:24:19
-compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:25:9: runtime error: negation of -9223372036854775808 cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
+compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:25:9: runtime error: negation of 8880086229349853276 cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior compiler-rt/test/ubsan/TestCases/Misc/abs.cpp:25:9 |
IR output is correct. And the codegen looks good. |
|
@vitalybuka @mstorsjo Can you please provide the command to reproduce the wrong IR/codegen? |
|
With https://martin.st/temp/abs-preproc.cpp as input, I run --- abs-good.ll 2025-04-20 00:10:20.155904358 +0300
+++ abs-bad.ll 2025-04-20 00:10:30.432882049 +0300
@@ -14,7 +14,7 @@
@7 = private unnamed_addr global { { ptr, i32, i32 }, ptr } { { ptr, i32, i32 } { ptr @.src, i32 481, i32 19 }, ptr @6 }
@8 = private unnamed_addr global { { ptr, i32, i32 }, ptr } { { ptr, i32, i32 } { ptr @.src, i32 482, i32 9 }, ptr @6 }
-; Function Attrs: mustprogress norecurse nounwind
+; Function Attrs: mustprogress norecurse nounwind memory(read, argmem: none, inaccessiblemem: readwrite)
define dso_local noundef i32 @main() local_unnamed_addr #0 {
entry:
%tmp = alloca i64, align 8
@@ -23,20 +23,18 @@
tail call void @__ubsan_handle_negate_overflow(ptr nonnull @2, i32 -2147483648) #2, !nosanitize !6
tail call void @__ubsan_handle_negate_overflow(ptr nonnull @4, i32 -2147483648) #2, !nosanitize !6
tail call void @__ubsan_handle_negate_overflow(ptr nonnull @5, i32 -2147483648) #2, !nosanitize !6
- store i64 -9223372036854775808, ptr %tmp, align 8, !nosanitize !6
%0 = ptrtoint ptr %tmp to i32, !nosanitize !6
call void @__ubsan_handle_negate_overflow(ptr nonnull @7, i32 %0) #2, !nosanitize !6
- store i64 -9223372036854775808, ptr %tmp14, align 8, !nosanitize !6
%1 = ptrtoint ptr %tmp14 to i32, !nosanitize !6
call void @__ubsan_handle_negate_overflow(ptr nonnull @8, i32 %1) #2, !nosanitize !6
ret i32 0
}
-; Function Attrs: uwtable
+; Function Attrs: memory(argmem: read, inaccessiblemem: readwrite) uwtable
declare dso_local void @__ubsan_handle_negate_overflow(ptr, i32) local_unnamed_addr #1
-attributes #0 = { mustprogress norecurse nounwind "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-attributes #1 = { uwtable }
+attributes #0 = { mustprogress norecurse nounwind memory(read, argmem: none, inaccessiblemem: readwrite) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { memory(argmem: read, inaccessiblemem: readwrite) uwtable }
attributes #2 = { nounwind }
!llvm.module.flags = !{!0, !1, !2} |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 6, 2025
…ttributes to recoverable ubsan handlers" (#136402) Reverts llvm/llvm-project#135135 Breaks several bots, details in #135135.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…rable ubsan handlers (llvm#135135) This patch relands llvm#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).
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…o recoverable ubsan handlers" (llvm#136402) Reverts llvm#135135 Breaks several bots, details in llvm#135135.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…o recoverable ubsan handlers" (llvm#136402) Reverts llvm#135135 Breaks several bots, details in llvm#135135.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…rable ubsan handlers (llvm#135135) This patch relands llvm#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).
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…o recoverable ubsan handlers" (llvm#136402) Reverts llvm#135135 Breaks several bots, details in llvm#135135.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 somememory/loop optimizations. It provides an average of 3% performance
improvement on llvm-test-suite (except for 49 test failures due to ubsan
diagnostics).