[cfe-commits] [PATCH] Add three new sanitizers depending on ASan.

[Richard Smith]

  The general approach of exposing more ASan features with -fsanitize= flags seems fine to me, as does making them require -fsanitize=address. The new sanitizer names seem reasonable.

  I'm not sure whether address-full should include init-order, since the initialization order sanitizer isn't checking for an invalid address problem (and it even traps on some obscure cases with defined behavior).


================
Comment at: lib/Driver/SanitizerArgs.h:59-71
@@ -58,1 +58,15 @@
+  
+    // Add args for LLVM backend.
+    if (Kind & InitOrder) {
+      CmdArgs.push_back("-mllvm");
+      CmdArgs.push_back("-asan-initialization-order");
+    }
+    if (Kind & UseAfterReturn) {
+      CmdArgs.push_back("-mllvm");
+      CmdArgs.push_back("-asan-use-after-return");
+    }
+    if (Kind & UseAfterScope) {
+      CmdArgs.push_back("-mllvm");
+      CmdArgs.push_back("-asan-use-lifetime");
+    }
   }
----------------
I would prefer this to be handled by the frontend instead of by the driver (the frontend is responsible for adding all the other IR instrumentation, including adding the ASan passes).

Have you considered passing these flags to ASan when creating the passes in addAddressSanitizerPass, rather than as command-line options?

================
Comment at: lib/Driver/Tools.cpp:1518-1523
@@ -1517,1 +1517,8 @@
+
+  // If -fsanitize contains extra features of ASan, it should also
+  // explicitly contain -fsanitize=address. 
+  if (NeedsAsan && ((Kind & Address) == 0))
+    D.Diag(diag::err_drv_argument_only_allowed_with)
+      << describeSanitizeArg(Args, AsanArg, NeedsAsanRt)
+      << "-fsanitize=address";
 }
----------------
For argument lists like "-fsanitize=use-after-return -fsanitize=address -fno-sanitize=address", we'll say "-fsanitize=address is only allowed with -fsanitize=address". The existing diagnostic has a similar issue for "-fsanitize=address -fsanitize=alignment -fsanitize=vptr -fno-sanitize=vptr", where it says "-fsanitize=vptr not allowed with -fsanitize=address". I think we'd need to teach describeSanitizerArg to re-parse the argument list to handle this properly.


http://llvm-reviews.chandlerc.com/D142