[PATCH] D21695: [clang] Version support for UBSan handlers

[Vedant Kumar via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D21695#513723, @filcab wrote:

> In https://reviews.llvm.org/D21695#510788, @vsk wrote:
>
> > After reading through the discussion in https://reviews.llvm.org/D19668, I don't think I understood the pros/cons of using a single ABI check (like asan does) versus adding version numbers to each handler routine. With the latter approach, wouldn't users only be able to link old object files with new runtimes if we never delete old checks? If that's the case, and assuming that we'd like to delete old checks, a single version symbol check would accomplish the same thing.
>
>
> With ASan it's very easy to add a single symbol for all of it, since it's a single pass, and when it runs, instrumentation is added. For UBSan it depends on it being enabled and you hitting a place where it checks for it. We can emit the version check symbol in emitCheckHandlerCall, though.


Ah, yeah, I can see how this might be cumbersome.

> With split versioning, as long as the checks you enable don't get different versions (it's rare that we change checks, too), they'll still work (arguably this is not as valuable as I think it is, but things like Android frameworks enabling UBSan checks in production might make it more valuable).


Running sanitized programs in production sounds strange to me. But, if there isn't really a cost to supporting this, I suppose it's fine.


================
Comment at: lib/CodeGen/CGExpr.cpp:2473
@@ +2472,3 @@
+      ("__ubsan_handle_" + CheckName +
+       (CheckInfo.Version ? "_v" + std::to_string(CheckInfo.Version) : "") +
+       (NeedsAbortSuffix ? "_abort" : ""))
----------------
filcab wrote:
> vsk wrote:
> > Wdyt of dropping the "_vN" component if the version is 0? That's one less compiler-rt change that we'd need.
> That's what it's doing:
>   (CheckInfo.Version ? "_v" + std::to_string(CheckInfo.Version) : "")
> 
Of course! My mistake.


https://reviews.llvm.org/D21695