[PATCH] D58796: [llvm-readobj] Display section names for STT_SECTION symbols.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 1 01:21:58 PST 2019


jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

The request changes is due to the test. Otherwise this basically looks fine to me.



================
Comment at: llvm/test/tools/llvm-readobj/symbols.test:17-19
+# Check the GNU presentation of the output.
+RUN: llvm-readelf --symbols %p/Inputs/trivial.obj.elf-i386 \
+RUN:   | FileCheck %s -check-prefix ELF-GNU
----------------
There is already a separate test for this that needs updating instead. See gnu-symbols.test.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:823
+    getSectionNameIndex(Symbol, Syms.begin(), SectionName, SectionIndex);
+    if (SectionName.size())
+      return SectionName;
----------------
I don't think you need this if at all. If the section name is empty and the symbol name is empty, I think we should just print an empty string (section symbols aren't versioned to my knowledge).


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

https://reviews.llvm.org/D58796





More information about the llvm-commits mailing list