<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 26, 2016 at 11:49 AM, Filipe Cabecinhas <span dir="ltr"><<a href="mailto:filcab+llvm.phabricator@gmail.com" target="_blank">filcab+llvm.phabricator@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Things will break loudly:</blockquote><div><br></div><div><br></div><div>Ideally, we should fail to link or at the very least verbosely fail at startup. </div><div>Failing loudly when there is a bug report is the worst option. <br></div><div>But if you can make it verbose enough, let's try. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>Newer object file with older lib:</div><div>Out of bounds access guaranteed (strict became smaller), TypeCheckKind will be wrong, as well as Alignment.<br>I don't expect users to use an older runtime with a newer clang to happen much.</div><div><br></div><div>Old objects with newer lib:</div><div>Those fields will be wrong too.</div><div><br></div><div>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).</div><div><br></div><div>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.<div><div class="h5"><span></span><br><br>On Thursday, 26 May 2016, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">kcc added a comment.<br>
<br>
In <a href="http://reviews.llvm.org/D19668#441261" target="_blank">http://reviews.llvm.org/D19668#441261</a>, @filcab wrote:<br>
<br>
> In <a href="http://reviews.llvm.org/D19668#441079" target="_blank">http://reviews.llvm.org/D19668#441079</a>, @kcc wrote:<br>
><br>
> > Sorry, I missed this CL.<br>
> >  Could you please repeat why we need backwards compatibility?<br>
> ><br>
> > I would rather prefer to not have this complexity, but instead make sure we don't mix old API and the new one,<br>
> >  e.g. the same way we do it in asan. (see asan/asan_init_version.h)<br>
><br>
><br>
> 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.<br>
><br>
> From your comment it seems you'd prefer one of:<br>
><br>
> - 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 ;-) )<br>
> - 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)<br>
><br>
>   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.<br>
<br>
<br>
I don't want to have multiple versions in the code base as it makes code maintenance more expensive.<br>
Making "users are responsible for X" work only if the users know when the break X.<br>
Will they know it in this case, or things will break silently?<br>
<br>
> Which one would you prefer (I have mild preference for one, but no strong opinion)?<br>
<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19668" target="_blank">http://reviews.llvm.org/D19668</a><br>
<br>
<br>
<br>
</blockquote></div></div></div>
</blockquote></div><br></div></div>