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

Mon Apr 10 14:15:18 PDT 2023

hctim added inline comments.

================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5908
+    if (auto E = Contents.takeError()) {
+      reportUniqueWarning("couldn't get SHT_AARCH64_MEMTAG_GLOBALS_DYNAMIC section contents: " + toString(std::move(E)));
+      return ArrayRef<uint8_t>();
----------------
jhenderson wrote:
> hctim wrote:
> > jhenderson wrote:
> > > I don't think this is tested?
> > this isn't really testable here; `getSectionContents()` has a few failure conditions but none of them are really memtag-relevant.
> > 
> > i have however added a check that the address of the SHT_AARCH64_MEMTAG_GLOBALS_DYNAMIC section is the same as the DT_AARCH64_MEMTAG_GLOBALS dynamic table entry, and tested it.
> We should still have a test that shows that if `getSectionContents` fails, we handle the failure appropriately. The easiest way of doing this is probably to simply override the sh_offset value of the section, using the `ShOffset` YAML key. There are a number of good examples of things like this in llvm-readobj tests. A simple one is in `stackmap.test`, where the same YAML is used, but with the yaml2obj -D option used to provide a specific sh_offset value for one test case that is for the equivalent of what you're checking here.
thanks for the handy link to `stackmap.test`, done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145761