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

[Alexey Samsonov]

Oops, hit send too early.

On Fri, Oct 5, 2012 at 4:08 PM, Alexey Samsonov <samsonov at google.com> wrote:

> Hi Richard,
>
> The patches are cool
>
> +/// \brief A description of a type.
> +class TypeDescriptor {
> Why don't use bitfields for TypeDescriptor?
>
Or at least a comment (1 bit for sign, 15 bits for width, 16 bits for kind)
etc.

I think later we'll be able to re-use some of your Clang-style error
messages in other sanitizer tools, which is great.


>
>
> +FUNCTIONS.ubsan-i386 :=
> +FUNCTIONS.ubsan-x86_64 :=
> Do you need to define this analogous to the way we define
> AsanFunctions/TsanFunctions etc. I'm sure what this variable
> means (and if it's needed at all) for "make" build system, however :(
>
>
>
> On Fri, Oct 5, 2012 at 12:28 PM, Kostya Serebryany <kcc at google.com> wrote:
>
>> 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).
>>>
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
>
> --
> Alexey Samsonov, MSK
>
>


-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121005/bc66f9a0/attachment.html>