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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 04:58:51 PDT 2019


jhenderson added a comment.

I absolutely agree with @MaskRay that the GNU format is less than ideal, but the overwhelming feedback at last year's BoF on the topic was that we should match the output of GNU unless the output was wrong. If it were up to me, I'd just omit the reference entirely in these instances, since they don't really add any value!

Maybe this is another indication that we need separate GNU and LLVM styles for llvm-objdump, similar to llvm-readobj/llvm-readelf?



================
Comment at: llvm/test/MC/AMDGPU/branch-comment.s:13
 s_branch loop_end_nosym
-// BIN: s_branch 0 // 000000000004: BF820000 <.text+0x8>
+// BIN: s_branch 0 // 000000000004: BF820000 <keep_symbol-0xC+0x8>
 // BIN-NOT: loop_end_nosym:
----------------
This one is particularly concerning. Does this case match GNU? I'd expect it to fold the two sums together (i.e. `keep_symbol-0x4`)


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-no-start-symbol.test:2
+## Test that if there is no symbol at the start of a section, we show a label
+## with the name <first-symbol-name>-0x<offset-of-first-symbol>
+
----------------
Nit: missing trailing full stop. It might be wise to quote the bit after "with the name".


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1155
+    // If the section has no symbol at the start, create one with the name
+    // <first-symbol>-<offset> if later symbol exists; if not, use section name.
+    std::string StartSymbol;
----------------
if later -> if a later


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63280





More information about the llvm-commits mailing list