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

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 4 13:47:10 PDT 2015


kcc added inline comments.

================
Comment at: lib/asan/asan_report.cc:626
@@ +625,3 @@
+                               bool fatal = false) {
+    halt_on_error = fatal || flags()->halt_on_error;
+
----------------
ygribov wrote:
> kcc wrote:
> > This is all exceptionally horrible.
> > I've spent quite a bit of time and failed to understand how it works, 
> > also I don't understand how to test it. 
> > Why not just grab a lock for the entire duration of error reporting? 
> > 
> > The current logic was there because we never reported more than one bug, 
> > now that we do -- simply grab a lock! 
> > also I don't understand how to test it. 
> 
> Did you ever understand how to test original code :) ? The behavior for fatal errors didn't change by the way.
> 
> > Why not just grab a lock for the entire duration of error reporting?
> 
> Because we may run into second error while reporting the first one in the same thread. Grabbing a lock would cause a deadlock in this case.
>> Did you ever understand how to test original code :) ? 
That's not an argument. We don't want to make things worse just because they are not good at the moment. 

>> Because we may run into second error while reporting the first one in the same thread. Grabbing a lock would cause a deadlock in this case.
Yea. Async signal safety... 


Repository:
  rL LLVM

http://reviews.llvm.org/D12318





More information about the llvm-commits mailing list