filcab added a comment.

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.

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).
With a single version for "UBSan", you get more problems:

- Should you rev when *adding* check handlers?
  - If so, why would I need a newer lib just because it includes a new check, 
even if I don't use that?
- You'll need to rev up, and use a newer version of the library even if the 
checks' interface (for the ones you're using) hasn't changed.


================
Comment at: lib/CodeGen/CGExpr.cpp:2473
@@ +2472,3 @@
+      ("__ubsan_handle_" + CheckName +
+       (CheckInfo.Version ? "_v" + std::to_string(CheckInfo.Version) : "") +
+       (NeedsAbortSuffix ? "_abort" : ""))
----------------
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) : "")



https://reviews.llvm.org/D21695



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to