Skip to content

Commit 9dbf701

Browse files
fmayergithub-actions[bot]
fmayer
authored and
github-actions[bot]
committed
Automerge: [UBSan] Use -fsanitize-handler-preserve-all-regs in codegen
Pull Request: llvm/llvm-project#168645
2 parents 6682191 + e2a29ec commit 9dbf701

File tree

11 files changed

+84
-14
lines changed

11 files changed

+84
-14
lines changed

‎clang/lib/CodeGen/BackendUtil.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
11341134
CodeGenOpts.SanitizeMinimalRuntime),
11351135
/*MayReturn=*/
11361136
CodeGenOpts.SanitizeRecover.has(SanitizerKind::LocalBounds),
1137+
/*HandlerPreserveAllRegs=*/
1138+
static_cast<bool>(CodeGenOpts.SanitizeHandlerPreserveAllRegs),
11371139
};
11381140
}
11391141
FPM.addPass(BoundsCheckingPass(Options));

‎clang/lib/CodeGen/CGExpr.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3819,6 +3819,8 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
38193819
bool NeedsAbortSuffix =
38203820
IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
38213821
bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;
3822+
bool HandlerPreserveAllRegs =
3823+
CGF.CGM.getCodeGenOpts().SanitizeHandlerPreserveAllRegs;
38223824
const SanitizerHandlerInfo &CheckInfo = SanitizerHandlers[CheckHandler];
38233825
const StringRef CheckName = CheckInfo.Name;
38243826
std::string FnName = "__ubsan_handle_" + CheckName.str();
@@ -3828,6 +3830,8 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
38283830
FnName += "_minimal";
38293831
if (NeedsAbortSuffix)
38303832
FnName += "_abort";
3833+
if (HandlerPreserveAllRegs && !NeedsAbortSuffix)
3834+
FnName += "_preserve";
38313835
bool MayReturn =
38323836
!IsFatal || RecoverKind == CheckRecoverableKind::AlwaysRecoverable;
38333837

@@ -3848,6 +3852,10 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
38483852
(CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
38493853
if (NoMerge)
38503854
HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
3855+
if (HandlerPreserveAllRegs && !NeedsAbortSuffix) {
3856+
// N.B. there is also a clang::CallingConv which is not what we want here.
3857+
HandlerCall->setCallingConv(llvm::CallingConv::PreserveAll);
3858+
}
38513859
if (!MayReturn) {
38523860
HandlerCall->setDoesNotReturn();
38533861
CGF.Builder.CreateUnreachable();

‎clang/lib/Driver/SanitizerArgs.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,14 +419,16 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
419419
const Driver &D = TC.getDriver();
420420
SanitizerMask TrappingKinds = parseSanitizeTrapArgs(D, Args, DiagnoseErrors);
421421
SanitizerMask InvalidTrappingKinds = TrappingKinds & NotAllowedWithTrap;
422+
const llvm::Triple &Triple = TC.getTriple();
422423

423424
MinimalRuntime =
424425
Args.hasFlag(options::OPT_fsanitize_minimal_runtime,
425426
options::OPT_fno_sanitize_minimal_runtime, MinimalRuntime);
426427
HandlerPreserveAllRegs =
427428
Args.hasFlag(options::OPT_fsanitize_handler_preserve_all_regs,
428429
options::OPT_fno_sanitize_handler_preserve_all_regs,
429-
HandlerPreserveAllRegs);
430+
HandlerPreserveAllRegs) &&
431+
MinimalRuntime && (Triple.isAArch64() || Triple.isX86_64());
430432

431433
// The object size sanitizer should not be enabled at -O0.
432434
Arg *OptLevel = Args.getLastArg(options::OPT_O_Group);
@@ -494,7 +496,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
494496
// -fsanitize=function and -fsanitize=kcfi instrument indirect function
495497
// calls to load a type hash before the function label. Therefore, an
496498
// execute-only target doesn't support the function and kcfi sanitizers.
497-
const llvm::Triple &Triple = TC.getTriple();
498499
if (isExecuteOnlyTarget(Triple, Args)) {
499500
if (SanitizerMask KindsToDiagnose =
500501
Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {

‎clang/test/CodeGen/cfi-icall-trap-recover-runtime.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ void xf();
171171
// PRESERVE_MIN-NEXT: [[TMP3:%.*]] = call i1 @llvm.type.test(ptr [[TMP2]], metadata !"_ZTSFvE"), !nosanitize [[META10:![0-9]+]]
172172
// PRESERVE_MIN-NEXT: br i1 [[TMP3]], label %[[CONT:.*]], label %[[HANDLER_CFI_CHECK_FAIL:.*]], !prof [[PROF11:![0-9]+]], !nosanitize [[META10]]
173173
// PRESERVE_MIN: [[HANDLER_CFI_CHECK_FAIL]]:
174-
// PRESERVE_MIN-NEXT: call void @__ubsan_handle_cfi_check_fail_minimal() #[[ATTR4:[0-9]+]], !nosanitize [[META10]]
174+
// PRESERVE_MIN-NEXT: call preserve_allcc void @__ubsan_handle_cfi_check_fail_minimal_preserve() #[[ATTR4:[0-9]+]], !nosanitize [[META10]]
175175
// PRESERVE_MIN-NEXT: br label %[[CONT]], !nosanitize [[META10]]
176176
// PRESERVE_MIN: [[CONT]]:
177177
// PRESERVE_MIN-NEXT: call void (...) [[TMP2]]()

‎clang/test/CodeGenCXX/cfi-vcall-trap-recover-runtime.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ struct S1 {
127127
// PRESERVE_MIN-NEXT: [[TMP2:%.*]] = call i1 @llvm.type.test(ptr [[VTABLE]], metadata !"all-vtables"), !nosanitize [[META5]]
128128
// PRESERVE_MIN-NEXT: br i1 [[TMP1]], label %[[CONT:.*]], label %[[HANDLER_CFI_CHECK_FAIL:.*]], !prof [[PROF6:![0-9]+]], !nosanitize [[META5]]
129129
// PRESERVE_MIN: [[HANDLER_CFI_CHECK_FAIL]]:
130-
// PRESERVE_MIN-NEXT: call void @__ubsan_handle_cfi_check_fail_minimal() #[[ATTR3:[0-9]+]], !nosanitize [[META5]]
130+
// PRESERVE_MIN-NEXT: call preserve_allcc void @__ubsan_handle_cfi_check_fail_minimal_preserve() #[[ATTR3:[0-9]+]], !nosanitize [[META5]]
131131
// PRESERVE_MIN-NEXT: br label %[[CONT]], !nosanitize [[META5]]
132132
// PRESERVE_MIN: [[CONT]]:
133133
// PRESERVE_MIN-NEXT: [[VFN:%.*]] = getelementptr inbounds ptr, ptr [[VTABLE]], i64 0

‎clang/test/Driver/fsanitize.c‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -984,10 +984,20 @@
984984
// CHECK-UBSAN-MINIMAL: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
985985
// CHECK-UBSAN-MINIMAL: "-fsanitize-minimal-runtime"
986986

987-
// RUN: %clang --target=x86_64-linux-gnu -fsanitize=undefined -fsanitize-minimal-runtime -fsanitize-handler-preserve-all-regs %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-MINIMAL-PRESERVE
988-
// CHECK-UBSAN-MINIMAL-PRESERVE: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
989-
// CHECK-UBSAN-MINIMAL-PRESERVE: "-fsanitize-minimal-runtime"
990-
// CHECK-UBSAN-MINIMAL-PRESERVE: "-fsanitize-handler-preserve-all-regs
987+
// RUN: %clang --target=x86_64-linux-gnu -fsanitize=undefined -fsanitize-minimal-runtime -fsanitize-handler-preserve-all-regs %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-MINIMAL-PRESERVE-X86-64
988+
// CHECK-UBSAN-MINIMAL-PRESERVE-X86-64: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
989+
// CHECK-UBSAN-MINIMAL-PRESERVE-X86-64: "-fsanitize-minimal-runtime"
990+
// CHECK-UBSAN-MINIMAL-PRESERVE-X86-64: "-fsanitize-handler-preserve-all-regs
991+
992+
// RUN: %clang --target=aarch64-linux-gnu -fsanitize=undefined -fsanitize-minimal-runtime -fsanitize-handler-preserve-all-regs %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-MINIMAL-PRESERVE-AARCH64
993+
// CHECK-UBSAN-MINIMAL-PRESERVE-AARCH64: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
994+
// CHECK-UBSAN-MINIMAL-PRESERVE-AARCH64: "-fsanitize-minimal-runtime"
995+
// CHECK-UBSAN-MINIMAL-PRESERVE-AARCH64: "-fsanitize-handler-preserve-all-regs
996+
997+
// RUN: %clang --target=i386-linux-gnu -fsanitize=undefined -fsanitize-minimal-runtime -fsanitize-handler-preserve-all-regs %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-MINIMAL-PRESERVE-I386
998+
// CHECK-UBSAN-MINIMAL-PRESERVE-I386: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
999+
// CHECK-UBSAN-MINIMAL-PRESERVE-I386: "-fsanitize-minimal-runtime"
1000+
// CHECK-UBSAN-MINIMAL-PRESERVE-I386-NOT: "-fsanitize-handler-preserve-all-regs
9911001

9921002
// RUN: %clang --target=x86_64-linux-gnu -fsanitize=integer -fsanitize-trap=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTSAN-TRAP
9931003
// CHECK-INTSAN-TRAP: "-fsanitize-trap=integer-divide-by-zero,shift-base,shift-exponent,signed-integer-overflow,unsigned-integer-overflow,unsigned-shift-base,implicit-unsigned-integer-truncation,implicit-signed-integer-truncation,implicit-integer-sign-change"

‎compiler-rt/test/ubsan_minimal/TestCases/override-callback.c‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 2>&1 | FileCheck %s
2-
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change -fno-sanitize-recover=all %s -o %t && not --crash %run %t 2>&1 | FileCheck %s
3-
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change -fno-sanitize-recover=all -DOVERRIDE=1 %s -o %t && not --crash %run %t 2>&1 | FileCheck %s --check-prefixes=FATAL
1+
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 2>&1 | FileCheck %s
2+
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change -fsanitize-handler-preserve-all-regs -DPRESERVE %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=PRESERVE
3+
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change -fno-sanitize-recover=all %s -o %t && not --crash %run %t 2>&1 | FileCheck %s
4+
// RUN: %clang_min_runtime -fsanitize=implicit-integer-sign-change -fno-sanitize-recover=all -DOVERRIDE=1 %s -o %t && not --crash %run %t 2>&1 | FileCheck %s --check-prefixes=FATAL
45

56
#include <stdint.h>
67
#include <stdio.h>
@@ -9,8 +10,21 @@
910
static int Result;
1011

1112
void __ubsan_report_error(const char *kind, uintptr_t caller) {
13+
// -fsanitize-handler-preserve-all-regs is ignored on other architectures.
14+
// Prented we called to other handler on those.
15+
#if defined(PRESERVE) && !defined(__aarch64__) && !defined(__x86_64__)
16+
fprintf(stderr, "CUSTOM_CALLBACK_PRESERVE: %s\n", kind);
17+
#else
1218
fprintf(stderr, "CUSTOM_CALLBACK: %s\n", kind);
19+
#endif
20+
}
21+
22+
#if defined(__aarch64__) || defined(__x86_64__)
23+
[[clang::preserve_all]] void __ubsan_report_error_preserve(const char *kind,
24+
uintptr_t caller) {
25+
fprintf(stderr, "CUSTOM_CALLBACK_PRESERVE: %s\n", kind);
1326
}
27+
#endif
1428

1529
#if OVERRIDE
1630
void __ubsan_report_error_fatal(const char *kind, uintptr_t caller) {
@@ -21,5 +35,6 @@ void __ubsan_report_error_fatal(const char *kind, uintptr_t caller) {
2135
int main(int argc, const char **argv) {
2236
int32_t t0 = (~((uint32_t)0));
2337
// CHECK: CUSTOM_CALLBACK: implicit-conversion
38+
// PRESERVE: CUSTOM_CALLBACK_PRESERVE: implicit-conversion
2439
// FATAL: FATAL_CALLBACK: implicit-conversion
2540
}

‎llvm/include/llvm/Transforms/Instrumentation/BoundsChecking.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "llvm/IR/PassManager.h"
1313
#include "llvm/Support/Compiler.h"
14+
#include "llvm/TargetParser/Triple.h"
1415
#include <optional>
1516

1617
namespace llvm {
@@ -23,10 +24,12 @@ class BoundsCheckingPass : public PassInfoMixin<BoundsCheckingPass> {
2324
public:
2425
struct Options {
2526
struct Runtime {
26-
Runtime(bool MinRuntime, bool MayReturn)
27-
: MinRuntime(MinRuntime), MayReturn(MayReturn) {}
27+
Runtime(bool MinRuntime, bool MayReturn, bool HandlerPreserveAllRegs)
28+
: MinRuntime(MinRuntime), MayReturn(MayReturn),
29+
HandlerPreserveAllRegs(HandlerPreserveAllRegs) {}
2830
bool MinRuntime;
2931
bool MayReturn;
32+
bool HandlerPreserveAllRegs;
3033
};
3134
std::optional<Runtime> Rt; // Trap if empty.
3235
bool Merge = false;

‎llvm/lib/Passes/PassBuilder.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,24 +1590,31 @@ parseBoundsCheckingOptions(StringRef Params) {
15901590
Options.Rt = {
15911591
/*MinRuntime=*/false,
15921592
/*MayReturn=*/true,
1593+
/*HandlerPreserveAllRegs=*/false,
15931594
};
15941595
} else if (ParamName == "rt-abort") {
15951596
Options.Rt = {
15961597
/*MinRuntime=*/false,
15971598
/*MayReturn=*/false,
1599+
/*HandlerPreserveAllRegs=*/false,
15981600
};
15991601
} else if (ParamName == "min-rt") {
16001602
Options.Rt = {
16011603
/*MinRuntime=*/true,
16021604
/*MayReturn=*/true,
1605+
/*HandlerPreserveAllRegs=*/false,
16031606
};
16041607
} else if (ParamName == "min-rt-abort") {
16051608
Options.Rt = {
16061609
/*MinRuntime=*/true,
16071610
/*MayReturn=*/false,
1611+
/*HandlerPreserveAllRegs=*/false,
16081612
};
16091613
} else if (ParamName == "merge") {
16101614
Options.Merge = true;
1615+
} else if (ParamName == "handler-preserve-all-regs") {
1616+
if (Options.Rt)
1617+
Options.Rt->HandlerPreserveAllRegs = true;
16111618
} else {
16121619
StringRef ParamEQ;
16131620
StringRef Val;

‎llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ getRuntimeCallName(const BoundsCheckingPass::Options::Runtime &Opts) {
178178
Name += "_minimal";
179179
if (!Opts.MayReturn)
180180
Name += "_abort";
181+
else if (Opts.HandlerPreserveAllRegs)
182+
Name += "_preserve";
181183
return Name;
182184
}
183185

@@ -267,7 +269,10 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
267269
TrapCall->setDoesNotReturn();
268270
IRB.CreateUnreachable();
269271
}
270-
272+
// The preserve-all logic is somewhat duplicated in CGExpr.cpp for
273+
// local-bounds. Make sure to change that too.
274+
if (Opts.Rt && Opts.Rt->HandlerPreserveAllRegs && MayReturn)
275+
TrapCall->setCallingConv(CallingConv::PreserveAll);
271276
if (!MayReturn && SingleTrapBB && !DebugTrapBB)
272277
ReuseTrapBB = TrapBB;
273278

0 commit comments

Comments
 (0)