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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!



================
Comment at: lld/ELF/Arch/AArch64.cpp:766
+static bool needsGotForMemtag(const Relocation &rel) {
+  return rel.sym && rel.sym->isTagged() && needsGot(rel.expr);
+}
----------------
`rel.sym && ` can be removed. `relocateAlloc` guarantees that `rel.sym != nullptr`.

Sorry for my suggestion in another place. `R_AARCH64_ABS64` does need a `rel.sym` check as it is used by `relocateNoSym` for thunks and `relocateNonAlloc`. I have done D73254 in the past to clean it a bit. I will do more cleanup so that 
`R_AARCH64_ABS64` can remove the `rel.sym` check as well.


================
Comment at: lld/ELF/Arch/AArch64.cpp:1048
+  for (InputFile *file : files) {
+    // TODO(hctim): Add bitcode (LTO) support.
+    if (file->kind() != InputFile::BinaryKind &&
----------------
Remove this comment. As mentioned, LTO is already supported.


================
Comment at: lld/ELF/SyntheticSections.cpp:3923
+
+  for (const auto &sym : symbols) {
+    if (!includeInSymtab(*sym))
----------------



================
Comment at: lld/ELF/SyntheticSections.cpp:3930
+    if (addr <= kMemtagGranuleSize && buf != nullptr)
+      error("address of the tagged symbol \"" + sym->getName() +
+            "\" falls in the ELF header. This is indicative of a "
----------------
Consider `errorOrWarn` so that `--noinhibit-exec` links can produce output even with errors.

I'm fine with no testing if this is difficult to do.



================
Comment at: lld/ELF/SyntheticSections.cpp:3932
+            "\" falls in the ELF header. This is indicative of a "
+            "compiler/linker bug.");
+    if (addr % kMemtagGranuleSize != 0)
----------------
no trailing full stop


================
Comment at: lld/ELF/Writer.h:58
 bool isMipsR6();
+bool includeInSymtab(const Symbol &b);
 } // namespace lld::elf
----------------
Below `addReservedSymbols` may be a good place. The last few declarations are for mips.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152921