[PATCH] D152921: [lld] Synthesize metadata for MTE globals
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 11 15:36:51 PDT 2023
MaskRay 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
----------------
Just use the unary operator.
================
Comment at: lld/ELF/Driver.cpp:2588
+
+ auto f = [&](auto relArray) {
+ for (const typename ELFT::Rela &rel : relArray) {
----------------
the lambda is only used once. just expand it.
================
Comment at: lld/ELF/Driver.cpp:2595
+ // STT_NOTYPE. In addition, this save us from checking untaggable symbols,
+ // like functions or TLS blocks.
+ if (sym.type != STT_OBJECT)
----------------
TLS blocks => TLS symbols
================
Comment at: lld/ELF/Driver.cpp:2604
+ }
+ referenceCount[&sym]++;
+ }
----------------
LLVM coding style prefers pre-increments
================
Comment at: lld/ELF/Driver.cpp:2959
+ // demote the symbol to be untagged.
+ if (config->emachine == EM_AARCH64 &&
+ config->androidMemtagMode != ELF::NT_MEMTAG_LEVEL_NONE) {
----------------
We probably should move the whole function and referenced function template `addTaggedSymbolReferences` to AArch64.cpp.
================
Comment at: lld/ELF/Driver.cpp:2967
+ for (InputFile* file : files) {
+ // TODO(hctim): Add bitcode (LTO) support.
+ if (file->kind() != InputFile::BinaryKind &&
----------------
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.
================
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);
----------------
This is guaranteed to be non-null.
================
Comment at: lld/ELF/SyntheticSections.cpp:3926
+ continue;
+ const uint64_t vAddr = sym->getVA();
+ const uint64_t size = sym->getSize();
----------------
We typically just use `addr` instead of `vAddr`.
================
Comment at: lld/ELF/SyntheticSections.cpp:3937
+ if (size == 0)
+ error("size of the tagged symbol \"" + sym->getName() + "\" at 0x" +
+ Twine::utohexstr(vAddr) + "\" is not allowed to be zero");
----------------
When buf is null, addr is 0. If size is 0, we'd report an error without knowing the actual address. We should drop the address from the diagnostic.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152921/new/
https://reviews.llvm.org/D152921
More information about the llvm-commits
mailing list