[PATCH] D19668: [ubsan] Minimize size of data for type_mismatch

[Kostya Serebryany via llvm-commits]

On Thu, May 26, 2016 at 11:49 AM, Filipe Cabecinhas <
filcab+llvm.phabricator at gmail.com> wrote:

> Things will break loudly:



Ideally, we should fail to link or at the very least verbosely fail at
startup.
Failing loudly when there is a bug report is the worst option.
But if you can make it verbose enough, let's try.


>
> Newer object file with older lib:
> Out of bounds access guaranteed (strict became smaller), TypeCheckKind
> will be wrong, as well as Alignment.
> I don't expect users to use an older runtime with a newer clang to happen
> much.
>
> Old objects with newer lib:
> Those fields will be wrong too.
>
> If we have the Version field (same struct size as without it), we can
> differentiate, and we can make old objects on the newer lib still work
> (same thing we did for FloatCastOverflow).
>
> I expect lots of false positives with the first two, which should make the
> users double-check their assumptions. But might end up making people spend
> a good chunk of time debugging until they find it out.
>
>
> On Thursday, 26 May 2016, Kostya Serebryany <kcc at google.com> wrote:
>
>> kcc added a comment.
>>
>> In http://reviews.llvm.org/D19668#441261, @filcab wrote:
>>
>> > In http://reviews.llvm.org/D19668#441079, @kcc wrote:
>> >
>> > > Sorry, I missed this CL.
>> > >  Could you please repeat why we need backwards compatibility?
>> > >
>> > > I would rather prefer to not have this complexity, but instead make
>> sure we don't mix old API and the new one,
>> > >  e.g. the same way we do it in asan. (see asan/asan_init_version.h)
>> >
>> >
>> > The backwards compatibility was to be able to mix older files, like
>> when we upgraded the float cast overflow to be able to deal with having the
>> source location or not.
>> >
>> > From your comment it seems you'd prefer one of:
>> >
>> > - Just use the newer and users have to match versions of clang with the
>> ubsan runtime (I think this is acceptable, but I might be an edge case ;-) )
>> > - Create `__ubsan_handle_type_mismatch2` and
>> `__ubsan_handle_type_mismatch2_abort` (or similar) and use the new struct
>> with that (no need to add a Version field if we do it this way, but we'll
>> have functions that might just start to bitrot)
>> >
>> >   I think doing ABI versions the way ASan does them doesn't really work
>> that well for UBSan, since the checks are mostly spot checks, which don't
>> depend on each other. I think the best it to either don't version it and
>> say users are responsible for matching revs, or have versions per check.
>>
>>
>> I don't want to have multiple versions in the code base as it makes code
>> maintenance more expensive.
>> Making "users are responsible for X" work only if the users know when the
>> break X.
>> Will they know it in this case, or things will break silently?
>>
>> > Which one would you prefer (I have mild preference for one, but no
>> strong opinion)?
>>
>>
>>
>> http://reviews.llvm.org/D19668
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160526/2c008c4b/attachment.html>