On Fri, Oct 5, 2012 at 1:03 AM, 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

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.<br>

<div class="gmail_quote"><div><br></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></div></div></blockquote><div><br></div>Done.<div><br><div class="gmail_quote">On Fri, Oct 5, 2012 at 1:28 AM, 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">Other then the includes the patch looks great. </blockquote><div><br></div><div>Thanks, committed in r165533.</div><div>

 </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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></blockquote><div><br></div><div>Thanks, I'll have a look.</div>

<div><br></div><div>On Fri, Oct 5, 2012 at 5:08 AM, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></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>+FUNCTIONS.ubsan-i386 :=</div><div><div>+FUNCTIONS.ubsan-x86_64 :=</div><div>Do you need to define this analogous to the way we define AsanFunctions/TsanFunctions etc. I'm sure what this variable</div><div></div>
</div></blockquote><div><br></div><div>I've taken out the configure/make support for now. It was undertested and I wasn't confident it was right. We can add it back in with appropriate testing and validation if there's a desire for that.</div>
</div><div><br></div><div>On Fri, Oct 5, 2012 at 5:32 AM, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></span> wrote:<br><div class="gmail_quote"><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 class="gmail_quote"><div class="im">On Fri, Oct 5, 2012 at 4:08 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></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">
Hi Richard,<div><br></div><div>The patches are cool</div><div><br></div><div><div>+/// \brief A description of a type.</div><div>+class TypeDescriptor {</div></div><div>Why don't use bitfields for TypeDescriptor? </div>
</blockquote></div><div></div></div></blockquote><div><br></div><div>I've switched to using two u16 fields and added explanatory comments.</div><div><br></div><div>On Tue, Oct 9, 2012 at 1:46 AM, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></span> wrote:<br>
<div class="gmail_quote"><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">On Tue, Oct 9, 2012 at 1:29 AM, Michael Spencer <span dir="ltr"><<a href="mailto:bigcheesegs@gmail.com" target="_blank">bigcheesegs@gmail.com</a>></span> wrote:<br>
<div class="gmail_quote"><div><div class="h5"><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>This fails to compile with -Werror. ubsan_value.cc has 3 functions</div>
with something similar to:<br><br>  SIntMax Value::getSIntValue() const {<br>    ...<br>    CHECK(0 && "unexpected bit width");<br>  }<br><br>Adding __builtin_unreachable(); fixes this. Does compiler-rt have a<br>
compatibility macro for this?<br></blockquote><div><br></div></div></div><div>Good idea. I've added UNREACHABLE(msg) macro to common sanitizer defs in r165492.</div></div></blockquote><div><br></div><div>Thanks, I've switched over to using it. </div>
</div></div></div></div></div></div>