[PATCH] D12318: [ASan] Enable optional ASan recovery

[Yury Gribov via llvm-commits]

ygribov added inline comments.

================
Comment at: include/clang/Frontend/CodeGenOptions.def:118
@@ -117,1 +117,3 @@
                                                  ///< offset in AddressSanitizer.
+CODEGENOPT(SanitizeAddressNoAbort, 1, 0) ///< Enable continue-after-error mode
+                                         ///< in AddressSanitizer.
----------------
eugenis wrote:
> Could you make it not specific to ASan? Smth like SanitizeRecover so that it could affect MSan as well in the future.
Right.

================
Comment at: lib/CodeGen/BackendUtil.cpp:343
@@ +342,3 @@
+      PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
+                             addAddressSanitizerNoAbortPasses);
+      PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
----------------
eugenis wrote:
> Why do you need separate "noabort" functions for this? See addMemorySanitizerPass which inspects CodeGenOptions in the function body.
Ok.

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:125
@@ -124,1 +124,3 @@
     cl::Hidden, cl::init(false));
+static cl::opt<bool> ClEnableNoAbort(
+    "asan-noabort",
----------------
eugenis wrote:
> Lets stick to existing terminology (we already have "recover" and "keep-going" names for the same concept).
Ok, I'll use "recover" then. Most of this was done in pre-recover times.

================
Comment at: lib/asan/asan_flags.inc:147
@@ +146,3 @@
+          "(warning: use at your own risk!)")
+ASAN_FLAG(int, num_reported_errors, 10,
+          "Maximum number of errors reported if keep_going mode is enabled")
----------------
eugenis wrote:
> call it max_errors, similarly to the compiler flag?
Sure.

================
Comment at: lib/asan/asan_poisoning.cc:221
@@ -220,3 +220,3 @@
       uptr __bad = __asan_region_is_poisoned(__p, __size);    \
-      __asan_report_error(pc, bp, sp, __bad, isWrite, __size, 0);\
+      __asan_report_error(pc, bp, sp, __bad, isWrite, __size, 0, false); \
     }                                                         \
----------------
eugenis wrote:
> why "false"? Do you need to check the keep_going flag?
Hm, probably yes.

================
Comment at: lib/asan/asan_report.cc:652
@@ +651,3 @@
+            if (TryLockReporting())
+              goto locked;
+          }
----------------
kcc wrote:
> OMG
This section could be removed if we are ok with ignoring bugs that run in parallel.


Repository:
  rL LLVM

http://reviews.llvm.org/D12318