[obj2yaml] Stop parsing the debug_str section when it encounters a string without the null terminator.

[Higuoxing]

Summary

When obj2yaml encounters a string without the null terminator, it should
stop parsing the debug_str section. This patch addresses comments in
D86867.

[Higuoxing]

Remove a header.

[jhenderson]

llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml

325

327

Might be worth an --implicit-check-not to show that a DWARF tag is not included (or a NO-TERMINATOR-NOT: DWARF before the Sections block).

llvm/test/tools/obj2yaml/ELF/DWARF/debug-str.yaml

103

May be worth extending this comment saying that it uses raw content instead of the DWARF tag in this situation. Also, see comment above re. checking there is no DWARF tag in the YAML output.

[Higuoxing]

Address review comments.

Thanks!

[jhenderson]

LGTM with comment update.

llvm/test/tools/obj2yaml/ELF/DWARF/debug-str.yaml

104–105

This is what I meant. Your comment suggested it was the input that was important, whereas I'm interested in explaining that it's the output that we're testing here.

[Higuoxing]

Update comment.

llvm/test/tools/obj2yaml/ELF/DWARF/debug-str.yaml

104–105

Thanks!

llvm/test/ObjectYAML/MachO/DWARF-debug_str.yaml

 ## c) Test generating and dumping a __debug_str section who only has an empty string.
 
 # RUN: yaml2obj --docnum=2 -DCONTENT='[ "" ]' -DSIZE=1 %s | obj2yaml | FileCheck %s --check-prefix=EMPTY-STRING
 
 #      EMPTY-STRING: DWARF:
 # EMPTY-STRING-NEXT:   debug_str:
 # EMPTY-STRING-NEXT:     - ''
 # EMPTY-STRING-NEXT: ...
+
+## d) Test generating and dumping a __debug_str section which contains a string without a null terminator.
+
+# RUN: yaml2obj --docnum=3 %s | obj2yaml | FileCheck %s --check-prefix=NO-TERMINATOR
+
+#  NO-TERMINATOR-NOT: DWARF:
+#      NO-TERMINATOR: Sections:
+# NO-TERMINATOR-NEXT:   - sectname:  __debug_str
+# NO-TERMINATOR-NEXT:     segname:   __DWARF
+# NO-TERMINATOR-NEXT:     addr:      0x0000000000000000
+# NO-TERMINATOR-NEXT:     size:      7
+# NO-TERMINATOR-NEXT:     offset:    0x00000210
+# NO-TERMINATOR-NEXT:     align:     0
+# NO-TERMINATOR-NEXT:     reloff:    0x00000000
+# NO-TERMINATOR-NEXT:     nreloc:    0
+# NO-TERMINATOR-NEXT:     flags:     0x00000000
+# NO-TERMINATOR-NEXT:     reserved1: 0x00000000
+# NO-TERMINATOR-NEXT:     reserved2: 0x00000000
+# NO-TERMINATOR-NEXT:     reserved3: 0x00000000
+# NO-TERMINATOR-NEXT:     content:   '61626300616263'
+# NO-TERMINATOR-NEXT: ...
+
+--- !mach-o
+FileHeader:
+  magic:      0xFEEDFACF
+  cputype:    0x01000007
+  cpusubtype: 0x00000003
+  filetype:   0x0000000A
+  ncmds:      1
+  sizeofcmds: 232
+  flags:      0x00000000
+  reserved:   0x00000000
+LoadCommands:
+  - cmd:      LC_SEGMENT_64
+    cmdsize:  152
+    segname:  __DWARF
+    vmaddr:   0x00
+    vmsize:   0x00
+    fileoff:  0x00
+    filesize: 0x00
+    maxprot:  0
+    initprot: 0
+    nsects:   1
+    flags:    0
+    Sections:
+      - sectname:  __debug_str
+        segname:   __DWARF
+        addr:      0x00
+        size:      7
+        offset:    0x210
+        align:     0
+        reloff:    0x00000000
+        nreloc:    0
+        flags:     0x00000000
+        reserved1: 0x00000000
+        reserved2: 0x00000000
+        reserved3: 0x00000000
+        content:   '61626300616263' ## "abc\0abc"

llvm/test/tools/obj2yaml/ELF/DWARF/debug-str.yaml

 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_EXEC
 DWARF:
   debug_str: []
+
+## d) Test that yaml2obj stops parsing the .debug_str section if it encounters a
+## string without a null terminator. The output uses a raw content section instead of
+## the DWARF tag to represent the broken .debug_str section.
+
+# RUN: yaml2obj --docnum=3 %s | obj2yaml | FileCheck %s --check-prefix=NO-TERMINATOR
+
+#  NO-TERMINATOR-NOT: DWARF:
+#      NO-TERMINATOR: Sections:
+# NO-TERMINATOR-NEXT:   - Name:    .debug_str
+# NO-TERMINATOR-NEXT:     Type:    SHT_PROGBITS
+# NO-TERMINATOR-NEXT:     Flags:   [ SHF_MERGE, SHF_STRINGS ]
+# NO-TERMINATOR-NEXT:     Content: '61626300616263'
+# NO-TERMINATOR-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:    .debug_str
+    Type:    SHT_PROGBITS
+    Content: "61626300616263" ## "abc\0abc"

llvm/tools/obj2yaml/dwarf2yaml.cpp

for (auto AbbrvDeclSet : *AbbrevSetPtr) {
         }
         Y.DebugAbbrev.back().ID = AbbrevTableID++;
         Y.DebugAbbrev.back().Table.push_back(Abbrv);
       }
     }
   }
 }
 
-void dumpDebugStrings(DWARFContext &DCtx, DWARFYAML::Data &Y) {
-  StringRef RemainingTable = DCtx.getDWARFObj().getStrSection();
-  Y.DebugStrings.emplace();
-  while (RemainingTable.size() > 0) {
-    auto SymbolPair = RemainingTable.split('\0');
-    RemainingTable = SymbolPair.second;
-    Y.DebugStrings->push_back(SymbolPair.first);
+Error dumpDebugStrings(DWARFContext &DCtx, DWARFYAML::Data &Y) {
+  DataExtractor StrData = DCtx.getStringExtractor();
+  uint64_t Offset = 0;
+  std::vector<StringRef> DebugStr;
+  Error Err = Error::success();
+  while (StrData.isValidOffset(Offset)) {
+    const char *CStr = StrData.getCStr(&Offset, &Err);
+    if (Err)
+      return Err;
+    DebugStr.push_back(CStr);
   }
+
+  Y.DebugStrings = DebugStr;
+  return Err;
 }
 
 Error dumpDebugARanges(DWARFContext &DCtx, DWARFYAML::Data &Y) {
   DWARFDataExtractor ArangesData(DCtx.getDWARFObj().getArangesSection(),
                                  DCtx.isLittleEndian(), 0);
   uint64_t Offset = 0;
   DWARFDebugArangeSet Set;
   std::vector<DWARFYAML::ARange> DebugAranges;

llvm/tools/obj2yaml/elf2yaml.cpp

for (std::unique_ptr<ELFYAML::Chunk> &C : Sections) {
     if (ELFYAML::RawContentSection *RawSec =
             dyn_cast<ELFYAML::RawContentSection>(C.get())) {
       Error Err = Error::success();
       cantFail(std::move(Err));
 
       if (RawSec->Name == ".debug_aranges")
         Err = dumpDebugARanges(*DWARFCtx.get(), DWARF);
       else if (RawSec->Name == ".debug_str")
-        dumpDebugStrings(*DWARFCtx.get(), DWARF);
+        Err = dumpDebugStrings(*DWARFCtx.get(), DWARF);
 
       // If the DWARF section cannot be successfully parsed, emit raw content
       // instead of an entry in the DWARF section of the YAML.
       if (Err)
         consumeError(std::move(Err));
       else
         RawSec->Content.reset();
     }

llvm/tools/obj2yaml/macho2yaml.cpp

static Error dumpDebugSection(StringRef SecName, DWARFContext &DCtx,
   }
   if (SecName.startswith("__debug_pub")) {
     // FIXME: We should extract pub-section dumpers from this function.
     dumpDebugPubSections(DCtx, DWARF);
     return Error::success();
   }
   if (SecName == "__debug_ranges")
     return dumpDebugRanges(DCtx, DWARF);
-  if (SecName == "__debug_str") {
-    dumpDebugStrings(DCtx, DWARF);
-    return Error::success();
-  }
+  if (SecName == "__debug_str")
+    return dumpDebugStrings(DCtx, DWARF);
   return createStringError(errc::not_supported,
                            "dumping " + SecName + " section is not supported");
 }
 
 template <typename SectionType, typename SegmentType>
 Expected<const char *> MachODumper::extractSections(
     const llvm::object::MachOObjectFile::LoadCommandInfo &LoadCmd,
     std::vector<MachOYAML::Section> &Sections, MachOYAML::Object &Y) {

llvm/tools/obj2yaml/obj2yaml.h

 
 void dumpDebugAbbrev(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
 llvm::Error dumpDebugARanges(llvm::DWARFContext &DCtx,
                              llvm::DWARFYAML::Data &Y);
 void dumpDebugPubSections(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
 void dumpDebugInfo(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
 void dumpDebugLines(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
 llvm::Error dumpDebugRanges(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
-void dumpDebugStrings(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
+llvm::Error dumpDebugStrings(llvm::DWARFContext &DCtx,
+                             llvm::DWARFYAML::Data &Y);
 
 #endif