[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

[Chris Bieneman via cfe-commits]

beanz added a comment.

Mostly formatting comments. I think for the most part this is the right way to go, but I don't really feel qualified to approve this. The one thing I'm not sure about on this is, do we really want this to be darwin-only? I kinda think it might be nice if we unified this behavior across all platforms.


================
Comment at: lib/Driver/SanitizerArgs.cpp:204
@@ -203,2 +203,3 @@
   const SanitizerMask Supported = setGroupBits(TC.getSupportedSanitizers());
+  const SanitizerMask RequiresTrap = setGroupBits(TC.getSanitizersRequiringTrap());
   ToolChain::RTTIMode RTTIMode = TC.getRTTIMode();
----------------
80 col

================
Comment at: lib/Driver/SanitizerArgs.cpp:240
@@ +239,3 @@
+      if ((Add & TrappingKinds) == 0) {
+        if (SanitizerMask KindsToDiagnose = Add & RequiresTrap & ~DiagnosedKinds) {
+          std::string Desc = describeSanitizeArg(*I, KindsToDiagnose);
----------------
Also 80 col violation.

Please clang-format.

================
Comment at: lib/Driver/ToolChains.cpp:1243
@@ -1214,2 +1242,3 @@
+
 SanitizerMask Darwin::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
----------------
I'm not super familiar with the driver code, but I wonder if this can't be better abstracted.

I'd really like to see all operating systems have the same behavior for this, so I think much of the code can be shared.

If the runtime library exists for a sanitizer (any sanitizer) I don't see any reason clang shouldn't then support the flags.

================
Comment at: lib/Driver/ToolChains.cpp:1258
@@ -1223,1 +1257,3 @@
+  if (!SanitizerRuntimeExists("ubsan")) {
+    Res |= (SanitizerKind::Undefined & ~SanitizerKind::Vptr & ~SanitizerKind::Function);
   }
----------------
Last time I'll comment this, please clang-format.


http://reviews.llvm.org/D15225