[PATCH] D63280: [llvm-objdump] Use <first-symbol>-<offset> as the section start symbol

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1383
+              // symbol.
+              if (TargetAddress == SectionAddr && TargetName.contains("-0x")) {
+                ++TargetSym;
----------------
jhenderson wrote:
> This check is rather horrid, if I'm honest, for all sorts of reasons (apart from anything else, ELF places no restrictions on symbol names so a valid symbol name could actually contain "-0x").
> 
> I think you need a different way of identifying these inserted symbols (assuming that it's not possible to just change when they are inserted). You might need a list of sections that have had symbols inserted, for example.
> This check is rather horrid, if I'm honest, for all sorts of reasons (apart from anything else, ELF places no restrictions on symbol names so a valid symbol name could actually contain "-0x").
It is horrible. I was assuming the low chance of an ELF name having "-0x" before this start symbol insertion, but it is still legitimate.

> I think you need a different way of identifying these inserted symbols (assuming that it's not possible to just change when they are inserted). 
Yes, the insertion is just before disassembling so it could happen earlier but not later.

>You might need a list of sections that have had symbols inserted, for example.
Did exactly this. I think it does the right thing.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63280