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

[Georgii Rymar via llvm-commits]

Author: Georgii Rymar
Date: 2019-11-20T12:11:13+03:00
New Revision: 19ddba95513af4e673ce6724e3a87077c278354c

URL: https://github.com/llvm/llvm-project/commit/19ddba95513af4e673ce6724e3a87077c278354c
DIFF: https://github.com/llvm/llvm-project/commit/19ddba95513af4e673ce6724e3a87077c278354c.diff

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

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.

Differential revision: https://reviews.llvm.org/D70112

Added: 
    

Modified: 
    llvm/test/tools/llvm-readobj/elf-linker-options.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-readobj/elf-linker-options.test b/llvm/test/tools/llvm-readobj/elf-linker-options.test
index ca4884e24591..488f2b16f2d1 100644
--- a/llvm/test/tools/llvm-readobj/elf-linker-options.test
+++ b/llvm/test/tools/llvm-readobj/elf-linker-options.test
@@ -1,13 +1,18 @@
 ## Check that we can use the --elf-linker-options option
 ## to dump SHT_LLVM_LINKER_OPTIONS sections.
 
-# RUN: yaml2obj %s -o %t1
-# RUN: llvm-readobj --elf-linker-options %t1 | FileCheck %s
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --elf-linker-options %t1 2>&1 | FileCheck %s --check-prefix=CHECK -DFILE=%t1
 
-# CHECK:      LinkerOptions [
-# CHECK-NEXT:  option 0: value 0
-# CHECK-NEXT:  option 1: value 1
-# CHECK-NEXT: ]
+# CHECK:       LinkerOptions [
+# CHECK:         option 0: value 0
+# CHECK:         option 1: value 1
+# CHECK-EMPTY:
+# 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-EMPTY:
+# CHECK-NEXT:    warning: '[[FILE]]': SHT_LLVM_LINKER_OPTIONS section at index 4 is broken: the content is not null-terminated
+# CHECK-NEXT:    option 3: value 3
+# CHECK-NEXT:  ]
 
 --- !ELF
 FileHeader:
@@ -16,13 +21,37 @@ FileHeader:
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
-  - Name: .linker-options
+## Case 1: a correct case.
+  - Name: .linker-options.valid1
     Type: SHT_LLVM_LINKER_OPTIONS
     Options:
       - Name:  option 0
         Value: value 0
       - Name:  option 1
         Value: value 1
+## Case 2: check we do not attempt to dump data from outside the SHT_LLVM_LINKER_OPTIONS section
+##         when it contains an incomplete key-value pair.
+  - Name:    .linker-options.incomplete
+    Type:    SHT_LLVM_LINKER_OPTIONS
+    Content: "610062006300" ## 'a', '\0', 'b', '\0', 'c', '\0'
+  - Type:    Fill
+    Pattern: "FF"
+    Size:    "1"
+## 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.
+  - 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

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index b8078ff7db64..f6975bdb45e8 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -6013,21 +6013,41 @@ template <class ELFT>
 void LLVMStyle<ELFT>::printELFLinkerOptions(const ELFFile<ELFT> *Obj) {
   ListScope L(W, "LinkerOptions");
 
+  unsigned I = -1;
   for (const Elf_Shdr &Shdr : unwrapOrError(this->FileName, Obj->sections())) {
+    ++I;
     if (Shdr.sh_type != ELF::SHT_LLVM_LINKER_OPTIONS)
       continue;
 
     ArrayRef<uint8_t> Contents =
         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 (Contents.empty())
+      continue;
 
-      W.printString(Key, Value);
+    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);
+      continue;
+    }
 
-      P = P + Key.size() + Value.size() + 2;
+    SmallVector<StringRef, 16> Strings;
+    toStringRef(Contents.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);
+      continue;
     }
+
+    for (size_t I = 0; I < Strings.size(); I += 2)
+      W.printString(Strings[I], Strings[I + 1]);
   }
 }