[PATCH] D143693: [llvm-readobj] Add --memtag

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/AArch64/memtag-missing.test:1
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readelf --memtag %t | FileCheck %s
----------------
This file can be folded into `memtag.test` by using `yaml2obj --docnum`.

Being a separate test, `CHECK-NOT: Memtag Android Note` below can easily go stale without being noticed.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/AArch64/memtag-missing.test:5
+
+## Ensure the header is printed, even if there's no relevant dynamic entries.
+# CHECK:      Memtag Dynamic Entries
----------------
I think you can just enumerate all lines since the output is very short.

Add `CHECK-NOT: {{.}}` in the end to assert that there is no additional output.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/AArch64/memtag-missing.test:6
+## Ensure the header is printed, even if there's no relevant dynamic entries.
+# CHECK:      Memtag Dynamic Entries
+# CHECK-NEXT: < none found >
----------------



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:301
+  virtual void printMemtag(
+      const ArrayRef<std::pair<std::string, std::string>> &DynamicEntries,
+      ArrayRef<uint8_t> AndroidNoteDesc) = 0;
----------------
ArrayRef is usually passed by value


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:586
+  void printMemtag(
+      const ArrayRef<std::pair<std::string, std::string>> &DynamicEntries,
+      ArrayRef<uint8_t> AndroidNoteDesc) override;
----------------
ArrayRef is usually passed by value


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:693
+  void printMemtag(
+      const ArrayRef<std::pair<std::string, std::string>> &DynamicEntries,
+      ArrayRef<uint8_t> AndroidNoteDesc) override;
----------------
ArrayRef is usually passed by value


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5899
+    switch (Tag) {
+    case DT_AARCH64_MEMTAG_MODE:
+    case DT_AARCH64_MEMTAG_HEAP:
----------------
These need a `e_machine == EM_AARCH64` check. These values are in the processor-specific dynamic tag range.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5904
+    case DT_AARCH64_MEMTAG_GLOBALS:
+      DynamicEntries.emplace_back(Obj.getDynamicTagAsString(Tag).c_str(),
+                                  getDynamicEntry(Tag, Entry.getVal()));
----------------
Drop `.c_str()`. `getDynamicTagAsString` returns is a std::string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143693