[cfe-commits] [PATCH] Add -fcatch-undefined-behavior runtime library

Kostya Serebryany kcc at google.com
Fri Oct 5 01:28:36 PDT 2012


Other then the includes the patch looks great.
Two comments which you may want to address in this or future patches:
  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.
  sanitizer_common has Printf which you may want to use instead of
fprintf(stderr, ..), for consistency and to allow redirecting to another
file.

--kcc

On Fri, Oct 5, 2012 at 12:03 PM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Thu, Oct 4, 2012 at 10:48 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Thu, Oct 4, 2012 at 3:36 AM, Kostya Serebryany <kcc at google.com> wrote:
>>
>>>
>>> asan/tsan/msan generally avoid #including system headers, especially in
>>> .h files.
>>> Once you start porting the code to non-linux, you'll know why.
>>> compiler-rt/lib/sanitizer_common contains lots of useful stuff that
>>> allows you to avoid using system headers.
>>> WDYT?
>>>
>>
>> I was trying to steer clear of most system headers, but I've tried a bit
>> harder now; new patch attached.
>>
> Cool!
>
>
>>
>> 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.
>>
>
> Mmm. Even with those two we had issues on windows, which forced us to
> have include/sanitizer/common_interface_defs.h
> Again, just for consistency (and for future windows porting) you may want
> to use common_interface_defs.h instead of system headers.
>
>
>> 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.
>>
>
> >> 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?
> No (other than consistency).
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121005/6ad9bc87/attachment.html>


More information about the cfe-commits mailing list