[PATCH] D145761: [MTE] [llvm-readobj] Add globals section parsing to --memtag

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

(I'm going to be out for 3 weeks in the next 4 weeks. Happy if others are happy. Don't wait on me :) )



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5974
+  std::vector<std::pair<uint64_t, uint64_t>> GlobalDescriptors;
+
+  uint64_t Address = 0;
----------------
Delete blank line.

Also, add a link to the ABI doc describing the encoding scheme?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7409
+    for (const auto &Descriptor : Descriptors) {
+      W.printHex(std::string("0x") + llvm::utohexstr(Descriptor.first),
+                Descriptor.second);
----------------
jhenderson wrote:
> Drop `llvm::`. Also, the `std::string` construction is unnecessary. Also, consider using `Twine` here and in other string concatenation places.
`std::string("0x")` => `"0x"` as `llvm::utohexstr` returns a `std::string`.

`printHex` takes a `StringRef`, so `Twine` cannot be used (without `toStringRef`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145761