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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I'm OK in principle with the chage, particularly as it matches GNU objdump. I sometimes find mapping symbols useful for Assembly language when mixing Arm and Thumb in the same function, that isn't particularly common in practice though, and we can get them back with --show-all-syms.

Some minor suggestions to the code.



================
Comment at: llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h:35
+  // Used by ELF to describe a mapping symbol that is usually not displayed.
+  bool MappingSymbol;
 
----------------
Would `IsMappingSymbol` be a better name. It would match the form of IsXCOFF and reads a bit better in statements like `if (a.IsMappingSymbol)` . Not a strong opinion though.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1691
         End = std::min(End, Symbols[SI].Addr);
+
       if (Start >= End || End <= StartAddress)
----------------
Was this change intentional? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156190