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

[Filipe Cabecinhas via llvm-commits]

I'm not sure what you mean here. Could you be more explicit?

To make sure we're on the same page: if we don't even try to account for
the difference in version, you can end up with lots of false positive
errors, but these will be printed as regular errors. We won't be able to
diagnose the older version unless we add a field to check for this.

On Thursday, 26 May 2016, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Thu, May 26, 2016 at 11:49 AM, Filipe Cabecinhas <
> filcab+llvm.phabricator at gmail.com
> <javascript:_e(%7B%7D,'cvml','filcab%2Bllvm.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
>> <javascript:_e(%7B%7D,'cvml','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/a13525f3/attachment.html>