[llvm-dwarfdump] Warn user when it encounters no null terminated strings.

[Higuoxing]

Summary

When llvm-dwarfdump encounters no null terminated strings, we should
warn user about it rather than ignore it and print nothing.

Before this patch, when llvm-dwarfdump dumps a .debug_str section whose
content is "abc", it prints:

.debug_str contents:

After this patch:

.debug_str contents:
warning: no null terminated string at offset 0x0

[MaskRay]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

533

operator bool sets the checked state if Error is in a success state.

So you can just run: (void)!Err

If you want to ensure an Error in a success state is also checked (not in this case), ErrorAsOutParameter ...(&Err)

[Higuoxing]

Address comment.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

533

Thanks. Actually I'm not sure about it. I didn't find anything about creating a checked error in LLVM Programmer's manual.

I usually do it using

Error Err = Error::success();
cantFail(std::move(Err));

Can we record it in programmer's manual?

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

533

Do you actually need this here? It seems to me a cantFail(std::move(Err)); after the loop might be more appropriate. The getCStr function handles the Error that gets passed in, so that it doesn't need checking beforehand.

llvm/test/tools/llvm-dwarfdump/debug-str.yaml

48

I think this reads better.

[Higuoxing]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

533

I'm not sure whether to put it after the loop or put it after Error::success() but I'm sure we need it. If we remove the cantFail(std::move(Err)) or (void)!Err, it will crash due to an unhandled error when parsing the following object file.

...
Sections:
  - Name: .debug_str
    Type: SHT_PROGBITS
    Size: 0

[Higuoxing]

Address comment.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

533

Sorry, I missed your point. We can remove the cantFail(std::move(Err)).

[jhenderson]

LGTM!

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

533

Actually that wasn't my point, but the solution is much better this way, thanks!

[MaskRay]

LGTM.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

while (!Parser.done()) {
                        DumpOpts.Verbose);
     }
   };
 
   auto DumpStrSection = [&](StringRef Section) {
     DataExtractor StrData(Section, isLittleEndian(), 0);
     uint64_t Offset = 0;
     uint64_t StrOffset = 0;
-    while (const char *CStr = StrData.getCStr(&Offset)) {
+    while (StrData.isValidOffset(Offset)) {
+      Error Err = Error::success();
+      const char *CStr = StrData.getCStr(&Offset, &Err);
+      if (Err) {
+        DumpOpts.WarningHandler(std::move(Err));
+        return;
+      }
       OS << format("0x%8.8" PRIx64 ": \"", StrOffset);
       OS.write_escaped(CStr);
       OS << "\"\n";
       StrOffset = Offset;
     }
   };
 
   if (const auto *Off = shouldDump(Explicit, ".debug_line", DIDT_ID_DebugLine,

llvm/test/tools/llvm-dwarfdump/debug-str.yaml

 #  ESCAPED-NEXT: 0x00000000: "\t"
 #  ESCAPED-NEXT: 0x00000002: "\001"
 #  ESCAPED-NEXT: 0x00000004: "\\001"
 #       ESCAPED: .debug_str.dwo contents:
 #  ESCAPED-NEXT: 0x00000000: "\t"
 #  ESCAPED-NEXT: 0x00000002: "\001"
 #  ESCAPED-NEXT: 0x00000004: "\\001"
 # ESCAPED-EMPTY:
+
+## c) Test that llvm-dwarfdump emits a warning when it encounters a string without a null terminator.
+
+##                        "abc\0"  "abc"
+# RUN: yaml2obj -DCONTENT="61626300616263" %s -o %t3.o
+# RUN: llvm-dwarfdump --debug-str %t3.o 2>&1 | FileCheck %s --check-prefix=WARN
+
+#      WARN: .debug_str contents:
+# WARN-NEXT: 0x00000000: "abc"
+# WARN-NEXT: warning: no null terminated string at offset 0x4
+#      WARN: .debug_str.dwo contents:
+# WARN-NEXT: 0x00000000: "abc"
+# WARN-NEXT: warning: no null terminated string at offset 0x4