rnk created this revision.
rnk added reviewers: pcc, phosek, rsmith, zequanwu.
Herald added subscribers: jansvoboda11, dexonsmith, dang.
rnk requested review of this revision.
Herald added a project: clang.
Without this flag, enabling optimizations causes clang to emit a single
ubsantrap for every check failure of a particular kind. Adding this flag
allows the user to control this behavior separately, so they can choose
to have increased code size in exchange for more debuggable code.
A Chrome developer requested this feature here:
https://crbug.com/1185451
I made this change in such a way that it doesn't litter the cc1 line
with redundant flags: if the user does not pass the positive or negative
version if this flag, it is not forwarded to the cc1 invocation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D100150
Files:
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/catch-undef-behavior.c
clang/test/CodeGen/cfi-nomerge.c
clang/test/CodeGen/trapv-nomerge.c
clang/test/Driver/sanitize-merge-traps.c
Index: clang/test/Driver/sanitize-merge-traps.c
===================================================================
--- /dev/null
+++ clang/test/Driver/sanitize-merge-traps.c
@@ -0,0 +1,8 @@
+// RUN: %clang -c %s -fsanitize=cfi -flto -O2 -fno-sanitize-merge-traps -### 2>&1 | FileCheck %s --check-prefix=NOMERGE
+// RUN: %clang -c %s -fsanitize=cfi -flto -O0 -fsanitize-merge-traps -### 2>&1 | FileCheck %s --check-prefix=MERGE
+// RUN: %clang -c %s -fsanitize=cfi -flto -O2 -### 2>&1 | FileCheck %s --check-prefix=NONE
+// RUN: %clang -c %s -fsanitize=cfi -flto -O0 -### 2>&1 | FileCheck %s --check-prefix=NONE
+
+// NOMERGE: "-fno-sanitize-merge-traps"
+// MERGE: "-fsanitize-merge-traps"
+// NONE-NOT: "-f{{(no-)?}}sanitize-merge-traps"
Index: clang/test/CodeGen/trapv-nomerge.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/trapv-nomerge.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -O2 -triple x86_64-apple-darwin10 -ftrapv -fno-sanitize-merge-traps %s -emit-llvm -o - | FileCheck %s --check-prefix=NOMERGE
+// RUN: %clang_cc1 -O2 -triple x86_64-apple-darwin10 -ftrapv -fsanitize-merge-traps %s -emit-llvm -o - | FileCheck %s --check-prefix=MERGE
+
+unsigned int ui, uj, uk;
+int i, j, k;
+
+int twoChecks() {
+ ++i;
+ ++j;
+ return 42;
+}
+
+// NOMERGE-LABEL: define i32 @twoChecks()
+// NOMERGE: call void @llvm.ubsantrap(i8 0) #[[ATTR:[0-9]+]]
+// NOMERGE-NEXT: unreachable
+// NOMERGE: call void @llvm.ubsantrap(i8 0) #[[ATTR]]
+// NOMERGE-NEXT: unreachable
+
+// NOMERGE: #[[ATTR]] = { nomerge noreturn nounwind }
+
+// MERGE-LABEL: define i32 @twoChecks()
+// MERGE: call void @llvm.ubsantrap(i8 0)
+// MERGE-NEXT: unreachable
+// MERGE-NOT: llvm.ubsantrap
+// MERGE: ret i32 42
Index: clang/test/CodeGen/cfi-nomerge.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/cfi-nomerge.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -emit-llvm -o - %s -fsanitize-merge-traps | FileCheck --check-prefix=MERGE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -emit-llvm -o - %s -fno-sanitize-merge-traps | FileCheck --check-prefix=NOMERGE %s
+
+int twoChecks(void (*f)(), void (*g)()) {
+ f();
+ g();
+ return 42;
+}
+
+// NOMERGE-LABEL: define dso_local i32 @twoChecks(
+// NOMERGE: call void @llvm.ubsantrap(i8 2) #[[ATTR:[0-9]+]]
+// NOMERGE-NEXT: unreachable
+// NOMERGE: call void @llvm.ubsantrap(i8 2) #[[ATTR]]
+// NOMERGE-NEXT: unreachable
+
+// NOMERGE: #[[ATTR]] = { nomerge noreturn nounwind }
+
+// MERGE-LABEL: define dso_local i32 @twoChecks(
+// MERGE: call void @llvm.ubsantrap(i8 2)
+// MERGE-NEXT: unreachable
+// MERGE-NOT: llvm.ubsantrap
+// MERGE: ret i32 42
Index: clang/test/CodeGen/catch-undef-behavior.c
===================================================================
--- clang/test/CodeGen/catch-undef-behavior.c
+++ clang/test/CodeGen/catch-undef-behavior.c
@@ -387,4 +387,4 @@
// CHECK-UBSAN: ![[WEIGHT_MD]] = !{!"branch_weights", i32 1048575, i32 1}
-// CHECK-TRAP: attributes [[NR_NUW]] = { noreturn nounwind }
+// CHECK-TRAP: attributes [[NR_NUW]] = { nomerge noreturn nounwind }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1935,6 +1935,13 @@
Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
Opts.SanitizeTrap);
+ // Merging sanitizer traps saves code size, but makes debugging harder. By
+ // default, merge them when optimizations are enabled. Let the user override
+ // the setting either way.
+ Opts.SanitizeMergeTraps = Args.hasFlag(options::OPT_fsanitize_merge_traps,
+ options::OPT_fno_sanitize_merge_traps,
+ OptimizationLevel > 0);
+
Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
if (Args.hasArg(options::OPT_ffinite_loops))
Index: clang/lib/Driver/SanitizerArgs.cpp
===================================================================
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -655,6 +655,14 @@
Stats = Args.hasFlag(options::OPT_fsanitize_stats,
options::OPT_fno_sanitize_stats, false);
+ // If the user didn't pass an explicit sanitize-merge-traps value, don't add
+ // it to the cc1 line.
+ if (Args.hasArg(options::OPT_fsanitize_merge_traps,
+ options::OPT_fno_sanitize_merge_traps)) {
+ MergeTraps = Args.hasFlag(options::OPT_fsanitize_merge_traps,
+ options::OPT_fno_sanitize_merge_traps);
+ }
+
if (MinimalRuntime) {
SanitizerMask IncompatibleMask =
Kinds & ~setGroupBits(CompatibleWithMinimalRuntime);
@@ -1066,6 +1074,13 @@
if (Stats)
CmdArgs.push_back("-fsanitize-stats");
+ if (MergeTraps) {
+ if (*MergeTraps)
+ CmdArgs.push_back("-fsanitize-merge-traps");
+ else
+ CmdArgs.push_back("-fno-sanitize-merge-traps");
+ }
+
if (MinimalRuntime)
CmdArgs.push_back("-fsanitize-minimal-runtime");
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3474,7 +3474,7 @@
TrapBBs.resize(CheckHandlerID + 1);
llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID];
- if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+ if (!CGM.getCodeGenOpts().SanitizeMergeTraps || !TrapBB) {
TrapBB = createBasicBlock("trap");
Builder.CreateCondBr(Checked, Cont, TrapBB);
EmitBlock(TrapBB);
@@ -3490,6 +3490,12 @@
}
TrapCall->setDoesNotReturn();
TrapCall->setDoesNotThrow();
+ if (!CGM.getCodeGenOpts().SanitizeMergeTraps) {
+ // Prevent the optimizer from merging traps if the user asked us not
+ // to.
+ TrapCall->addAttribute(llvm::AttributeList::FunctionIndex,
+ llvm::Attribute::NoMerge);
+ }
Builder.CreateUnreachable();
} else {
auto Call = TrapBB->begin();
Index: clang/include/clang/Driver/SanitizerArgs.h
===================================================================
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -57,6 +57,7 @@
// True if cross-dso CFI support if provided by the system (i.e. Android).
bool ImplicitCfiRuntime = false;
bool NeedsMemProfRt = false;
+ Optional<bool> MergeTraps;
public:
/// Parses the sanitizer arguments from an argument list.
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1608,6 +1608,13 @@
PosFlag<SetTrue, [], "Enable">, NegFlag<SetFalse, [CoreOption, NoXarchOption], "Disable">,
BothFlags<[], " sanitizer statistics gathering.">>,
Group<f_clang_Group>;
+def fsanitize_merge_traps : Flag<["-"], "fsanitize-merge-traps">,
+ Group<f_clang_Group>,
+ HelpText<"Merge sanitizer check trap instructions">;
+def fno_sanitize_merge_traps : Flag<["-"], "fno-sanitize-merge-traps">,
+ Group<f_clang_Group>,
+ Flags<[CoreOption, NoXarchOption]>,
+ HelpText<"Do not merge sanitizer check trap instructions">;
def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
Group<f_clang_Group>,
HelpText<"Enable memory access instrumentation in ThreadSanitizer (default)">;
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -254,6 +254,7 @@
CODEGENOPT(SanitizeCoveragePCTable, 1, 0) ///< Create a PC Table.
CODEGENOPT(SanitizeCoverageNoPrune, 1, 0) ///< Disable coverage pruning.
CODEGENOPT(SanitizeCoverageStackDepth, 1, 0) ///< Enable max stack depth tracing
+CODEGENOPT(SanitizeMergeTraps, 1, 0) ///< Whether to share trap instructions.
CODEGENOPT(SanitizeStats , 1, 0) ///< Collect statistics for sanitizers.
CODEGENOPT(SimplifyLibCalls , 1, 1) ///< Set when -fbuiltin is enabled.
CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
