[PATCH] D95927: DebugInfo/Symbolize: Retrieve filename from the preceding STT_FILE for .symtab symbolization

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 01:18:43 PST 2021


jhenderson accepted this revision.
jhenderson added a comment.

LGTM, with a few nits.



================
Comment at: llvm/test/DebugInfo/Symbolize/ELF/symtab-file-conflict.s:9
+# CHECK-NEXT:  1.c:0:0
+# CHECK-EMPTY:
+
----------------
Nit: Do we actually need the CHECK-EMPTY line?


================
Comment at: llvm/test/DebugInfo/Symbolize/ELF/symtab-file2.yaml:18
+# CHECK-NEXT:  ??:0:0
+# CHECK-EMPTY:
+
----------------
Same comment as above - do we need the trailing CHECK-EMPTY?


================
Comment at: llvm/test/DebugInfo/Symbolize/ELF/symtab-file2.yaml:47
+
+## If st_name of the STT_FILE symbols is invalid, the symbol name is lost as well.
+# RUN: yaml2obj --docnum=2 %s -o %t2
----------------
This is a bit unfortunate - possibly worth adding a TODO saying that we shouldn't throw away the symbol's name?


================
Comment at: llvm/test/DebugInfo/Symbolize/ELF/symtab-file2.yaml:55
+# CHECK2-EMPTY:
+# CHECK2:      ??
+# CHECK2-NEXT: ??:0:0
----------------
Should this be CHECK2-NEXT?


================
Comment at: llvm/test/DebugInfo/Symbolize/ELF/symtab-file2.yaml:57
+# CHECK2-NEXT: ??:0:0
+# CHECK2-EMPTY:
+
----------------
Unnecessary CHECK-EMPTY?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95927



More information about the llvm-commits mailing list