[PATCH] D100150: [Sanitizers] Add a flag -f[no-]sanitize-merge-traps

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

The `CodeGen` tests you added are failing pre-merge checks. This is most likely 
because we recently (D97462 ) started 
verifying that all all CC1 command line options can be serialized from a 
`CompilerInvocation` instance. To reproduce this locally, you'd need to build 
with assertions or manually set `-DCLANG_ROUND_TRIP_CC1_ARGS=ON` when building 
with CMake.

The solution would be to add complementary code that generates 
`"-f[no-]sanitize-merge-traps"` from `CodeGenOptions::SanitizeMergeTraps` in 
`CompilerInvocation::GenerateCodeGenArgs`.

More info is here 
https://clang.llvm.org/docs/InternalsManual.html#compiler-invocation and in the 
following sections.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100150/new/

https://reviews.llvm.org/D100150

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100150: [Sanitizers] Add a flag -f[no-]sanitize-merge-traps

[Reid Kleckner via Phabricator via cfe-commits]

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