[PATCH] D156190: [llvm-objdump] -d: don't display mapping symbols as labels

[Job Noorman via Phabricator via llvm-commits]

jobnoorman accepted this revision.
jobnoorman added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1379
+          if (Name.consume_front("$") && Name.size() &&
+              strchr("adtx", Name[0])) {
+            AllMappingSymbols[*SecI].emplace_back(Address - SectionAddr,
----------------
MaskRay wrote:
> jobnoorman wrote:
> > This recognizes every non-`STT_SECTION` symbol that has `SF_FormatSpecific` set with a name that starts with `$[adtx]` as a mapping symbol. I suppose this is correct for the currently supported targets but I see two problems with this:
> > - It partially duplicates the logic [here](https://github.com/llvm/llvm-project/blob/efa43d785ee600ef4cc14589e4777264f0613ec9/llvm/include/llvm/Object/ELFObjectFile.h#L742-L763)
> > - What if some target has `SF_FormatSpecific` non-mapping, non-`STT_SECTION` symbols with the same naming pattern?
> > 
> > It seems to me that `SF_FormatSpecific` is too overloaded. Would it make sense to add a new flag, say `SF_Mapping`? This would also benefit a number of tools that now have duplicate the logic to recognize mapping symbols. For example, [llvm-objdump](https://github.com/llvm/llvm-project/blob/efa43d785ee600ef4cc14589e4777264f0613ec9/llvm/tools/llvm-objdump/llvm-objdump.cpp#L500-L522) and [llvm-nm](https://github.com/llvm/llvm-project/blob/744a968f91f7bb92594a422c1b71f03a47c2415d/llvm/tools/llvm-nm/llvm-nm.cpp#L1801-L1808). Note that the detection is always slightly different which may lead to subtle bugs.
> > 
> > I understand that this is something that should probably not be addresses by this patch but I thought it might be a good place to start the discussion.
> Maybe. I think a separate `SF_Mapping` is clearer but likely won't simplify much. The distinction mostly matters for llvm-nm and llvm-objdump and the two tools need special handling anyway.
> 
> STT_FILE/STT_SECTION/mapping symbols are hidden in GNU nm by different means.
> `nm -a` displays BFD_DEBUGGING symbols, e.g. STT_FILE/STT_SECTION symbols.
> `nm --special-syms` is for mapping symbols.
> For RISC-V, nm has another behavior https://sourceware.org/bugzilla/show_bug.cgi?id=27584 that we don't completely port. Actually, I don't know if filtering out `.L0` in nm by default is a good behavior. In GNU nm, whether BFD is configured with RISC-V makes the behavior different as well.
> 
Thanks for the explanation @MaskRay! I might consider creating a patch for `SF_Mapping` in the future to see if it can simplify some things and we can continue the discussion then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156190