[PATCH] D143159: [SanitizerBinaryMetadata] Treat constant globals and non-escaping addresses specially

[Marco Elver via Phabricator via llvm-commits]

melver marked an inline comment as done.
melver added a comment.

PTAL.

Reworked some things to make it more efficient in the presence of constant data accesses.



================
Comment at: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp:383
+      !PointerMayBeCaptured(Addr, true, true)) {
+    // The addressable object is on the stack and does not escape.
+    return false;
----------------
dvyukov wrote:
> melver wrote:
> > dvyukov wrote:
> > > What if we pretend these are atomic as well?
> > Marking things as atomic increases binary size. It's better to just not emit any metadata (less space used).
> > 
> > There are plenty function-local objects and if we mark all accesses to them as atomic, we'll end up with larger binaries.
> > 
> > Of course this will only help if all accesses in a function are non-escaping. If there's at least e.g. 1 global access, the function will end up being "covered" with atomics metadata again.
> But I think it's highly-likely a function also contains non-local accesses (esp for optimized builds with inlining where functions tend to be larger).
> Maybe we should skip functions that contain only atomics accesses and local/non-interesting accesses (contains at least 1 interesting access).
> 
Changed it to look for shared+mutable.

So functions that access only non-escaping addresses and constant globals will be excluded entirely.

If a function is already covered by atomics metadata, constant globals will be considered atomic.

However, I think we still shouldn't pretend accesses to non-escaping memory are atomic because we'll increase binary size. Constant accesses should be relatively rare and can easily be optimized.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143159/new/

https://reviews.llvm.org/D143159