[PATCH] Introduce -fsanitize-trap= flag.

Richard Smith richard at metafoo.co.uk
Mon Jun 15 17:57:51 PDT 2015


================
Comment at: docs/UsersManual.rst:1098
@@ +1097,3 @@
+
+   Controls which checks enabled by ``-fsanitize=`` flag trap. This option
+   is intended for use in cases where the sanitizer runtime cannot be used
----------------
Missing 'the' before `-fsanitize=`?

================
Comment at: docs/UsersManual.rst:1099-1101
@@ +1098,5 @@
+   Controls which checks enabled by ``-fsanitize=`` flag trap. This option
+   is intended for use in cases where the sanitizer runtime cannot be used
+   (for instance, when building libc or a kernel module), or where the binary
+   size increase caused by the sanitizer runtime is a concern.
+
----------------
I think this description will leave people wondering why we have a per-sanitizer trap flag rather than a global flag. It'd be good to extend this description with the CFI case.

================
Comment at: docs/UsersManual.rst:1104
@@ +1103,3 @@
+   This flag is only compatible with sanitizers in the
+   UndefinedBehaviorSanitizer group other than ``vptr``. If this flag is
+   supplied together with ``-fsanitize=undefined``, the ``vptr`` sanitizer
----------------
It's probably better to refer to the group as "``undefined``" rather than "UndefinedBehaviorSanitizer", for symmetry with "``vptr``".

================
Comment at: lib/CodeGen/CGExpr.cpp:2306-2315
@@ -2304,7 +2305,12 @@
     llvm::Value *Check = Checked[i].first;
+    if (CGM.getCodeGenOpts().SanitizeTrap.has(Checked[i].second)) {
+      // -fsanitize-trap= overrides -fsanitize-recover=.
+      TrapCond = TrapCond ? Builder.CreateAnd(TrapCond, Check) : Check;
+      continue;
+    }
     llvm::Value *&Cond =
         CGM.getCodeGenOpts().SanitizeRecover.has(Checked[i].second)
             ? RecoverableCond
             : FatalCond;
     Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
   }
----------------
The differing style in these two cases is strange. Please pick one or the other and apply it to both the `Recover` and the `Trap` cases.

================
Comment at: lib/Driver/SanitizerArgs.cpp:209
@@ +208,3 @@
+  SanitizerMask TrappingKinds = parseSanitizeTrapArgs(D, Args);
+  if (TrappingKinds & UndefinedGroup)
+    NotSupported |= Vptr;
----------------
I think this should be `TrappingKinds & Vptr`. If I write `-fsanitize-trap=undefined -fsanitize=vptr -fno-sanitize-trap=vptr`, I think you'll currently mark `vptr` as unavailable, but it seems reasonable to support that case.

http://reviews.llvm.org/D10464

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list