[PATCH] D152921: [lld] Synthesize metadata for MTE globals

[Mitch Phillips via Phabricator via llvm-commits]

hctim marked 2 inline comments as done.
hctim added inline comments.


================
Comment at: lld/ELF/SyntheticSections.h:1272
+  bool isNeeded() const override {
+    return !config->isStatic && !symbols.empty();
+  }
----------------
MaskRay wrote:
> hctim wrote:
> > hctim wrote:
> > > MaskRay wrote:
> > > > Why is the `!config->isStatic &&` condition?
> > > > 
> > > > `isConfig` can be toggled by `-Bstatic`/`-static` and `-Bdynamic`. 
> > > > `-Bstatic` can be used with a dynamically linked executable as well.
> > > MTE globals are not supported for fully static executables, there needs to be a dynamic loader that processes relocations.
> > > 
> > > Is there a better way to describe this in code? Maybe pulling `needsInterpSection` out of Writer.cpp?
> > @MaskRay - any suggestions here?
> Yes, moving `needsInterpSection` to `Writer.h` and exposing it here looks good to me.
> 
> You will need to move `isNeeded` to `SyntheticSections.cpp` as we cannot make SyntheticSections.h include Writer.h.
> 
> Ideally SyntheticSections.cpp does not use more functions from Writer.h, so SyntheticSections.cpp defining `needsInterpSection` is also fine.
Yeah, so it needed a little bit more precise of a check, which I put straight in the relevant bit of `Writer.cpp` (so it doesn't need to be exported).

We don't allow MTE globals for fully-static binaries, as the loader needs to do some work. I added a test with `--static` (and nothing else, as this is what `clang -static` seems to emit, along with ctrbegin/end as static) as well. We basically error-out when you try and `--android-memtag-mode` on a non-DSO non-partially-linked (`--relocatable`) non-interpreted binary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152921