Other then the includes the patch looks great. <div>Two comments which you may want to address in this or future patches: </div><div> sanitizer_common has all required code to unwind and print symbolized stack. It might be useful for environments that don't have stack unwinder/symbolizer on SIGILL. </div>
<div> sanitizer_common has Printf which you may want to use instead of fprintf(stderr, ..), for consistency and to allow redirecting to another file. </div><div><br></div><div>--kcc <br><br><div class="gmail_quote">On Fri, Oct 5, 2012 at 12:03 PM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br><br><div class="gmail_quote"><div class="im">On Thu, Oct 4, 2012 at 10:48 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On Thu, Oct 4, 2012 at 3:36 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br></div><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>asan/tsan/msan generally avoid #including system headers, especially in .h files. </div><div>Once you start porting the code to non-linux, you'll know why. </div><div>compiler-rt/lib/sanitizer_common contains lots of useful stuff that allows you to avoid using system headers. </div>
<div>WDYT? </div></blockquote><div><br></div></div><div>I was trying to steer clear of most system headers, but I've tried a bit harder now; new patch attached.</div></div></blockquote></div><div>Cool! </div><div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><br></div><div>I'm not concerned about the includes in ubsan_diag.cc, since I intend for that code to be replaced in the medium term (and to be made user-replaceable -- some applications will want to provide their own reporting functionality). That only leaves <stdint.h> and <stddef.h>, which are both provided by Clang.</div>
</div></blockquote><div><br></div></div><div>Mmm. Even with those two we had issues on windows, which forced us to have include/sanitizer/common_interface_defs.h</div><div>Again, just for consistency (and for future windows porting) you may want to use common_interface_defs.h instead of system headers. </div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div> I'm not hugely interested in making the runtime build with any other compiler, since you need to have a Clang which can target your platform anyway in order for it to be useful.</div>
</div>
</blockquote></div></div><br><div class="im"><div>>> <span style="color:rgb(34,34,34);font-size:13px;font-family:arial,sans-serif">Since these symbols are already extern "C", their mangled names are just __ubsan_handle_divrem_overflow etc, so there's no difference from a debugging perspective. Is there a benefit to moving them out of the namespace?</span></div>
</div><div><span style="color:rgb(34,34,34);font-size:13px;font-family:arial,sans-serif">No (other than </span>consistency). </div><div><br></div>
</blockquote></div><br></div>