Re: [PATCH 7/7] Libsanitizer merge from upstream r249633.

[Maxim Ostapenko <m.ostapenko@partner.samsung.com>]

On 14/10/15 10:48, Jakub Jelinek wrote:
> On Tue, Oct 13, 2015 at 02:22:36PM +0300, Maxim Ostapenko wrote:
>> This is the final patch. Force libsanitizer to use an old ABI for ubsan
>> float cast data descriptors, because for some exprs (e.g. that type of
>> tcc_declaration) we can't get the right location for now. I'm not sure about
>> this, perhaps it should be fixed in GCC somehow.
> I don't like this (neither the heuristics on the libubsan, it wouldn't be a
> big deal to add a new library entrypoint).
> If because of the heuristics you need to ensure that the SourceLocation is
> always known, then either you check in ubsan.c whether expand_location
> gives you NULL xloc.file and in that case use old style float cast overflow
> (without location) - i.e. pass 0, NULL, otherwise you use new style, i.e.
> pass 1, &loc.  Or arrange through some special option to emit something like
> { "<unknown>", 0, 0 } instead of { NULL, 0, 0 } for the float cast case.
> And, regardless of this, any progress in making sure we have fewer cases
> with UNKNOWN_LOCATION on this will not hurt.  I think at this point I'd
> prefer the first choice, i.e. using old style for locations without
> filename, and new style otherwise.
>
>> 2015-10-13  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>
>> 	* ubsan/ubsan_handlers.cc (looksLikeFloatCastOverflowDataV1): Always
>> 	return true for now.
>>
>> Index: libsanitizer/ubsan/ubsan_handlers.cc
>> ===================================================================
>> --- libsanitizer/ubsan/ubsan_handlers.cc	(revision 250059)
>> +++ libsanitizer/ubsan/ubsan_handlers.cc	(working copy)
>> @@ -307,6 +307,9 @@
>>   }
>>   
>>   static bool looksLikeFloatCastOverflowDataV1(void *Data) {
>> +  // (TODO): propagate SourceLocation into DataDescriptor and use this
>> +  // heuristic than.
>> +  return true;
>>     // First field is either a pointer to filename or a pointer to a
>>     // TypeDescriptor.
>>     u8 *FilenameOrTypeDescriptor;
>
> 	Jakub
>

Ok, got it. The first solution would require changes in libsanitizer 
because heuristic doesn't work for GCC, so perhaps new UBSan entry point 
should go upstream, right? Or this may be implemented as local patch for 
GCC?

BTW, I actually saw UNKNOWN_LOCATION for this expr:

volatile double var;  // this is tcc_decaration, so we have 
UNKNOWN_LOCATION for it.

I wonder if we need emit __ubsan_handle_float_cast_overflow here at all.