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

[Kostya Serebryany via llvm-commits]

On Thu, May 26, 2016 at 2:32 PM, Filipe Cabecinhas <
filcab+llvm.phabricator at gmail.com> wrote:

> 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.
>

That's clearly unacceptable. So, the only reasonable thing to do here is to
ensure at link-time that the versions (compiler+run-time) match (as in asan)


>
>
> 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> 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/92572c98/attachment.html>