[PATCH] D109650: [llvm-readobj] [COFF] Resolve relocations pointing at section symbols for arm64 too

Mon Sep 13 04:56:51 PDT 2021

mstorsjo added inline comments.

================
Comment at: llvm/test/tools/llvm-readobj/COFF/arm64-unwind-preferred-symbol2.yaml:18
+    Alignment:       4
+    SectionData:     FE0F1FF8E133009100000094FE0741F8C0035FD6FF8300D1F60B00F9FE0F00F9E1330091E0031FAA00000094FE0F40F9F60B40F9FF830091C0035FD6
+    Relocations:
----------------
jhenderson wrote:
> I've not got much experience with yaml2obj for COFF, but is this reasonably minimal? It looks like a lot of data, for what looks like a fairly small piece of testing.
It's two functions that are nontrivial enough to actually generate unwind info, and one of them even less trivial so that it can't be described with the packed unwind info format. (The packed unwind info format has one relocation, the full format has two relocations, thus wanting to include both forms in the test.)

Sure there's no strict need to have valid code here, but it doesn't cost anything with regards to actual number of lines in this test, and makes for a slightly more understandable example to have the code actually match the unwind info.

================
Comment at: llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp:292
+
+    if (FunctionOnly) // Resolve label/section symbols into function names
+      SymOrErr = getPreferredSymbol(COFF, *SymOrErr, SymbolOffset);
----------------
jhenderson wrote:
> Nit: add missing full stop whilst you're moving this comment.
Sure, will do.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109650