[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