<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 31, 2016 at 1:49 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On May 31, 2016, at 1:43 PM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Tue, May 31, 2016 at 1:38 PM, Mehdi Amini<span> </span><span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On May 31, 2016, at 1:22 PM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:</div><br><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 31, 2016 at 1:16 PM, Mehdi Amini<span> </span><span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div>Committed in r271323.</div><div><br></div></div></blockquote><div>Thanks!  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div></div>I wonder what is the rational for the discrepancy between Darwin and Linux?</div></blockquote><div><br></div><div>Don't get me started :) </div></div></div></div></div></blockquote><div><br></div></span><div>Sorry :)</div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><a href="http://reviews.llvm.org/D7203" rel="noreferrer" style="font-size:12.8px" target="_blank">http://reviews.llvm.org/D7203</a><br></div><div><a href="https://groups.google.com/forum/#!topic/address-sanitizer/FGYgD_P_884" rel="noreferrer" style="font-size:12.8px" target="_blank">https://groups.google.com/forum/#!topic/address-sanitizer/FGYgD_P_884</a><span style="font-size:12.8px"> </span><br></div></div></div></div></div></blockquote><div><br></div><div>I see some rational to abort, I don't see some strong rational to *not* abort on every platform.</div></div></div></blockquote><div><br></div><div>Abort is slow</div></div></div></blockquote><div><br></div></span><div>An ASAN failure is not supposed to be on your critical path, right?</div></div></div></blockquote><div>No :) </div><div>One of the modes of operation is to run a binary for a very short period of time (5-10 seconds) </div><div>and kill it if it did not crash with asan report. </div><div>abort() may break this workflow as abort() will take several seconds to complete on a large binary</div><div><br></div><div>Another problem, is that we want to run asan's own tests in the default mode and we can't cheaply run them with abort.</div><div>This got broken on Mac since on Mac we now run the tests in non-default mode. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>(Disabling it for testing using a build setting/env-var seems appropriate if it is the only concern...)</div><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> and verbose</div></div></div></blockquote><div><br></div><div>Being verbose on crashes by default seems like a good thing to me?</div></div></div></blockquote><div><br></div><div>Not too verbose. </div><div>Asan provides all the details needed to investigate memory bugs, any extra info will distract. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div>, it often involves lots of extra stuff that you don't need when you see an asan report. </div></div></div></blockquote><div><br></div></span><div>Is core dump the concern? You mentioned in the thread that is is possible to tweak this when ASAN is enabled though.</div></div></div></blockquote><div><br></div><div>Core is disabled with asan by default, that's not a problem. </div><div><br></div><div><br></div><div>BTW: choosing the default is hard. </div><div><br></div><div>--kcc</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div class="h5"><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div><br></div><div>--kcc</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span><font color="#888888"><div><br></div><div><br></div><div>-- </div><div>Mehdi</div></font></span><div><div><div><br></div><div><br></div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>--kcc </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><span><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div></font></span><div><div><div><br><div><div><blockquote type="cite"><div>On May 31, 2016, at 1:11 PM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:</div><br><div><div dir="ltr">Ah, I think this is caused by different defaults. <div>On linux by default asan dies with _exit and on Mac it dies with abort()</div><div>This difference was introduced by Kuba after a long resistance from my side :)</div><div>The fix should be pretty simple: add abort_on_error=0 to %tool_options in the test. </div><div>Could you please check if this helps on Mac? </div><div>If so, feel free to submit. </div><div><br></div><div>Thanks! </div><div>--kcc</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 31, 2016 at 1:01 PM, Mehdi Amini<span> </span><span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Hi Kostya,<br><br>We see a bot failure with the test case you added below:<span> </span><a href="http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-globalisel_check/2068/console" rel="noreferrer" target="_blank">http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-globalisel_check/2068/console</a><br><br>After investigating, it seems that they trigger an ASAN failure, which is returning a negative error code. The `not` command tool interprets it as a crash, and you need to pass the --crash option to it.<br><br>Don't you observe this on your setup? I wonder why they are passing?<br><span><font color="#888888"><br>--<br>Mehdi<br></font></span><div><div><br><br><br><br>> On May 27, 2016, at 2:23 PM, Kostya Serebryany via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>><br>> Author: kcc<br>> Date: Fri May 27 16:23:05 2016<br>> New Revision: 271046<br>><br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=271046&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=271046&view=rev</a><br>> Log:<br>> [sanitizers] introduce __sanitizer_set_report_fd so that we can re-route the sanitizer logging to another fd from inside the process<br>><br>> Added:<br>>    compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_fd_test.cc<br>> Modified:<br>>    compiler-rt/trunk/include/sanitizer/common_interface_defs.h<br>>    compiler-rt/trunk/lib/asan/asan_posix.cc<br>>    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc<br>><br>> Modified: compiler-rt/trunk/include/sanitizer/common_interface_defs.h<br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/include/sanitizer/common_interface_defs.h?rev=271046&r1=271045&r2=271046&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/include/sanitizer/common_interface_defs.h?rev=271046&r1=271045&r2=271046&view=diff</a><br>> ==============================================================================<br>> --- compiler-rt/trunk/include/sanitizer/common_interface_defs.h (original)<br>> +++ compiler-rt/trunk/include/sanitizer/common_interface_defs.h Fri May 27 16:23:05 2016<br>> @@ -41,6 +41,9 @@ extern "C" {<br>><br>>   // Tell the tools to write their reports to "path.<pid>" instead of stderr.<br>>   void __sanitizer_set_report_path(const char *path);<br>> +  // Tell the tools to write their reports to the provided file descriptor<br>> +  // (casted to void *).<br>> +  void __sanitizer_set_report_fd(void *fd);<br>><br>>   // Notify the tools that the sandbox is going to be turned on. The reserved<br>>   // parameter will be used in the future to hold a structure with functions<br>><br>> Modified: compiler-rt/trunk/lib/asan/asan_posix.cc<br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_posix.cc?rev=271046&r1=271045&r2=271046&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_posix.cc?rev=271046&r1=271045&r2=271046&view=diff</a><br>> ==============================================================================<br>> --- compiler-rt/trunk/lib/asan/asan_posix.cc (original)<br>> +++ compiler-rt/trunk/lib/asan/asan_posix.cc Fri May 27 16:23:05 2016<br>> @@ -36,8 +36,9 @@ namespace __asan {<br>> void AsanOnDeadlySignal(int signo, void *siginfo, void *context) {<br>>   ScopedDeadlySignal signal_scope(GetCurrentThread());<br>>   int code = (int)((siginfo_t*)siginfo)->si_code;<br>> -  // Write the first message using the bullet-proof write.<br>> -  if (18 != internal_write(2, "ASAN:DEADLYSIGNAL\n", 18)) Die();<br>> +  // Write the first message using fd=2, just in case.<br>> +  // It may actually fail to write in case stderr is closed.<br>> +  internal_write(2, "ASAN:DEADLYSIGNAL\n", 18);<br>>   SignalContext sig = SignalContext::Create(siginfo, context);<br>><br>>   // Access at a reasonable offset above SP, or slightly below it (to account<br>><br>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc<br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc?rev=271046&r1=271045&r2=271046&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc?rev=271046&r1=271045&r2=271046&view=diff</a><br>> ==============================================================================<br>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc (original)<br>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc Fri May 27 16:23:05 2016<br>> @@ -496,6 +496,11 @@ void __sanitizer_set_report_path(const c<br>>   report_file.SetReportPath(path);<br>> }<br>><br>> +void __sanitizer_set_report_fd(void *fd) {<br>> +  report_file.fd = reinterpret_cast<uptr>(fd);<br>> +  report_file.fd_pid = internal_getpid();<br>> +}<br>> +<br>> void __sanitizer_report_error_summary(const char *error_summary) {<br>>   Printf("%s\n", error_summary);<br>> }<br>><br>> Added: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_fd_test.cc<br>> URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_fd_test.cc?rev=271046&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_fd_test.cc?rev=271046&view=auto</a><br>> ==============================================================================<br>> --- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_fd_test.cc (added)<br>> +++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_fd_test.cc Fri May 27 16:23:05 2016<br>> @@ -0,0 +1,37 @@<br>> +// Test __sanitizer_set_report_fd:<br>> +// RUN: %clangxx -O2 %s -o %t<br>> +// RUN: not %run %t 2>&1   | FileCheck %s<br>> +// RUN: not %run %t stdout | FileCheck %s<br>> +// RUN: not %run %t %t-out && FileCheck < %t-out %s<br>> +<br>> +// REQUIRES: stable-runtime<br>> +// FIXME: implement SEGV handler in other sanitizers, not just asan.<br>> +// XFAIL: msan<br>> +// XFAIL: lsan<br>> +// XFAIL: tsan<br>> +<br>> +#include <sanitizer/common_interface_defs.h><br>> +#include <stdio.h><br>> +#include <string.h><br>> +#include <stdlib.h><br>> +#include <sys/types.h><br>> +#include <sys/stat.h><br>> +#include <fcntl.h><br>> +#include <assert.h><br>> +<br>> +volatile int *null = 0;<br>> +<br>> +int main(int argc, char **argv) {<br>> +  if (argc == 2) {<br>> +    if (!strcmp(argv[1], "stdout")) {<br>> +      __sanitizer_set_report_fd(reinterpret_cast<void*>(1));<br>> +    } else {<br>> +      int fd = open(argv[1], O_CREAT | O_WRONLY | O_TRUNC, S_IRWXU);<br>> +      assert(fd > 0);<br>> +      __sanitizer_set_report_fd(reinterpret_cast<void*>(fd));<br>> +    }<br>> +  }<br>> +  *null = 0;<br>> +}<br>> +<br>> +// CHECK: ERROR: {{.*}} SEGV on unknown address<br>><br>><br>> _______________________________________________<br>> llvm-commits mailing list<br>><span> </span><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>><span> </span><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></div></div></blockquote></div></div></div></blockquote></div></div></div></div></div></div></blockquote></div></div></div></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div></div></div><br></div></blockquote></div><br></div></div>