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

Wed Jul 26 02:59:45 PDT 2023

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,
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156190