[PATCH] D72973: [llvm-objdump] Use symbol index+symbol name + storage mapping class as label for -D

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 27 16:32:49 PDT 2020


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/xcoff-disassemble-all.test:4
+
+# RUN: llvm-objdump -D --symbol-description %p/Inputs/xcoff-section-headers.o | \
+# RUN:   FileCheck --check-prefixes=COMMON,DESC %s
----------------
The entire object file has only one label definition and it is currently not tested. The symbol being used for the address associated with the label definition is currently a `C_STAT` symbol. One way of salvaging the test coverage is to implement the sorting change I am requesting.


================
Comment at: llvm/test/tools/llvm-objdump/xcoff-disassemble-all.test:24
+COMMON: Disassembly of section .text:
+PLAIN:      00000000 .text:
+DESC:       00000000 (idx: 4) .text:
----------------
This has helped me understand something about what is happening.

We should change the sorting so that we prefer symbols with CSECT Auxiliary Entries (i.e., it has a storage mapping class--even if it is a label and we won't print it). We should also prefer labels before csects. In other words, the name should only be considered after the criteria I just listed. The rationale is that the more specific symbol would be selected for the address.

Yes, it does mean that we should generate the XCOFF form of the symbol info always.
@jasonliu @daltenty @sfertile, fyi.

This might be a separate patch; however, note that doing the above as part of this patch happens to fix a test coverage issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72973





More information about the llvm-commits mailing list