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

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: lld/ELF/Arch/AArch64.cpp:390
+         rel.addend >= static_cast<int64_t>(rel.sym->getSize())))
+      write64(loc, -1 * rel.addend);
+    else
----------------
MaskRay wrote:
> Just use the unary operator.
Unfortunately not possible here. e.g. `~17 != -1 * 17`. While we could change the ABI to be granules-away (and then do a `% 16` on the offset at the end of the calculation), giving an exact offset in bytes gives a nice easy way to debug the exact address of the tag derivation (and doesn't waste any bytes doing so).


================
Comment at: lld/ELF/Driver.cpp:2967
+    for (InputFile* file : files) {
+      // TODO(hctim): Add bitcode (LTO) support.
+      if (file->kind() != InputFile::BinaryKind &&
----------------
MaskRay wrote:
> Avoid referencing an individual user. When needed (actually very rarely done for lld), reference a public issue link.
> 
> In this case, bitcode files should already work. This place is after `compileBitcodeFiles`, so `ctx.objectFiles contains all relocatable object files (including extracted lazy object files) and compiled bitcode files.
Thanks for the tip!


================
Comment at: lld/ELF/SyntheticSections.cpp:1456
       addInt(DT_AARCH64_MEMTAG_STACK, config->androidMemtagStack);
+      if (mainPart->memtagDescriptors && mainPart->memtagDescriptors->getSize()) {
+        addInSec(DT_AARCH64_MEMTAG_GLOBALS, *mainPart->memtagDescriptors);
----------------
MaskRay wrote:
> This is guaranteed to be non-null.
`isNeeded()` is even better :).


================
Comment at: lld/test/ELF/Inputs/aarch64-memtag-globals.s:383
+	.size	global_extern_untagged_definition_but_tagged_import, 4
+
----------------
MaskRay wrote:
> no trailing blank line
assuming you mean no double-trailing-newline, done


================
Comment at: lld/test/ELF/aarch64-memtag-globals.s:19
+# CHECK: Symbol table '.dynsym' contains
+# CHECK-DAG: {{0*}}[[GLOBAL:[0-9a-f]+]] 32 OBJECT GLOBAL DEFAULT {{.*}} global{{$}}
+# CHECK-DAG: {{0*}}[[GLOBAL_UNTAGGED:[0-9a-f]+]] 4 OBJECT GLOBAL DEFAULT {{.*}} global_untagged{{$}}
----------------
MaskRay wrote:
> Ignoring a decimal number prefers `[[#]]`
Yep, for ignoring sizes, done.

For ignoring relocation types, changed to `{{[0-9a-f]+}}` which to me is more understandable than `[[#%x,]]``.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152921