[PATCH] D38019: [asan] Fix nested error detection

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_report.cc:126
   explicit ScopedInErrorReport(bool fatal = false) {
     halt_on_error_ = fatal || flags()->halt_on_error;
     u32 current_tid = GetCurrentTidOrInvalid();
----------------
This looks like a data race.
While one thread executes ScopedInErrorReport, another thread can do something like:

halt_on_error_ = fatal;
if (flags()->halt_on_error) halt_on_error_ = true;

And then the first thread can fail to halt and the program will hang for 100 seconds.

Or, there can be 2 thread: one with fatal=true and another with fatal=false, and asan will halt after a non-fatal error which can cause user confusion.


================
Comment at: compiler-rt/lib/asan/asan_report.cc:129
 
-    if (halt_on_error_) {
-      // Do not print more than one report, otherwise they will mix up.
-      // Error reporting functions shouldn't return at this situation, as
-      // they are effectively no-returns.
+    for (int i = 0; i < 100; ++i) {
+      u32 expected_tid = kInvalidTid;
----------------
This looks somewhat fishy. Is there any known problem with making this an infinite loop as it is now?
Pretty sure SleepForMillis can undersleep if SIGPOF profiling is enabled, and then we can terminate the process while another thread still reports an error.


https://reviews.llvm.org/D38019