Skip to content

Commit 5bb6503

Browse files
thurstondthurstond
authored
Remove -bounds-checking-unique-traps (replace with -fno-sanitize-merge=local-bounds) (#120682)
#120613 removed -ubsan-unique-traps and replaced it with -fno-sanitize-merge (introduced in #120511), which allows fine-grained control of which UBSan checks to prevent merging. This analogous patch removes -bound-checking-unique-traps, and allows it to be controlled via -fno-sanitize-merge=local-bounds. Most of this patch is simply plumbing through the compiler flags into the bounds checking pass. Note: this patch subtly changes -fsanitize-merge (the default) to also include -fsanitize-merge=local-bounds. This is different from the previous behavior, where -fsanitize-merge (or the old -ubsan-unique-traps) did not affect local-bounds (requiring the separate -bounds-checking-unique-traps). However, we argue that the new behavior is more intuitive. Removing -bounds-checking-unique-traps and merging its functionality into -fsanitize-merge breaks backwards compatibility; we hope that this is acceptable since '-mllvm -bounds-checking-unique-traps' was an experimental flag.
1 parent 82b5bda commit 5bb6503

File tree

13 files changed

+169
-102
lines changed

13 files changed

+169
-102
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,6 @@ New Compiler Flags
445445
- The ``-Warray-compare-cxx26`` warning has been added to warn about array comparison
446446
starting from C++26, this warning is enabled as an error by default.
447447

448-
- '-fsanitize-merge' (default) and '-fno-sanitize-merge' have been added for
449-
fine-grained control of which UBSan checks are allowed to be merged by the
450-
backend (for example, -fno-sanitize-merge=bool,enum).
451-
452448
Deprecated Compiler Flags
453449
-------------------------
454450

@@ -488,8 +484,6 @@ Removed Compiler Flags
488484
derivatives) is now removed, since it's no longer possible to suppress the
489485
diagnostic (see above). Users can expect an `unknown warning` diagnostic if
490486
it's still in use.
491-
- The experimental flag '-ubsan-unique-traps' has been removed. It is
492-
superseded by '-fno-sanitize-merge'.
493487

494488
Attribute Changes in Clang
495489
--------------------------
@@ -1213,6 +1207,11 @@ Sanitizers
12131207

12141208
- Implemented ``-f[no-]sanitize-trap=local-bounds``, and ``-f[no-]sanitize-recover=local-bounds``.
12151209

1210+
- ``-fsanitize-merge`` (default) and ``-fno-sanitize-merge`` have been added for
1211+
fine-grained, unified control of which UBSan checks can potentially be merged
1212+
by the compiler (for example,
1213+
``-fno-sanitize-merge=bool,enum,array-bounds,local-bounds``).
1214+
12161215
Python Binding Changes
12171216
----------------------
12181217
- Fixed an issue that led to crashes when calling ``Type.get_exception_specification_kind``.

‎clang/docs/UndefinedBehaviorSanitizer.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ Stack traces and report symbolization
276276
If you want UBSan to print symbolized stack trace for each error report, you
277277
will need to:
278278

279-
#. Compile with ``-g`` and ``-fno-omit-frame-pointer`` to get proper debug
280-
information in your binary.
279+
#. Compile with ``-g``, ``-fno-sanitize-merge`` and ``-fno-omit-frame-pointer``
280+
to get proper debug information in your binary.
281281
#. Run your program with environment variable
282282
``UBSAN_OPTIONS=print_stacktrace=1``.
283283
#. Make sure ``llvm-symbolizer`` binary is in ``PATH``.

‎clang/lib/CodeGen/BackendUtil.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,9 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
10301030
PB.registerScalarOptimizerLateEPCallback(
10311031
[this](FunctionPassManager &FPM, OptimizationLevel Level) {
10321032
BoundsCheckingPass::ReportingMode Mode;
1033+
bool Merge = CodeGenOpts.SanitizeMergeHandlers.has(
1034+
SanitizerKind::LocalBounds);
1035+
10331036
if (CodeGenOpts.SanitizeTrap.has(SanitizerKind::LocalBounds)) {
10341037
Mode = BoundsCheckingPass::ReportingMode::Trap;
10351038
} else if (CodeGenOpts.SanitizeMinimalRuntime) {
@@ -1041,7 +1044,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
10411044
? BoundsCheckingPass::ReportingMode::FullRuntime
10421045
: BoundsCheckingPass::ReportingMode::FullRuntimeAbort;
10431046
}
1044-
FPM.addPass(BoundsCheckingPass(Mode));
1047+
BoundsCheckingPass::BoundsCheckingOptions Options(Mode, Merge);
1048+
FPM.addPass(BoundsCheckingPass(Options));
10451049
});
10461050

10471051
// Don't add sanitizers if we are here from ThinLTO PostLink. That already

‎clang/test/CodeGen/bounds-checking.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1-
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
2-
// RUN: %clang_cc1 -fsanitize=array-bounds -O -emit-llvm -triple x86_64-apple-darwin10 %s -o - | not FileCheck %s
1+
// N.B. The clang driver defaults to -fsanitize-merge but clang_cc1 effectively
2+
// defaults to -fno-sanitize-merge.
33
// RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s
4+
// RUN: %clang_cc1 -fsanitize=array-bounds -O -emit-llvm -triple x86_64-apple-darwin10 %s -o - | not FileCheck %s
45
//
5-
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -O3 -mllvm -bounds-checking-unique-traps -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTLOCAL
6-
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | not FileCheck %s --check-prefixes=NOOPTLOCAL
6+
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
7+
//
8+
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTLOCAL
9+
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -fno-sanitize-merge -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTLOCAL
10+
// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -fsanitize-merge=local-bounds -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | not FileCheck %s --check-prefixes=NOOPTLOCAL
711
//
8-
// N.B. The clang driver defaults to -fsanitize-merge but clang_cc1 effectively
9-
// defaults to -fno-sanitize-merge.
1012
// RUN: %clang_cc1 -fsanitize=array-bounds -fsanitize-trap=array-bounds -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTARRAY
1113
// RUN: %clang_cc1 -fsanitize=array-bounds -fsanitize-trap=array-bounds -fno-sanitize-merge -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTARRAY
1214
// RUN: %clang_cc1 -fsanitize=array-bounds -fsanitize-trap=array-bounds -fsanitize-merge=array-bounds -O3 -emit-llvm -triple x86_64-apple-darwin10 %s -o - | not FileCheck %s --check-prefixes=NOOPTARRAY

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class Function;
1717
/// A pass to instrument code and perform run-time bounds checking on loads,
1818
/// stores, and other memory intrinsics.
1919
class BoundsCheckingPass : public PassInfoMixin<BoundsCheckingPass> {
20+
2021
public:
2122
enum class ReportingMode {
2223
Trap,
@@ -26,15 +27,21 @@ class BoundsCheckingPass : public PassInfoMixin<BoundsCheckingPass> {
2627
FullRuntimeAbort,
2728
};
2829

29-
private:
30-
ReportingMode Mode = ReportingMode::Trap;
30+
struct BoundsCheckingOptions {
31+
BoundsCheckingOptions(ReportingMode Mode, bool Merge);
3132

32-
public:
33-
BoundsCheckingPass(ReportingMode Mode) : Mode(Mode) {}
33+
ReportingMode Mode;
34+
bool Merge;
35+
};
36+
37+
BoundsCheckingPass(BoundsCheckingOptions Options) : Options(Options) {}
3438
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
3539
static bool isRequired() { return true; }
3640
void printPipeline(raw_ostream &OS,
3741
function_ref<StringRef(StringRef)> MapClassName2PassName);
42+
43+
private:
44+
BoundsCheckingOptions Options;
3845
};
3946

4047
} // end namespace llvm

‎llvm/lib/Passes/PassBuilder.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,31 +1281,33 @@ parseRegAllocFastPassOptions(PassBuilder &PB, StringRef Params) {
12811281
return Opts;
12821282
}
12831283

1284-
Expected<BoundsCheckingPass::ReportingMode>
1284+
Expected<BoundsCheckingPass::BoundsCheckingOptions>
12851285
parseBoundsCheckingOptions(StringRef Params) {
1286-
BoundsCheckingPass::ReportingMode Mode =
1287-
BoundsCheckingPass::ReportingMode::Trap;
1286+
BoundsCheckingPass::BoundsCheckingOptions Options(
1287+
BoundsCheckingPass::ReportingMode::Trap, false);
12881288
while (!Params.empty()) {
12891289
StringRef ParamName;
12901290
std::tie(ParamName, Params) = Params.split(';');
12911291
if (ParamName == "trap") {
1292-
Mode = BoundsCheckingPass::ReportingMode::Trap;
1292+
Options.Mode = BoundsCheckingPass::ReportingMode::Trap;
12931293
} else if (ParamName == "rt") {
1294-
Mode = BoundsCheckingPass::ReportingMode::FullRuntime;
1294+
Options.Mode = BoundsCheckingPass::ReportingMode::FullRuntime;
12951295
} else if (ParamName == "rt-abort") {
1296-
Mode = BoundsCheckingPass::ReportingMode::FullRuntimeAbort;
1296+
Options.Mode = BoundsCheckingPass::ReportingMode::FullRuntimeAbort;
12971297
} else if (ParamName == "min-rt") {
1298-
Mode = BoundsCheckingPass::ReportingMode::MinRuntime;
1298+
Options.Mode = BoundsCheckingPass::ReportingMode::MinRuntime;
12991299
} else if (ParamName == "min-rt-abort") {
1300-
Mode = BoundsCheckingPass::ReportingMode::MinRuntimeAbort;
1300+
Options.Mode = BoundsCheckingPass::ReportingMode::MinRuntimeAbort;
1301+
} else if (ParamName == "merge") {
1302+
Options.Merge = true;
13011303
} else {
13021304
return make_error<StringError>(
13031305
formatv("invalid BoundsChecking pass parameter '{0}' ", ParamName)
13041306
.str(),
13051307
inconvertibleErrorCode());
13061308
}
13071309
}
1308-
return Mode;
1310+
return Options;
13091311
}
13101312

13111313
} // namespace

‎llvm/lib/Passes/PassRegistry.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -624,8 +624,8 @@ FUNCTION_PASS_WITH_PARAMS(
624624
parseWinEHPrepareOptions, "demote-catchswitch-only")
625625
FUNCTION_PASS_WITH_PARAMS(
626626
"bounds-checking", "BoundsCheckingPass",
627-
[](BoundsCheckingPass::ReportingMode Mode) {
628-
return BoundsCheckingPass(Mode);
627+
[](BoundsCheckingPass::BoundsCheckingOptions Options) {
628+
return BoundsCheckingPass(Options);
629629
},
630630
parseBoundsCheckingOptions, "trap")
631631
#undef FUNCTION_PASS_WITH_PARAMS

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

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,16 @@ using namespace llvm;
3737
static cl::opt<bool> SingleTrapBB("bounds-checking-single-trap",
3838
cl::desc("Use one trap block per function"));
3939

40-
static cl::opt<bool> DebugTrapBB("bounds-checking-unique-traps",
41-
cl::desc("Always use one trap per check"));
42-
4340
STATISTIC(ChecksAdded, "Bounds checks added");
4441
STATISTIC(ChecksSkipped, "Bounds checks skipped");
4542
STATISTIC(ChecksUnable, "Bounds checks unable to add");
4643

4744
using BuilderTy = IRBuilder<TargetFolder>;
4845

46+
BoundsCheckingPass::BoundsCheckingOptions::BoundsCheckingOptions(
47+
ReportingMode Mode, bool Merge)
48+
: Mode(Mode), Merge(Merge) {}
49+
4950
/// Gets the conditions under which memory accessing instructions will overflow.
5051
///
5152
/// \p Ptr is the pointer that will be read/written, and \p InstVal is either
@@ -105,7 +106,7 @@ static Value *getBoundsCheckCond(Value *Ptr, Value *InstVal,
105106
return Or;
106107
}
107108

108-
static CallInst *InsertTrap(BuilderTy &IRB) {
109+
static CallInst *InsertTrap(BuilderTy &IRB, bool DebugTrapBB) {
109110
if (!DebugTrapBB)
110111
return IRB.CreateIntrinsic(Intrinsic::trap, {}, {});
111112
// FIXME: Ideally we would use the SanitizerHandler::OutOfBounds constant.
@@ -169,9 +170,10 @@ struct ReportingOpts {
169170
bool MayReturn = false;
170171
bool UseTrap = false;
171172
bool MinRuntime = false;
173+
bool MayMerge = true;
172174
StringRef Name;
173175

174-
ReportingOpts(BoundsCheckingPass::ReportingMode Mode) {
176+
ReportingOpts(BoundsCheckingPass::ReportingMode Mode, bool Merge) {
175177
switch (Mode) {
176178
case BoundsCheckingPass::ReportingMode::Trap:
177179
UseTrap = true;
@@ -193,6 +195,8 @@ struct ReportingOpts {
193195
Name = "__ubsan_handle_local_out_of_bounds_abort";
194196
break;
195197
}
198+
199+
MayMerge = Merge;
196200
}
197201
};
198202

@@ -253,13 +257,12 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
253257
BasicBlock *TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
254258
IRB.SetInsertPoint(TrapBB);
255259

260+
bool DebugTrapBB = !Opts.MayMerge;
256261
CallInst *TrapCall = Opts.UseTrap
257-
? InsertTrap(IRB)
262+
? InsertTrap(IRB, DebugTrapBB)
258263
: InsertCall(IRB, Opts.MayReturn, Opts.Name);
259-
if (DebugTrapBB) {
260-
// FIXME: Pass option form clang.
264+
if (DebugTrapBB)
261265
TrapCall->addFnAttr(llvm::Attribute::NoMerge);
262-
}
263266

264267
TrapCall->setDoesNotThrow();
265268
TrapCall->setDebugLoc(DebugLoc);
@@ -289,7 +292,8 @@ PreservedAnalyses BoundsCheckingPass::run(Function &F, FunctionAnalysisManager &
289292
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
290293
auto &SE = AM.getResult<ScalarEvolutionAnalysis>(F);
291294

292-
if (!addBoundsChecking(F, TLI, SE, ReportingOpts(Mode)))
295+
if (!addBoundsChecking(F, TLI, SE,
296+
ReportingOpts(Options.Mode, Options.Merge)))
293297
return PreservedAnalyses::all();
294298

295299
return PreservedAnalyses::none();
@@ -299,21 +303,24 @@ void BoundsCheckingPass::printPipeline(
299303
raw_ostream &OS, function_ref<StringRef(StringRef)> MapClassName2PassName) {
300304
static_cast<PassInfoMixin<BoundsCheckingPass> *>(this)->printPipeline(
301305
OS, MapClassName2PassName);
302-
switch (Mode) {
306+
switch (Options.Mode) {
303307
case ReportingMode::Trap:
304-
OS << "<trap>";
308+
OS << "<trap";
305309
break;
306310
case ReportingMode::MinRuntime:
307-
OS << "<min-rt>";
311+
OS << "<min-rt";
308312
break;
309313
case ReportingMode::MinRuntimeAbort:
310-
OS << "<min-rt-abort>";
314+
OS << "<min-rt-abort";
311315
break;
312316
case ReportingMode::FullRuntime:
313-
OS << "<rt>";
317+
OS << "<rt";
314318
break;
315319
case ReportingMode::FullRuntimeAbort:
316-
OS << "<rt-abort>";
320+
OS << "<rt-abort";
317321
break;
318322
}
323+
if (Options.Merge)
324+
OS << ";merge";
325+
OS << ">";
319326
}

‎llvm/test/Instrumentation/BoundsChecking/many-trap.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
; RUN: opt < %s -passes=bounds-checking -S | FileCheck %s
2-
; RUN: opt < %s -passes=bounds-checking -bounds-checking-single-trap -S | FileCheck -check-prefix=SINGLE %s
1+
; RUN: opt < %s -passes='bounds-checking<merge>' -S | FileCheck %s
2+
; RUN: opt < %s -passes='bounds-checking<merge>' -bounds-checking-single-trap -S | FileCheck -check-prefix=SINGLE %s
33
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
44

55
; CHECK: @f1

0 commit comments

Comments
 (0)