Skip to content

Commit ffff7bb

Browse files
authored
Reapply "[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (#120…464)" (#120511)
This reverts commit 2691b96. This reapply fixes the buildbot breakage of the original patch, by updating clang/test/CodeGen/ubsan-trap-debugloc.c to specify -fsanitize-merge (the default, which is merge, is applied by the driver but not clang_cc1). This reapply also expands clang/test/CodeGen/ubsan-trap-merge.c. ---- Original commit message: '-mllvm -ubsan-unique-traps' (#65972) applies to all UBSan checks. This patch introduces -fsanitize-merge (defaults to on, maintaining the status quo behavior) and -fno-sanitize-merge (equivalent to '-mllvm -ubsan-unique-traps'), with the option to selectively applying non-merged handlers to a subset of UBSan checks (e.g., -fno-sanitize-merge=bool,enum). N.B. we do not use "trap" in the argument name since #119302 has generalized -ubsan-unique-traps to work for non-trap modes (min-rt and regular rt). This patch does not remove the -ubsan-unique-traps flag; that will override -f(no-)sanitize-merge.
1 parent 60a2f32 commit ffff7bb

File tree

10 files changed

+624
-51
lines changed

10 files changed

+624
-51
lines changed

clang/include/clang/Basic/CodeGenOptions.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
380380
/// Set of sanitizer checks that trap rather than diagnose.
381381
SanitizerSet SanitizeTrap;
382382

383+
/// Set of sanitizer checks that can merge handlers (smaller code size at
384+
/// the expense of debuggability).
385+
SanitizerSet SanitizeMergeHandlers;
386+
383387
/// List of backend command-line options for -fembed-bitcode.
384388
std::vector<uint8_t> CmdArgs;
385389

clang/include/clang/Driver/Options.td

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2555,6 +2555,21 @@ def fno_sanitize_trap : Flag<["-"], "fno-sanitize-trap">, Group<f_clang_Group>,
25552555
Alias<fno_sanitize_trap_EQ>, AliasArgs<["all"]>,
25562556
Visibility<[ClangOption, CLOption]>,
25572557
HelpText<"Disable trapping for all sanitizers">;
2558+
def fsanitize_merge_handlers_EQ
2559+
: CommaJoined<["-"], "fsanitize-merge=">,
2560+
Group<f_clang_Group>,
2561+
HelpText<"Allow compiler to merge handlers for specified sanitizers">;
2562+
def fno_sanitize_merge_handlers_EQ
2563+
: CommaJoined<["-"], "fno-sanitize-merge=">,
2564+
Group<f_clang_Group>,
2565+
HelpText<"Do not allow compiler to merge handlers for specified sanitizers">;
2566+
def fsanitize_merge_handlers : Flag<["-"], "fsanitize-merge">, Group<f_clang_Group>,
2567+
Alias<fsanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
2568+
HelpText<"Allow compiler to merge handlers for all sanitizers">;
2569+
def fno_sanitize_merge_handlers : Flag<["-"], "fno-sanitize-merge">, Group<f_clang_Group>,
2570+
Alias<fno_sanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
2571+
Visibility<[ClangOption, CLOption]>,
2572+
HelpText<"Do not allow compiler to merge handlers for any sanitizers">;
25582573
def fsanitize_undefined_trap_on_error
25592574
: Flag<["-"], "fsanitize-undefined-trap-on-error">, Group<f_clang_Group>,
25602575
Alias<fsanitize_trap_EQ>, AliasArgs<["undefined"]>;

clang/include/clang/Driver/SanitizerArgs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class SanitizerArgs {
2525
SanitizerSet Sanitizers;
2626
SanitizerSet RecoverableSanitizers;
2727
SanitizerSet TrapSanitizers;
28+
SanitizerSet MergeHandlers;
2829

2930
std::vector<std::string> UserIgnorelistFiles;
3031
std::vector<std::string> SystemIgnorelistFiles;

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3546,7 +3546,7 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
35463546
ArrayRef<llvm::Value *> FnArgs,
35473547
SanitizerHandler CheckHandler,
35483548
CheckRecoverableKind RecoverKind, bool IsFatal,
3549-
llvm::BasicBlock *ContBB) {
3549+
llvm::BasicBlock *ContBB, bool NoMerge) {
35503550
assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
35513551
std::optional<ApplyDebugLocation> DL;
35523552
if (!CGF.Builder.getCurrentDebugLocation()) {
@@ -3581,10 +3581,9 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
35813581
llvm::AttributeList::FunctionIndex, B),
35823582
/*Local=*/true);
35833583
llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
3584-
bool NoMerge =
3585-
ClSanitizeDebugDeoptimization ||
3586-
!CGF.CGM.getCodeGenOpts().OptimizationLevel ||
3587-
(CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
3584+
NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
3585+
!CGF.CGM.getCodeGenOpts().OptimizationLevel ||
3586+
(CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
35883587
if (NoMerge)
35893588
HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
35903589
if (!MayReturn) {
@@ -3608,6 +3607,7 @@ void CodeGenFunction::EmitCheck(
36083607
llvm::Value *FatalCond = nullptr;
36093608
llvm::Value *RecoverableCond = nullptr;
36103609
llvm::Value *TrapCond = nullptr;
3610+
bool NoMerge = false;
36113611
for (int i = 0, n = Checked.size(); i < n; ++i) {
36123612
llvm::Value *Check = Checked[i].first;
36133613
// -fsanitize-trap= overrides -fsanitize-recover=.
@@ -3618,6 +3618,9 @@ void CodeGenFunction::EmitCheck(
36183618
? RecoverableCond
36193619
: FatalCond;
36203620
Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
3621+
3622+
if (!CGM.getCodeGenOpts().SanitizeMergeHandlers.has(Checked[i].second))
3623+
NoMerge = true;
36213624
}
36223625

36233626
if (ClSanitizeGuardChecks) {
@@ -3632,7 +3635,7 @@ void CodeGenFunction::EmitCheck(
36323635
}
36333636

36343637
if (TrapCond)
3635-
EmitTrapCheck(TrapCond, CheckHandler);
3638+
EmitTrapCheck(TrapCond, CheckHandler, NoMerge);
36363639
if (!FatalCond && !RecoverableCond)
36373640
return;
36383641

@@ -3698,7 +3701,7 @@ void CodeGenFunction::EmitCheck(
36983701
// Simple case: we need to generate a single handler call, either
36993702
// fatal, or non-fatal.
37003703
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
3701-
(FatalCond != nullptr), Cont);
3704+
(FatalCond != nullptr), Cont, NoMerge);
37023705
} else {
37033706
// Emit two handler calls: first one for set of unrecoverable checks,
37043707
// another one for recoverable.
@@ -3708,10 +3711,10 @@ void CodeGenFunction::EmitCheck(
37083711
Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
37093712
EmitBlock(FatalHandlerBB);
37103713
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
3711-
NonFatalHandlerBB);
3714+
NonFatalHandlerBB, NoMerge);
37123715
EmitBlock(NonFatalHandlerBB);
37133716
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
3714-
Cont);
3717+
Cont, NoMerge);
37153718
}
37163719

37173720
EmitBlock(Cont);
@@ -3901,7 +3904,8 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
39013904
}
39023905

39033906
void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
3904-
SanitizerHandler CheckHandlerID) {
3907+
SanitizerHandler CheckHandlerID,
3908+
bool NoMerge) {
39053909
llvm::BasicBlock *Cont = createBasicBlock("cont");
39063910

39073911
// If we're optimizing, collapse all calls to trap down to just one per
@@ -3911,9 +3915,9 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
39113915

39123916
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
39133917

3914-
bool NoMerge = ClSanitizeDebugDeoptimization ||
3915-
!CGM.getCodeGenOpts().OptimizationLevel ||
3916-
(CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
3918+
NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
3919+
!CGM.getCodeGenOpts().OptimizationLevel ||
3920+
(CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
39173921

39183922
if (TrapBB && !NoMerge) {
39193923
auto Call = TrapBB->begin();

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5171,7 +5171,8 @@ class CodeGenFunction : public CodeGenTypeCache {
51715171

51725172
/// Create a basic block that will call the trap intrinsic, and emit a
51735173
/// conditional branch to it, for the -ftrapv checks.
5174-
void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID);
5174+
void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID,
5175+
bool NoMerge = false);
51755176

51765177
/// Emit a call to trap or debugtrap and attach function attribute
51775178
/// "trap-func-name" if specified.

clang/lib/Driver/SanitizerArgs.cpp

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ static const SanitizerMask TrappingSupported =
6868
SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
6969
SanitizerKind::LocalBounds | SanitizerKind::CFI |
7070
SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
71+
static const SanitizerMask MergeDefault = SanitizerKind::Undefined;
7172
static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
7273
static const SanitizerMask CFIClasses =
7374
SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
@@ -696,6 +697,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
696697
TrappingKinds &= Kinds;
697698
RecoverableKinds &= ~TrappingKinds;
698699

700+
// Parse -f(no-)?sanitize-nonmerged-handlers flags
701+
SanitizerMask MergeKinds =
702+
parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {},
703+
options::OPT_fsanitize_merge_handlers_EQ,
704+
options::OPT_fno_sanitize_merge_handlers_EQ);
705+
MergeKinds &= Kinds;
706+
699707
// Setup ignorelist files.
700708
// Add default ignorelist from resource directory for activated sanitizers,
701709
// and validate special case lists format.
@@ -1113,6 +1121,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
11131121
TrapSanitizers.Mask |= TrappingKinds;
11141122
assert(!(RecoverableKinds & TrappingKinds) &&
11151123
"Overlap between recoverable and trapping sanitizers");
1124+
1125+
MergeHandlers.Mask |= MergeKinds;
11161126
}
11171127

11181128
static std::string toString(const clang::SanitizerSet &Sanitizers) {
@@ -1274,6 +1284,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
12741284
CmdArgs.push_back(
12751285
Args.MakeArgString("-fsanitize-trap=" + toString(TrapSanitizers)));
12761286

1287+
if (!MergeHandlers.empty())
1288+
CmdArgs.push_back(
1289+
Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers)));
1290+
12771291
addSpecialCaseListOpt(Args, CmdArgs,
12781292
"-fsanitize-ignorelist=", UserIgnorelistFiles);
12791293
addSpecialCaseListOpt(Args, CmdArgs,
@@ -1441,13 +1455,16 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
14411455

14421456
SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
14431457
bool DiagnoseErrors) {
1444-
assert((A->getOption().matches(options::OPT_fsanitize_EQ) ||
1445-
A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
1446-
A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
1447-
A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
1448-
A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
1449-
A->getOption().matches(options::OPT_fno_sanitize_trap_EQ)) &&
1450-
"Invalid argument in parseArgValues!");
1458+
assert(
1459+
(A->getOption().matches(options::OPT_fsanitize_EQ) ||
1460+
A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
1461+
A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
1462+
A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
1463+
A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
1464+
A->getOption().matches(options::OPT_fno_sanitize_trap_EQ) ||
1465+
A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) ||
1466+
A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) &&
1467+
"Invalid argument in parseArgValues!");
14511468
SanitizerMask Kinds;
14521469
for (int i = 0, n = A->getNumValues(); i != n; ++i) {
14531470
const char *Value = A->getValue(i);

clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,6 +1792,10 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
17921792
for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeTrap))
17931793
GenerateArg(Consumer, OPT_fsanitize_trap_EQ, Sanitizer);
17941794

1795+
for (StringRef Sanitizer :
1796+
serializeSanitizerKinds(Opts.SanitizeMergeHandlers))
1797+
GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer);
1798+
17951799
if (!Opts.EmitVersionIdentMetadata)
17961800
GenerateArg(Consumer, OPT_Qn);
17971801

@@ -2269,6 +2273,9 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
22692273
parseSanitizerKinds("-fsanitize-trap=",
22702274
Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
22712275
Opts.SanitizeTrap);
2276+
parseSanitizerKinds("-fsanitize-merge=",
2277+
Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ),
2278+
Diags, Opts.SanitizeMergeHandlers);
22722279

22732280
Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
22742281

clang/test/CodeGen/ubsan-trap-debugloc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow %s -o - -debug-info-kind=line-tables-only | FileCheck %s
1+
// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -fsanitize-merge=signed-integer-overflow %s -o - -debug-info-kind=line-tables-only | FileCheck %s
22

33

44
void foo(volatile int a) {

0 commit comments

Comments
 (0)