[llvm-readobj] - Improve dumping of the SHT_LLVM_LINKER_OPTIONS sections.

[grimar]

Summary

I've added a few tests that shows how the current code could overrun the section data
buffer while dumping. I had to rewrite the code to fix this.
Main problem was that the current code assumed that the section data is correct.
But when it is not - it failed.

[grimar]

• Added a test for "empty section" case.

[jhenderson]

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

32

attemp -> attempt
a data -> data
outside of -> outside

39

Should we make these INCOMPLETE-NEXT to show that nothing is printed for the entry? In that case, what do you think about adding a valid pair before to show that earlier valid entries are (not) printed?

56

Same comments for this case as above (both comments and check).

llvm/tools/llvm-readobj/ELFDumper.cpp

6038

found, the -> found. The

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

6023

Should this be continue?

[compnerd]

llvm/tools/llvm-readobj/ELFDumper.cpp

6020

Contents is the correct word for this.

6029

This should probably be a continue.

[grimar]

Thanks for all the comments. I am going to be off until the next week, so will update this patch after 18th of november.

[grimar]

• Addressed review comments and reimplemented.

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

39

Yeah, I had to significantly change the structure here.

Using returns in the code was not correct I think,
then, if we want to continue dumping sections after reporting a warning, it is probably worth
to show how we dump the possible mixed cases too. So I've combined the valid/invalid cases and improved the error messages displayed.

llvm/tools/llvm-readobj/ELFDumper.cpp

6023

Right. I've changed all the logic here to continue dumping sections even after reporting an error.

6029

Yes.

[jhenderson]

LGTM with two nits.

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

14

'"c' -> "c"?

43

a one more -> another

[grimar]

@MaskRay, @compnerd, do you have more comments?

[MaskRay]

LG once @jhenderson's comments are addressed.

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 %s -o %t1
+# RUN: yaml2obj --docnum=1 %s -o %t1
 # RUN: llvm-readobj --elf-linker-options %t1 | FileCheck %s
 
 # CHECK:      LinkerOptions [
 # CHECK-NEXT:  option 0: value 0
 # CHECK-NEXT:  option 1: value 1
 # CHECK-NEXT: ]
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name: .linker-options
     Type: SHT_LLVM_LINKER_OPTIONS
     Options:
       - Name:  option 0
         Value: value 0
       - Name:  option 1
         Value: value 1
 
 ## 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!
+
+## Check we do not attemp to dump a data from outside of the SHT_LLVM_LINKER_OPTIONS section
+## when it contains an incomplete key-value pair.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-readobj --elf-linker-options %t2 2>&1 | FileCheck %s --check-prefix=INCOMPLETE -DFILE=%t2
+
+# INCOMPLETE: LinkerOptions [
+# INCOMPLETE:   warning: '[[FILE]]': SHT_LLVM_LINKER_OPTIONS is broken: an incomplete key-value pair was found, the last possible key was: "foo"
+# INCOMPLETE: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:    .linker-options
+    Type:    SHT_LLVM_LINKER_OPTIONS
+    Content: "666f6f00" ## 'f', 'o', 'o', '\0'
+  - Type:    Fill
+    Pattern: "FF"
+    Size:    "1"
+
+## Check we do not attemp to dump a data from outside of the SHT_LLVM_LINKER_OPTIONS section
+## when it is not null-terminated.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-readobj --elf-linker-options %t3 2>&1 | FileCheck %s --check-prefix=NOT-NULL-TERMINATED -DFILE=%t3
+
+# NOT-NULL-TERMINATED: LinkerOptions [
+# NOT-NULL-TERMINATED:   warning: '[[FILE]]': SHT_LLVM_LINKER_OPTIONS is broken: the content is not null-terminated
+# NOT-NULL-TERMINATED: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:    .linker-options
+    Type:    SHT_LLVM_LINKER_OPTIONS
+    Content: "61"
+  - Type:    Fill
+    Pattern: "6200"
+    Size:    "2"
+
+## Check what we do for an empty SHT_LLVM_LINKER_OPTIONS section.
+
+# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: llvm-readobj --elf-linker-options %t4 2>&1 | FileCheck %s --check-prefix=EMPTY
+
+# EMPTY:      LinkerOptions [
+# EMPTY-NEXT: ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:    .linker-options
+    Type:    SHT_LLVM_LINKER_OPTIONS
+    Content: ""

llvm/tools/llvm-readobj/ELFDumper.cpp

 template <class ELFT>
 void LLVMStyle<ELFT>::printELFLinkerOptions(const ELFFile<ELFT> *Obj) {
   ListScope L(W, "LinkerOptions");
 
   for (const Elf_Shdr &Shdr : unwrapOrError(this->FileName, Obj->sections())) {
     if (Shdr.sh_type != ELF::SHT_LLVM_LINKER_OPTIONS)
       continue;
 
-    ArrayRef<uint8_t> Contents =
+    ArrayRef<uint8_t> Content =
         unwrapOrError(this->FileName, Obj->getSectionContents(&Shdr));
-    for (const uint8_t *P = Contents.begin(), *E = Contents.end(); P < E; ) {
-      StringRef Key = StringRef(reinterpret_cast<const char *>(P));
-      StringRef Value =
-          StringRef(reinterpret_cast<const char *>(P) + Key.size() + 1);
+    if (Content.empty())
+      return;
 
-      W.printString(Key, Value);
+    if (Content.back() != 0) {
+      reportWarning(createError("SHT_LLVM_LINKER_OPTIONS is broken: the "
+                                "content is not null-terminated"),
+                    FileName);
+      return;
+    }
 
-      P = P + Key.size() + Value.size() + 2;
+    SmallVector<StringRef, 16> Strings;
+    toStringRef(Content.drop_back()).split(Strings, '\0');
+    if (Strings.size() % 2 != 0) {
+      reportWarning(
+          createError(
+              "SHT_LLVM_LINKER_OPTIONS is broken: an incomplete "
+              "key-value pair was found, the last possible key was: \"" +
+              Strings.back() + "\""),
+          FileName);
+      return;
     }
+
+    for (size_t I = 0; I < Strings.size(); I += 2)
+      W.printString(Strings[I], Strings[I + 1]);
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printStackSizes(const ELFObjectFile<ELFT> *Obj) {
   ListScope L(W, "StackSizes");
   if (Obj->isRelocatableObject())
     this->printRelocatableStackSizes(Obj, []() {});