Skip to content

Commit

Permalink
[ubsan] Add -fsanitize-merge (and -fno-sanitize-merge) (llvm#120464)
Browse files Browse the repository at this point in the history
'-mllvm -ubsan-unique-traps'
(llvm#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
llvm#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.
  • Loading branch information
thurstond authored Dec 18, 2024
1 parent 0fd7c49 commit 7eaf470
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 22 deletions.
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/CodeGenOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// Set of sanitizer checks that trap rather than diagnose.
SanitizerSet SanitizeTrap;

/// Set of sanitizer checks that can merge handlers (smaller code size at
/// the expense of debuggability).
SanitizerSet SanitizeMergeHandlers;

/// List of backend command-line options for -fembed-bitcode.
std::vector<uint8_t> CmdArgs;

Expand Down
15 changes: 15 additions & 0 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -2555,6 +2555,21 @@ def fno_sanitize_trap : Flag<["-"], "fno-sanitize-trap">, Group<f_clang_Group>,
Alias<fno_sanitize_trap_EQ>, AliasArgs<["all"]>,
Visibility<[ClangOption, CLOption]>,
HelpText<"Disable trapping for all sanitizers">;
def fsanitize_merge_handlers_EQ
: CommaJoined<["-"], "fsanitize-merge=">,
Group<f_clang_Group>,
HelpText<"Allow compiler to merge handlers for specified sanitizers">;
def fno_sanitize_merge_handlers_EQ
: CommaJoined<["-"], "fno-sanitize-merge=">,
Group<f_clang_Group>,
HelpText<"Do not allow compiler to merge handlers for specified sanitizers">;
def fsanitize_merge_handlers : Flag<["-"], "fsanitize-merge">, Group<f_clang_Group>,
Alias<fsanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
HelpText<"Allow compiler to merge handlers for all sanitizers">;
def fno_sanitize_merge_handlers : Flag<["-"], "fno-sanitize-merge">, Group<f_clang_Group>,
Alias<fno_sanitize_merge_handlers_EQ>, AliasArgs<["all"]>,
Visibility<[ClangOption, CLOption]>,
HelpText<"Do not allow compiler to merge handlers for any sanitizers">;
def fsanitize_undefined_trap_on_error
: Flag<["-"], "fsanitize-undefined-trap-on-error">, Group<f_clang_Group>,
Alias<fsanitize_trap_EQ>, AliasArgs<["undefined"]>;
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Driver/SanitizerArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class SanitizerArgs {
SanitizerSet Sanitizers;
SanitizerSet RecoverableSanitizers;
SanitizerSet TrapSanitizers;
SanitizerSet MergeHandlers;

std::vector<std::string> UserIgnorelistFiles;
std::vector<std::string> SystemIgnorelistFiles;
Expand Down
30 changes: 17 additions & 13 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3546,7 +3546,7 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
ArrayRef<llvm::Value *> FnArgs,
SanitizerHandler CheckHandler,
CheckRecoverableKind RecoverKind, bool IsFatal,
llvm::BasicBlock *ContBB) {
llvm::BasicBlock *ContBB, bool NoMerge) {
assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
std::optional<ApplyDebugLocation> DL;
if (!CGF.Builder.getCurrentDebugLocation()) {
Expand Down Expand Up @@ -3581,10 +3581,9 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
llvm::AttributeList::FunctionIndex, B),
/*Local=*/true);
llvm::CallInst *HandlerCall = CGF.EmitNounwindRuntimeCall(Fn, FnArgs);
bool NoMerge =
ClSanitizeDebugDeoptimization ||
!CGF.CGM.getCodeGenOpts().OptimizationLevel ||
(CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
!CGF.CGM.getCodeGenOpts().OptimizationLevel ||
(CGF.CurCodeDecl && CGF.CurCodeDecl->hasAttr<OptimizeNoneAttr>());
if (NoMerge)
HandlerCall->addFnAttr(llvm::Attribute::NoMerge);
if (!MayReturn) {
Expand All @@ -3608,6 +3607,7 @@ void CodeGenFunction::EmitCheck(
llvm::Value *FatalCond = nullptr;
llvm::Value *RecoverableCond = nullptr;
llvm::Value *TrapCond = nullptr;
bool NoMerge = false;
for (int i = 0, n = Checked.size(); i < n; ++i) {
llvm::Value *Check = Checked[i].first;
// -fsanitize-trap= overrides -fsanitize-recover=.
Expand All @@ -3618,6 +3618,9 @@ void CodeGenFunction::EmitCheck(
? RecoverableCond
: FatalCond;
Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;

if (!CGM.getCodeGenOpts().SanitizeMergeHandlers.has(Checked[i].second))
NoMerge = true;
}

if (ClSanitizeGuardChecks) {
Expand All @@ -3632,7 +3635,7 @@ void CodeGenFunction::EmitCheck(
}

if (TrapCond)
EmitTrapCheck(TrapCond, CheckHandler);
EmitTrapCheck(TrapCond, CheckHandler, NoMerge);
if (!FatalCond && !RecoverableCond)
return;

Expand Down Expand Up @@ -3698,7 +3701,7 @@ 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);
(FatalCond != nullptr), Cont, NoMerge);
} else {
// Emit two handler calls: first one for set of unrecoverable checks,
// another one for recoverable.
Expand All @@ -3708,10 +3711,10 @@ void CodeGenFunction::EmitCheck(
Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
EmitBlock(FatalHandlerBB);
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
NonFatalHandlerBB);
NonFatalHandlerBB, NoMerge);
EmitBlock(NonFatalHandlerBB);
emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
Cont);
Cont, NoMerge);
}

EmitBlock(Cont);
Expand Down Expand Up @@ -3901,7 +3904,8 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
}

void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
SanitizerHandler CheckHandlerID) {
SanitizerHandler CheckHandlerID,
bool NoMerge) {
llvm::BasicBlock *Cont = createBasicBlock("cont");

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

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

bool NoMerge = ClSanitizeDebugDeoptimization ||
!CGM.getCodeGenOpts().OptimizationLevel ||
(CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());
NoMerge = NoMerge || ClSanitizeDebugDeoptimization ||
!CGM.getCodeGenOpts().OptimizationLevel ||
(CurCodeDecl && CurCodeDecl->hasAttr<OptimizeNoneAttr>());

if (TrapBB && !NoMerge) {
auto Call = TrapBB->begin();
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/CodeGen/CodeGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -5171,7 +5171,8 @@ class CodeGenFunction : public CodeGenTypeCache {

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

/// Emit a call to trap or debugtrap and attach function attribute
/// "trap-func-name" if specified.
Expand Down
31 changes: 24 additions & 7 deletions clang/lib/Driver/SanitizerArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ static const SanitizerMask TrappingSupported =
SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
SanitizerKind::LocalBounds | SanitizerKind::CFI |
SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
static const SanitizerMask MergeDefault = SanitizerKind::Undefined;
static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
static const SanitizerMask CFIClasses =
SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
Expand Down Expand Up @@ -696,6 +697,13 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
TrappingKinds &= Kinds;
RecoverableKinds &= ~TrappingKinds;

// Parse -f(no-)?sanitize-nonmerged-handlers flags
SanitizerMask MergeKinds =
parseSanitizeArgs(D, Args, DiagnoseErrors, MergeDefault, {}, {},
options::OPT_fsanitize_merge_handlers_EQ,
options::OPT_fno_sanitize_merge_handlers_EQ);
MergeKinds &= Kinds;

// Setup ignorelist files.
// Add default ignorelist from resource directory for activated sanitizers,
// and validate special case lists format.
Expand Down Expand Up @@ -1114,6 +1122,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
TrapSanitizers.Mask |= TrappingKinds;
assert(!(RecoverableKinds & TrappingKinds) &&
"Overlap between recoverable and trapping sanitizers");

MergeHandlers.Mask |= MergeKinds;
}

static std::string toString(const clang::SanitizerSet &Sanitizers) {
Expand Down Expand Up @@ -1275,6 +1285,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
CmdArgs.push_back(
Args.MakeArgString("-fsanitize-trap=" + toString(TrapSanitizers)));

if (!MergeHandlers.empty())
CmdArgs.push_back(
Args.MakeArgString("-fsanitize-merge=" + toString(MergeHandlers)));

addSpecialCaseListOpt(Args, CmdArgs,
"-fsanitize-ignorelist=", UserIgnorelistFiles);
addSpecialCaseListOpt(Args, CmdArgs,
Expand Down Expand Up @@ -1442,13 +1456,16 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,

SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
bool DiagnoseErrors) {
assert((A->getOption().matches(options::OPT_fsanitize_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_trap_EQ)) &&
"Invalid argument in parseArgValues!");
assert(
(A->getOption().matches(options::OPT_fsanitize_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_trap_EQ) ||
A->getOption().matches(options::OPT_fsanitize_merge_handlers_EQ) ||
A->getOption().matches(options::OPT_fno_sanitize_merge_handlers_EQ)) &&
"Invalid argument in parseArgValues!");
SanitizerMask Kinds;
for (int i = 0, n = A->getNumValues(); i != n; ++i) {
const char *Value = A->getValue(i);
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1792,6 +1792,10 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeTrap))
GenerateArg(Consumer, OPT_fsanitize_trap_EQ, Sanitizer);

for (StringRef Sanitizer :
serializeSanitizerKinds(Opts.SanitizeMergeHandlers))
GenerateArg(Consumer, OPT_fsanitize_merge_handlers_EQ, Sanitizer);

if (!Opts.EmitVersionIdentMetadata)
GenerateArg(Consumer, OPT_Qn);

Expand Down Expand Up @@ -2269,6 +2273,9 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
parseSanitizerKinds("-fsanitize-trap=",
Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
Opts.SanitizeTrap);
parseSanitizerKinds("-fsanitize-merge=",
Args.getAllArgValues(OPT_fsanitize_merge_handlers_EQ),
Diags, Opts.SanitizeMergeHandlers);

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

Expand Down
7 changes: 6 additions & 1 deletion clang/test/CodeGen/ubsan-trap-merge.c
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
// NOTE: Assertions have mostly been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
// The most important assertion is the attributes at the end of the file, which
// shows whether -ubsan-unique-traps attaches 'nomerge' to each ubsan call.
// shows whether -ubsan-unique-traps and -fno-sanitize-merge attach 'nomerge'
// to each ubsan call.
//
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefix=TRAP
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - | FileCheck %s --check-prefix=HANDLER
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -mllvm -ubsan-unique-traps %s -o - -fsanitize-minimal-runtime | FileCheck %s --check-prefix=MINRT
//
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow %s -o - -fsanitize-trap=signed-integer-overflow | FileCheck %s --check-prefix=TRAP
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow %s -o - | FileCheck %s --check-prefix=HANDLER
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -fsanitize=signed-integer-overflow -O3 -fno-sanitize-merge=signed-integer-overflow %s -o - -fsanitize-minimal-runtime | FileCheck %s --check-prefix=MINRT
//
// REQUIRES: x86-registered-target

// TRAP-LABEL: define dso_local range(i32 -2147483523, -2147483648) i32 @f(
Expand Down
Loading

0 comments on commit 7eaf470

Please sign in to comment.