[llvm-readobj] - Refine the error reporting in LLVMStyle<ELFT>::printELFLinkerOptions.

[grimar]

Summary

It is possible to:

1. Avoid using the unwrapOrError calls and hence allow to continue dumping even when something is not OK with one of SHT_LLVM_LINKER_OPTIONS sections.
2. replace reportWarning with reportUniqueWarning calls. In this method it is no-op, because it is not possible to have a duplicated warnings anyways, but since we probably want to switch to reportUniqueWarning globally, this is a good thing to do.

This patch addresses both these points.

[MaskRay]

2. replace reportWarning with reportUniqueWarning calls. In this method it is no-op, because it is not possible to have a duplicated warnings anyways, but since we probably want to switch to reportUniqueWarning globally, this is a good thing to do.

My understanding of the inline comments in D69671 is that we will change reportWarning call sites to use reportUniqueWarning and then rename reportUniqueWarning to reportWarning. Is that the case?

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/linker-options.test

12

Repeating the "index 5" bit in the warning seems sub-optimal. I think it's only necessary if we don't trust the warning produced by the Object library to include the index?

[grimar]

In D83131#2131493, @MaskRay wrote:

2. replace reportWarning with reportUniqueWarning calls. In this method it is no-op, because it is not possible to have a duplicated warnings anyways, but since we probably want to switch to reportUniqueWarning globally, this is a good thing to do.

My understanding of the inline comments in D69671 is that we will change reportWarning call sites to use reportUniqueWarning and then rename reportUniqueWarning to reportWarning. Is that the case?

Yes.

llvm/test/tools/llvm-readobj/ELF/linker-options.test

12

That is why I've introduced it, but now I see that getSectionContents() always includes the index and it is probably unlikely that one day it will get to some another case when it will not do it. I'll update this place.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM.

llvm/test/tools/llvm-readobj/ELF/linker-options.test

 ## Check that we can use the --elf-linker-options option
 ## to dump SHT_LLVM_LINKER_OPTIONS sections.
 
 # RUN: yaml2obj --docnum=1 %s -o %t1
-# RUN: llvm-readobj --elf-linker-options %t1 2>&1 | FileCheck %s --check-prefix=CHECK -DFILE=%t1
+# RUN: llvm-readobj --elf-linker-options %t1 2>&1 | FileCheck %s -DFILE=%t1
 
 # CHECK:      LinkerOptions [
 # CHECK:        option 0: value 0
 # CHECK:        option 1: value 1
 # CHECK-NEXT:   warning: '[[FILE]]': SHT_LLVM_LINKER_OPTIONS section at index 2 is broken: an incomplete key-value pair was found. The last possible key was: "c"
 # CHECK-NEXT:   warning: '[[FILE]]': SHT_LLVM_LINKER_OPTIONS section at index 4 is broken: the content is not null-terminated
+# CHECK-NEXT:   warning: '[[FILE]]': unable to read the content of the SHT_LLVM_LINKER_OPTIONS section: section [index 5] has a sh_offset (0xffffffff) + sh_size (0x8) that is greater than the file size (0x370)
 # CHECK-NEXT:   option 3: value 3
 # CHECK-NEXT: ]
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
## Case 3: in case of an empty section we dump nothing.
   - Name:    .linker-options.empty
     Type:    SHT_LLVM_LINKER_OPTIONS
     Content: ""
 ## Case 4: check we do not attempt to dump data from outside the SHT_LLVM_LINKER_OPTIONS section
 ##         when it is not null-terminated.
   - Name:    .linker-options.nonul
     Type:    SHT_LLVM_LINKER_OPTIONS
     Content: "61"
-## Case 5: another correct case to show we do not stop dumping after reporting a warning.
+## Case 5: check we report a warning when it is not possible to read
+##         the content of the SHT_LLVM_LINKER_OPTIONS section.
+  - Name:     .linker-options.broken.content
+    Type:     SHT_LLVM_LINKER_OPTIONS
+    ShOffset: 0xffffffff
+    Options:
+      - Name:  foo
+        Value: bar
+## Case 6: another correct case to show we do not stop dumping after reporting a warning.
   - Name: .linker-options.valid2
     Type: SHT_LLVM_LINKER_OPTIONS
     Options:
       - Name:  option 3
         Value: value 3
 
 ## llvm-readelf doesn't support --elf-linker-options yet.
 # RUN: llvm-readelf --elf-linker-options %t1 2>&1 | FileCheck %s --check-prefix=READELF
 
 # READELF: printELFLinkerOptions not implemented!

llvm/tools/llvm-readobj/ELFDumper.cpp

void LLVMStyle<ELFT>::printELFLinkerOptions(const ELFFile<ELFT> *Obj) {
   ListScope L(W, "LinkerOptions");
 
   unsigned I = -1;
   for (const Elf_Shdr &Shdr : cantFail(Obj->sections())) {
     ++I;
     if (Shdr.sh_type != ELF::SHT_LLVM_LINKER_OPTIONS)
       continue;
 
-    ArrayRef<uint8_t> Contents =
-        unwrapOrError(this->FileName, Obj->getSectionContents(&Shdr));
-    if (Contents.empty())
+    Expected<ArrayRef<uint8_t>> ContentsOrErr = Obj->getSectionContents(&Shdr);
+    if (!ContentsOrErr) {
+      this->reportUniqueWarning(
+          createError("unable to read the content of the "
+                      "SHT_LLVM_LINKER_OPTIONS section: " +
+                      toString(ContentsOrErr.takeError())));
+      continue;
+    }
+    if (ContentsOrErr->empty())
       continue;
 
-    if (Contents.back() != 0) {
-      reportWarning(createError("SHT_LLVM_LINKER_OPTIONS section at index " +
-                                Twine(I) +
-                                " is broken: the "
-                                "content is not null-terminated"),
-                    this->FileName);
+    if (ContentsOrErr->back() != 0) {
+      this->reportUniqueWarning(
+          createError("SHT_LLVM_LINKER_OPTIONS section at index " + Twine(I) +
+                      "content is not null-terminated"));
       continue;
     }
 
     SmallVector<StringRef, 16> Strings;
-    toStringRef(Contents.drop_back()).split(Strings, '\0');
+    toStringRef(ContentsOrErr->drop_back()).split(Strings, '\0');
     if (Strings.size() % 2 != 0) {
-      reportWarning(
-          createError(
-              "SHT_LLVM_LINKER_OPTIONS section at index " + Twine(I) +
-              " is broken: an incomplete "
-              "key-value pair was found. The last possible key was: \"" +
-              Strings.back() + "\""),
-          this->FileName);
+      this->reportUniqueWarning(createError(
+          Strings.back() + "\""));
       continue;
     }
 
     for (size_t I = 0; I < Strings.size(); I += 2)
       W.printString(Strings[I], Strings[I + 1]);
   }
 }