[PATCH] D153027: [llvm-objdump] --adjust-vma adjust symbol table

Tue Jun 20 00:44:59 PDT 2023

jhenderson added a comment.

As this patch is supposed to be fixing an issue in Github, please add "Fixes #63203" to your commit message for this patch.

================
Comment at: llvm/test/tools/llvm-objdump/X86/adjust-vma.test:35-38
+# ADJUST:      SYMBOL TABLE:
+# ADJUST-NEXT:  0000000000123001  l F .text 0000000000000000 func
+# ADJUST-NEXT:  0000000000123000  l   .text 0000000000000000 sym
+# ADJUST-NEXT:  0000000000123000  l d .text 0000000000000000 .text
----------------
Nit: You will see in the other parts of this test that the ADJUST blocks are indented slightly to ensure they line up with the NOADJUST blocks. Please could you do the same here, to make it easier to read and compare.

================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2403
                                    ArchitectureName);
+  Address += AdjustVMA;
   if ((Address < StartAddress) || (Address > StopAddress))
----------------
mysterymath wrote:
> This needs a `shouldAdjustVA(Section)` guard like other places where the adjustment is added, to avoid adjusting e.g. non-alloc sections.
We need a test case that shows that this `shouldAdjustVA` function has been called. I don't believe any of the symbols in the test you've added are impacted by this? You can add a symbol to the YAML in the test file that is in the .debug_str section, to achieve this, I believe.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153027