[PATCH] D69671: [llvm-readobj] Change errors to warnings for symbol section name dumping

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/elf-section-symbols.test:42
+
+# WARN2: warning: '{{.*}}.tmp2': extended symbol index (1) is past the end of the SHT_SYMTAB_SHNDX section of size 0
+
----------------
May be regroup to isolate test cases? (they are not very similar, it is easier to think about them as about different tests and hence
read them separatelly. It also might be helpful if we will want to add more cases here not to mix them all).

```
# RUN: yaml2obj %s -o %t1
# RUN: llvm-readobj %t1 --symbols 2> %t.llvm.err1 | FileCheck %s --check-prefix=LLVM1
# RUN: FileCheck %s --input-file %t.llvm.err1 --check-prefix=WARN1
# RUN: llvm-readelf %t1 --symbols 2> %t.gnu.err1 | FileCheck %s --check-prefix=GNU1
# RUN: FileCheck %s --input-file %t.gnu.err1 --check-prefix=WARN1

# LLVM1: ....

# GNU1:   ...

# WARN1: ...

<YAML1>

# RUN: yaml2obj %s --docnum=2 -o %t2
# RUN: llvm-readobj %t2 --symbols 2> %t.llvm.err2 | FileCheck %s --check-prefix=LLVM2
# RUN: FileCheck %s --input-file %t.llvm.err2 --check-prefix=WARN2
# RUN: llvm-readelf %t2 --symbols 2> %t.gnu.err2 | FileCheck %s --check-prefix=GNU2
# RUN: FileCheck %s --input-file %t.gnu.err2 --check-prefix=WARN2

# LLVM2: ....

# GNU2:   ...

# WARN2: ...

<YAML1>
```



================
Comment at: llvm/test/tools/llvm-readobj/elf-symbol-shndx.test:1
-# Show that llvm-readobj prints symbol shndxes correctly, for valid indexes,
-# invalid indexes (i.e. section indexes that don't correspond to a real
-# section), reserved values and processor/os-specific index values, for both GNU
-# and LLVM styles.
-
-# RUN: yaml2obj --docnum=1 %s > %t1
-# RUN: llvm-readobj --symbols %t1 | FileCheck %s --check-prefix=LLVM1
-# RUN: llvm-readelf --symbols %t1 | FileCheck %s --check-prefix=GNU1
-
-# llvm-readobj doesn't accept shndx values that are not valid section indexes
-# for LLVM style, so only test GNU output in this case.
-# RUN: yaml2obj --docnum=2 %s > %t2
-# RUN: llvm-readelf --symbols %t2 | FileCheck %s --check-prefix=GNU2
-
-# LLVM1: Name:    undef
-# LLVM1: Section:
-# LLVM1-SAME:     Undefined (0x0)
-# LLVM1: Name:    normal
-# LLVM1: Section:
-# LLVM1-SAME:     .text (0x1)
-# LLVM1: Name:    common
-# LLVM1: Section:
-# LLVM1-SAME:     Common (0xFFF2)
-# LLVM1: Name:    absolute
-# LLVM1: Section:
-# LLVM1-SAME:     Absolute (0xFFF1)
-# LLVM1: Name:    proc
-# LLVM1: Section:
-# LLVM1-SAME:     Processor Specific (0xFF01)
-# LLVM1: Name:    os
-# LLVM1: Section:
-# LLVM1-SAME:     Operating System Specific (0xFF21)
-# LLVM1: Name:    reserved
-# LLVM1: Section:
-# LLVM1-SAME:     Reserved (0xFFFE)
-# LLVM1: Name:    xindex
-# LLVM1: Section:
-# LLVM1:          .text (0x1)
-
-# GNU1:      Symbol table '.symtab' contains 9 entries:
-# GNU1-NEXT:   Num: {{.*}} Ndx Name
-# GNU1-NEXT:     0: {{.*}} UND
-# GNU1-NEXT:     1: {{.*}} UND undef
-# GNU1-NEXT:     2: {{.*}}   1 normal
-# GNU1-NEXT:     3: {{.*}} COM common
-# GNU1-NEXT:     4: {{.*}} ABS absolute
-# GNU1-NEXT:     5: {{.*}} PRC[0xff01] proc
-# GNU1-NEXT:     6: {{.*}} OS[0xff21] os
-# GNU1-NEXT:     7: {{.*}} RSV[0xfffe] reserved
-# GNU1-NEXT:     8: {{.*}}   1 xindex
-
-# GNU2:      Symbol table '.symtab' contains 2 entries:
-# GNU2-NEXT:   Num: {{.*}} Ndx Name
-# GNU2-NEXT:     0: {{.*}} UND
-# GNU2-NEXT:     1: {{.*}}  66 bad
-
---- !ELF
-FileHeader:
-  Class:   ELFCLASS32
-  Data:    ELFDATA2LSB
-  Type:    ET_REL
-  Machine: EM_386
-Sections:
-  - Name: .text
-    Type: SHT_PROGBITS
-  - Name: .symtab_shndx
-    Type: SHT_SYMTAB_SHNDX
-    Link: .symtab
-    Entries: [ 0, 0, 0, 0, 0, 0, 0, 0, 1 ]
-Symbols:
-  - Name:    undef
-    Binding: STB_GLOBAL
-  - Name:    normal
-    Section: .text
-    Binding: STB_GLOBAL
-  - Name:    common
-    Index:   SHN_COMMON
-    Binding: STB_GLOBAL
-  - Name:    absolute
-    Index:   SHN_ABS
-    Binding: STB_GLOBAL
-  - Name:    proc
-    Index:   0xff01
-    Binding: STB_GLOBAL
-  - Name:    os
-    Index:   0xff21
-    Binding: STB_GLOBAL
-  - Name:    reserved
-    Index:   0xfffe
-    Binding: STB_GLOBAL
-  - Name:    xindex
-    Index:   SHN_XINDEX
-    Binding: STB_GLOBAL
-
---- !ELF
-FileHeader:
-  Class:   ELFCLASS32
-  Data:    ELFDATA2LSB
-  Type:    ET_REL
-  Machine: EM_386
-Symbols:
-  - Name:    bad
-    Index:   0x42
-    Binding: STB_GLOBAL
+## Show that llvm-readobj prints symbol shndxes correctly, for valid indexes,
+## invalid indexes (i.e. section indexes that don't correspond to a real
----------------
The same suggestion about regrouping here.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:562
+template <class ELFT>
+void DumpStyle<ELFT>::reportUniqueWarning(Error Err) const {
+  handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
----------------
Our warnings are always unique. Should it be just `reportWarning` (it is much more common name for reporting)?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:840
+    if (!SectionIndex)
+      return "<?>";
+    Expected<StringRef> NameOrErr = getSymbolSectionName(Symbol, *SectionIndex);
----------------
I think you need to `consumeError()` here.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5496
+  } else
+    W.printHex("Section", *SectionName, *SectionIndex);
 }
----------------
Wrap it into curly bracers too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69671