[DebugInfo] Make most debug line prologue errors non-fatal to parsing

[jhenderson]

Summary

Many of the debug line prologue errors are not inherently fatal. In most cases, we can make reasonable assumptions and carry on. This patch does exactly that. In the case of length problems, the approach of "assume stated length is correct" is taken which means the offset might need adjusting.

Depends on D72157.

[jhenderson]

Early ping - I'd like to get this and the other related reviews in before the release branch is created if possible.

[dblaikie]

As mentioned in another review - I'm not sure "assume bigger is correct" is a great strategy & not sure there's a lot of value in continuing in the face of a conflict like that. What sort of situations really benefit from such parsing optimism?

[jhenderson]

In D72158#1813369, @dblaikie wrote:

As mentioned in another review - I'm not sure "assume bigger is correct" is a great strategy & not sure there's a lot of value in continuing in the face of a conflict like that. What sort of situations really benefit from such parsing optimism?

See my comments in D72155. Having discussed it with a colleague, I think going with "the length field is always right" is a better approach, where possible. The extra parsing could be useful to some users, I guess.

[jhenderson]

Rebased + changed to assuming the stated length is always correct. This required some additional test changes to better demonstrate the behaviour difference.

[jhenderson]

Ping!

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

463–477

Note for future work: This seems fairly convoluted in terms of how to iterate over the section, parsing the contributions & handling the errors.

Having the explicit "skip" operation might be nice to avoid - and there's always recurring discussions about how to make this sort of parsing lazy so that users can get as much or as little data as they need without parsing things they don't need (so if the API were made more lazy generally - then "skip" would just be "parseLazily" then request the contribution length - if that doesn't error out, jump that many bytes ahead and parse the next one) - this would also move to fewer callback based errors, and more errors propagated on specific API interactions (eg: if you ask for the length, you return an ErrorOr<Length>, etc). Not sure if it's better or worse, but it's an option to consider.

Where does this code capture hard failures? Ah, in "Parser.done()"? So any hard failure (eg: failure to parse the length at all) results in the Parser moving to state "done" (& producing an error through one of the callbacks?)?

& should one or both of the "dumpWarning"s be errors? Recoverable failures (ultimately in the worst case "we could parse the length, but nothing else makes any sense" is a recoverable error) aren't necessarily warnings. Take Clang's behavior - lots of hard errors are recoverable (you missed a semicolon, we're going to keep going assuming you meant to have a semicolon there - but we aren't going to compile this code, it's still an error)

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

329–331

For this and other comments - probably not important/might be distracting to say what these would be parsed as rather than just to say that they're garbage data within the encoded length of the extended opcode, but beyond the parsed representation of that opcode? (or somtehing to that effect)

380–382

Not sure I understand what's happening here - again, perhaps you're describing two different ways this could be parsed? I'd focus on the way it is being parsed & not the ambiguity of other ways it could be parsed/error-recovered. (you can mention what assumptions about previous invalid data are made to form that particular parse)

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

463–477

Thanks for the comments!

Where does this code capture hard failures? Ah, in "Parser.done()"? So any hard failure (eg: failure to parse the length at all) results in the Parser moving to state "done" (& producing an error through one of the callbacks?)?

Right. The hard failures are primarily there to handle the cases where we really don't know where to continue from because you gave it data that doesn't really look like a line table. Since the parser would then be in a bad state, trying to get the next unit doesn't really make sense, so we just say we're done.

& should one or both of the "dumpWarning"s be errors? Recoverable failures (ultimately in the worst case "we could parse the length, but nothing else makes any sense" is a recoverable error) aren't necessarily warnings.

It's difficult to say, because it depends entirely on the client. Ideally, the tool using this library would configure the situation for its own needs. Always warning may well not be appropriate for some clients, but should it really be an error if e.g. lldb can't parse some of the debug data? I think the main client here is actually llvm-dwarfdump, where it could be argued that lots of things are really errors, but we shouldn't stop. But should the dumper exit with code 1 if the format is malformed? I don't know!

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

329–331

Fair enough. In some cases, the extra context is there to explain why we check what we do, but I'm okay changing it.

380–382

Actually, in this context, the header is being read twice, once actually as the header, and once again as part of the body, so indeed I am describing two different ways this is parsed. The change in behaviour is to continue reading from the end of the header, as claimed in the header length field, which means these parts are both part of the file table, and also the start of a line table sequence.

The comments are intended to help people (including myself) trying to match up what the data in the body is doing versus the checks in the file. I could format them differently as a table perhaps, to make it clearer what they represent on each parse?

[jhenderson]

Improve test comments.

[jhenderson]

@dblaikie, I'm going to hold off pushing this for a day or so, in case you've got any more comments about my latest comment update.

[jhenderson]

This broke LLDB's build, due to the interface change of Prologue::parse, as well as breaking an LLD test (I don't know why the LLD test failure didn't show up locally until after I committed...). I've reverted for the time being.

I'm not an LLDB developer (and don't have LLDB builds enabled), so I'm not really sure what would be the right way to fix this. Can anybody (@JDevlieghere?) help out here? My thinking is to take the code that currently handles parse errors and to pass that in as the callback too, as this best preserves the current behaviour. On the other hand, a prologue was parsed, at least partly successfully, so maybe it shouldn't be treated as an error?

[jhenderson]

Fix LLD test + fix LLDB build.

I'm uncertain if the LLDB fix is the right fix to make or not, but it does at least stop this change breaking the build.

[jhenderson]

Could somebody please look at the LLDB change?

[labath]

If I understand this correctly, this will cause lldb to continue to read the parsed line table contribution after encountering some errors in the prologue, whereas previously we would stop straight away. That sounds reasonable if now or in the future we will be able to get some useful information (at least some subset of file names, or line numbers without file names, etc.) out of these kinds of line tables. If that is not the case, and these errors are recoverable only in the sense that they allow you to jump to the next contribution, then it might be better to treat these errors as unrecoverable for lldb purposes (jumping to the next contribution is not interesting for us since we always use DW_AT_stmt_list to locate the line table).

However, I don't think that resolving that needs to hold this patch up, as this behavior can be easily adjusted from within lldb.

[jhenderson]

In D72158#1844534, @labath wrote:

If I understand this correctly, this will cause lldb to continue to read the parsed line table contribution after encountering some errors in the prologue, whereas previously we would stop straight away. That sounds reasonable if now or in the future we will be able to get some useful information (at least some subset of file names, or line numbers without file names, etc.) out of these kinds of line tables.

Indeed, that's what this change allows: the parser will continue parsing after reporting the Errors via the new callback. In cases where it can't (i.e. unsupported versions or reserved unit length values), it will stop and return the Error (as it previously did, and also did for the now-recoverable Errors). For now I've changed LLDB to record both kinds of Errors in the same way as they were before, but the recoverable errors do not prevent it subsequently calling ParseSupportFilesFromPrologue. I could just as easily change it to doing what it always did for all cases, namely log the errors and not call ParseSupportFilesFromPrologue. That's probably the safer approach on further reflection, so I'll update the patch tomorrow (I'm just about to leave the office for the day), unless someone thinks changing the behaviour is good.

[labath]

Actually I think this is fine. We would want to squeeze as much information as possible from these kinds of line tables.

I don't think fully preserving the existing behavior would be that easy, actually. We have another call to the llvm line table parser in ParseLLVMLineTable (line 154), but this one calls DWARFDebugLine::LineTable::getOrParseLineTable There, we already ignore (log) recoverable errors, and it'd be hard to tell the "new" kinds of errors from the "old" ones...

[jhenderson]

Thanks for the review comments! I'll go ahead and land it like this, assuming my local test run passes.

[labath]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

323–325

BTW, I think this error should be recoverable too. I believe the reason why the length field comes *before* the version number is specifically so that one can skip over contributions with unsupported (future) version numbers.

While it's hard to say what the future versions of dwarf will look like, I would expect that the committee will try very hard to avoid making changes in the length field. I think they'd use one of the DW_LENGTH_lo_reserved..DW_LENGTH_hi_reserved-1 constants for severely incompatible changes.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

323–325

"Unrecoverable" here means don't try to parse this table, but do allow parsing the next. I think the comment might be slightly misleading in this regard. FWIW, a version of 0 or 1 probably doesn't have a leading length, so it is definitely unrecoverable. For versions > 5, which are now checked, we don't know what the structure of the header is, so although we could take a guess, we'd almost certainly get it wrong and produce invalid (possibly very invalid) output. I don't have a strong opinion as to whether that should be an unrecoverable error or not (currently it is).

[labath]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

323–325

Ah, right, I see now what you mean. I agree it makes no sense to parse the contents of a contribution with an unrecognized version. The part about not being able to trust the length field threw me off, as the length of the contribution is the one thing we can expect to remain unchanged between dwarf versions.

Sorry about the false alarm.

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

struct Prologue {
     getFileNameByIndex(uint64_t FileIndex, StringRef CompDir,
                        DILineInfoSpecifier::FileLineInfoKind Kind,
                        std::string &Result,
                        sys::path::Style Style = sys::path::Style::native) const;
 
     void clear();
     void dump(raw_ostream &OS, DIDumpOptions DumpOptions) const;
     Error parse(const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr,
+                function_ref<void(Error)> RecoverableErrorCallback,
                 const DWARFContext &Ctx, const DWARFUnit *U = nullptr);
   };
 
   /// Standard .debug_line state machine structure.
   struct Row {
     explicit Row(bool DefaultIsStmt = false);
 
     /// Called after a row is appended to the matrix.
public:
     parseNext(
         function_ref<void(Error)> RecoverableErrorCallback,
         function_ref<void(Error)> UnrecoverableErrorCallback,
         raw_ostream *OS = nullptr);
 
     /// Skip the current line table and go to the following line table (if
     /// present) immediately.
     ///
-    /// \param ErrorCallback - report any prologue parsing issues via this
-    /// callback.
-    void skip(function_ref<void(Error)> ErrorCallback);
+    /// \param RecoverableErrorCallback - report any recoverable prologue
+    /// parsing issues via this callback.
+    /// \param UnrecoverableErrorCallback - report any unrecoverable prologue
+    /// parsing issues via this callback.
+    void skip(function_ref<void(Error)> RecoverableErrorCallback,
+              function_ref<void(Error)> UnrecoverableErrorCallback);
 
     /// Indicates if the parser has parsed as much as possible.
     ///
     /// \note Certain problems with the line table structure might mean that
     /// parsing stops before the end of the section is reached.
     bool done() const { return Done; }
 
     /// Get the offset the parser has reached.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

if (shouldDump(Explicit, ".debug_aranges", DIDT_ID_DebugAranges,
     DWARFDebugArangeSet set;
     while (set.extract(arangesData, &offset))
       set.dump(OS);
   }
 
   auto DumpLineSection = [&](DWARFDebugLine::SectionParser Parser,
                              DIDumpOptions DumpOpts,
                              Optional<uint64_t> DumpOffset) {
     while (!Parser.done()) {
       if (DumpOffset && Parser.getOffset() != *DumpOffset) {
-        Parser.skip(dumpWarning);
+        Parser.skip(dumpWarning, dumpWarning);
         continue;
       }
       OS << "debug_line[" << format("0x%8.8" PRIx64, Parser.getOffset())
          << "]\n";
       if (DumpOpts.Verbose) {
         Parser.parseNext(dumpWarning, dumpWarning, &OS);
       } else {
         DWARFDebugLine::LineTable LineTable =
             Parser.parseNext(dumpWarning, dumpWarning);
         LineTable.dump(OS, DumpOpts);
       }
     }
   };
 
   if (const auto *Off = shouldDump(Explicit, ".debug_line", DIDT_ID_DebugLine,
                                    DObj->getLineSection().Data)) {
     DWARFDataExtractor LineData(*DObj, DObj->getLineSection(), isLittleEndian(),
                                 0);
     DWARFDebugLine::SectionParser Parser(LineData, *this, compile_units(),
                                          type_units());

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

for (auto Descriptor : *FileDescriptors) {
         break;
       }
     }
     FileNames.push_back(FileEntry);
   }
   return Error::success();
 }
 
-Error DWARFDebugLine::Prologue::parse(const DWARFDataExtractor &DebugLineData,
-                                      uint64_t *OffsetPtr,
-                                      const DWARFContext &Ctx,
-                                      const DWARFUnit *U) {
+Error DWARFDebugLine::Prologue::parse(
+    const DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr,
+    function_ref<void(Error)> RecoverableErrorCallback, const DWARFContext &Ctx,
   const uint64_t PrologueOffset = *OffsetPtr;
 
   clear();
   TotalLength = DebugLineData.getRelocatedValue(4, OffsetPtr);
   if (TotalLength == dwarf::DW_LENGTH_DWARF64) {
     FormParams.Format = dwarf::DWARF64;
     TotalLength = DebugLineData.getU64(OffsetPtr);
   } else if (TotalLength >= dwarf::DW_LENGTH_lo_reserved) {
+    // Treat this error as unrecoverable - we have no way of knowing where the
+    // table ends.
     return createStringError(errc::invalid_argument,
         "parsing line table prologue at offset 0x%8.8" PRIx64
         " unsupported reserved unit length found of value 0x%8.8" PRIx64,
         PrologueOffset, TotalLength);
   }
   FormParams.Version = DebugLineData.getU16(OffsetPtr);
   if (getVersion() < 2)
+    // Treat this error as unrecoverable - we cannot be sure what any of
+    // the data represents including the length field, so cannot skip it or make
+    // any reasonable assumptions.
     return createStringError(errc::not_supported,
                        "parsing line table prologue at offset 0x%8.8" PRIx64
                        " found unsupported version 0x%2.2" PRIx16,
                        PrologueOffset, getVersion());
 
   if (getVersion() >= 5) {
     FormParams.AddrSize = DebugLineData.getU8(OffsetPtr);
     assert((DebugLineData.getAddressSize() == 0 ||
for (uint32_t I = 1; I < OpcodeBase; ++I) {
     uint8_t OpLen = DebugLineData.getU8(OffsetPtr);
     StandardOpcodeLengths.push_back(OpLen);
   }
 
   if (getVersion() >= 5) {
     if (Error e = parseV5DirFileTables(
             DebugLineData, OffsetPtr, EndPrologueOffset, FormParams, Ctx, U,
             ContentTypes, IncludeDirectories, FileNames)) {
-      return joinErrors(
+      RecoverableErrorCallback(joinErrors(
           createStringError(
               errc::invalid_argument,
               "parsing line table prologue at 0x%8.8" PRIx64
               " found an invalid directory or file table description at"
               " 0x%8.8" PRIx64,
               PrologueOffset, *OffsetPtr),
-          std::move(e));
+          std::move(e)));
+      // Skip to the end of the prologue, since the chances are that the parser
+      // did not read the whole table. This prevents the length check below from
+      // executing.
+      if (*OffsetPtr < EndPrologueOffset)
+        *OffsetPtr = EndPrologueOffset;
     }
   } else
     parseV2DirFileTables(DebugLineData, OffsetPtr, EndPrologueOffset,
                          ContentTypes, IncludeDirectories, FileNames);
 
   if (*OffsetPtr != EndPrologueOffset)
-    return createStringError(errc::invalid_argument,
-                       "parsing line table prologue at 0x%8.8" PRIx64
-                       " should have ended at 0x%8.8" PRIx64
-                       " but it ended at 0x%8.8" PRIx64,
-                       PrologueOffset, EndPrologueOffset, *OffsetPtr);
+    RecoverableErrorCallback(createStringError(
+        errc::invalid_argument,
+        " should have ended at 0x%8.8" PRIx64 " but it ended at 0x%8.8" PRIx64,
+        PrologueOffset, EndPrologueOffset, *OffsetPtr));
+  if (*OffsetPtr < EndPrologueOffset)
+    *OffsetPtr = EndPrologueOffset;
   return Error::success();
 }
 
 DWARFDebugLine::Row::Row(bool DefaultIsStmt) { reset(DefaultIsStmt); }
 
 void DWARFDebugLine::Row::postAppend() {
   Discriminator = 0;
   BasicBlock = false;

 Error DWARFDebugLine::LineTable::parse(
     DWARFDataExtractor &DebugLineData, uint64_t *OffsetPtr,
     const DWARFContext &Ctx, const DWARFUnit *U,
     function_ref<void(Error)> RecoverableErrorCallback, raw_ostream *OS) {
   const uint64_t DebugLineOffset = *OffsetPtr;
 
   clear();
 
-  Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr, Ctx, U);
+  Error PrologueErr = Prologue.parse(DebugLineData, OffsetPtr,
+                                     RecoverableErrorCallback, Ctx, U);
 
   if (OS) {
     // The presence of OS signals verbose dumping.
     DIDumpOptions DumpOptions;
     DumpOptions.Verbose = true;
     Prologue.dump(*OS, DumpOptions);
   }
 
DWARFDebugLine::LineTable DWARFDebugLine::SectionParser::parseNext(
   if (Error Err = LT.parse(DebugLineData, &Offset, Context, U,
                            RecoverableErrorCallback, OS))
     UnrecoverableErrorCallback(std::move(Err));
   moveToNextTable(OldOffset, LT.Prologue);
   return LT;
 }
 
 void DWARFDebugLine::SectionParser::skip(
-    function_ref<void(Error)> ErrorCallback) {
+    function_ref<void(Error)> RecoverableErrorCallback,
+    function_ref<void(Error)> UnrecoverableErrorCallback) {
   assert(DebugLineData.isValidOffset(Offset) &&
          "parsing should have terminated");
   DWARFUnit *U = prepareToParse(Offset);
   uint64_t OldOffset = Offset;
   LineTable LT;
-  if (Error Err = LT.Prologue.parse(DebugLineData, &Offset, Context, U))
-    ErrorCallback(std::move(Err));
+  if (Error Err = LT.Prologue.parse(DebugLineData, &Offset,
+                                    RecoverableErrorCallback, Context, U))
+    UnrecoverableErrorCallback(std::move(Err));
   moveToNextTable(OldOffset, LT.Prologue);
 }
 
 DWARFUnit *DWARFDebugLine::SectionParser::prepareToParse(uint64_t Offset) {
   DWARFUnit *U = nullptr;
   auto It = LineToUnit.find(Offset);
   if (It != LineToUnit.end())
     U = It->second;

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

 .quad   0x8877665544332211
 .byte   0, 1, 1         # DW_LNE_end_sequence
 .Lunit_v5_end:
 
 # Short prologue.
 .long   .Lunit_short_prologue_end - .Lunit_short_prologue_start # unit length
 .Lunit_short_prologue_start:
 .short  4               # version
-.long   .Lprologue_short_prologue_end-.Lprologue_short_prologue_start - 2 # Length of Prologue
+.long   .Lprologue_short_prologue_end-.Lprologue_short_prologue_start - 1 # Length of Prologue
 .Lprologue_short_prologue_start:
 .byte   1               # Minimum Instruction Length
 .byte   1               # Maximum Operations per Instruction
 .byte   1               # Default is_stmt
 .byte   -5              # Line Base
 .byte   14              # Line Range
 .byte   13              # Opcode Base
 .byte   0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 # Standard Opcode Lengths
 .asciz "dir1"           # Include table
 .asciz "dir2"
 .byte   0
 .asciz "file1"          # File table
 .byte   0, 0, 0
 .asciz "file2"
 .byte   1, 2, 3
-.byte   0
+# FIXME: There should be an additional 0 byte here, but the file name parsing
+#        code does not recognise a missing null terminator.
 .Lprologue_short_prologue_end:
 .byte   0, 9, 2         # DW_LNE_set_address
 .quad   0x1122334455667788
 .byte   0, 1, 1         # DW_LNE_end_sequence
 .Lunit_short_prologue_end:
 
 # Over-long prologue.
 .long   .Lunit_long_prologue_end - .Lunit_long_prologue_start # unit length
 .Lunit_long_prologue_start:
 .short  4               # version
-.long   .Lprologue_long_prologue_end-.Lprologue_long_prologue_start + 1 # Length of Prologue
+.long   .Lprologue_long_prologue_end-.Lprologue_long_prologue_start # Length of Prologue
 .Lprologue_long_prologue_start:
 .byte   1               # Minimum Instruction Length
 .byte   1               # Maximum Operations per Instruction
 .byte   1               # Default is_stmt
 .byte   -5              # Line Base
 .byte   14              # Line Range
 .byte   13              # Opcode Base
 .byte   0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 # Standard Opcode Lengths
 .asciz "dir1"           # Include table
 .asciz "dir2"
 .byte   0
 .asciz "file1"          # File table
 .byte   0, 0, 0
 .asciz "file2"
 .byte   1, 2, 3
 .byte   0
+# Skipped byte (treated as part of prologue). Would be DW_LNS_negate_stmt.
+.byte   6
 .Lprologue_long_prologue_end:
 .byte   0, 9, 2        # DW_LNE_set_address
 .quad   0x1111222233334444
 .byte   0, 1, 1        # DW_LNE_end_sequence
 .Lunit_long_prologue_end:
 
 # Over-long extended opcode.
 .long   .Lunit_long_opcode_end - .Lunit_long_opcode_start # unit length

 .byte   0x0b            # DW_FORM_data1
 .byte   2               # DW_LNCT_directory_index
 .byte   0x0b            # DW_FORM_data1
 # File table entries
 .byte   1               # 1 file
 .asciz  "a.c"
 .byte   0
 # Data to show that the rest of the prologue is skipped.
-.byte   6
+.byte   6               # Would be interpreted as DW_LNS_negate_stmt if parsed
+                        # as part of program.
 .Linvalid_md5_header_end0:
 .byte   0, 9, 2         # DW_LNE_set_address
 .quad   0x1234123412341234
 .byte   0, 1, 1         # DW_LNE_end_sequence
 .Linvalid_md5_end0:
 
 # Invalid MD5 hash, when data beyond the prologue length has
 # been read before the MD5 problem is identified.
 .long   .Linvalid_md5_end1-.Linvalid_md5_start1   # Length of Unit

 .byte   0x08            # DW_FORM_string
 .byte   5               # DW_LNCT_MD5
 .byte   0x0b            # DW_FORM_data1
 .byte   2               # DW_LNCT_directory_index
 .byte   0x0b            # DW_FORM_data1
 # File table entries
 .byte   1               # 1 file
 .asciz  "a.c"
-.byte   6               # This byte will be consumed when reading the MD5 value.
-.byte   0xb             # This byte will not be read as part of the prologue.
+.byte   6               # This byte will be consumed when reading the MD5 value and
+                        # not interpreted as DW_LNS_negate_stmt in the program.
+.byte   0xb             # This byte will not be read as part of the prologue, and
+                        # will be treated as part of the line table
+                        # (DW_LNS_set_epilogue_begin) instead.
 .Linvalid_md5_header_end1:
 .byte   0, 9, 2         # DW_LNE_set_address
 .quad   0x4321432143214321
 .byte   0, 1, 1         # DW_LNE_end_sequence
 .Linvalid_md5_end1:
 
 # Trailing good section.
 .long   .Lunit_good_end - .Lunit_good_start # Length of Unit (DWARF-32 format)
 .Lunit_good_start:
 .short  4               # DWARF version number
 .long   .Lprologue_good_end-.Lprologue_good_start # Length of Prologue
 .Lprologue_good_start:
 .byte   1               # Minimum Instruction Length
 .byte   1               # Maximum Operations per Instruction
 .byte   1               # Default is_stmt

llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test

 # NOLATER-NOT: end_sequence
 
 ## For fatal issues, the following table(s) should not be dumped.
 # FATAL:      debug_line[0x00000048]
 # FATAL-NEXT: Line table prologue
 # FATAL-NEXT: total_length: 0xfffffffe
 # FATAL-NOT:  debug_line
 
-## For non-fatal prologue issues, the table prologue should be dumped, and any
-## subsequent tables should also be.
+## For non-fatal issues, the table data should be dumped.
 ## Case 1: Version 0 table.
 # NONFATAL:      debug_line[0x00000048]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL-NOT:  Address
 
 ## Case 2: Version 1 table.
 # NONFATAL:      debug_line[0x0000004e]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL-NOT:  Address
 
 ## Case 3: Malformed directory format with no path component.
 # NONFATAL:      debug_line[0x00000054]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL-NOT:  include_directories
 # NONFATAL-NOT:  file_names
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x8877665544332211 {{.*}} end_sequence
 
 ## Case 4: Prologue with length shorter than parsed.
 # NONFATAL:      debug_line[0x00000081]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      file_names[  2]:
 # NONFATAL-NEXT:            name: "file2"
 # NONFATAL-NEXT:       dir_index: 1
 # NONFATAL-NEXT:        mod_time: 0x00000002
 # NONFATAL-NEXT:          length: 0x00000003
-# NONFATAL-NOT:  file_names
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x1122334455667788 {{.*}} end_sequence
 
 ## Case 5: Prologue with length longer than parsed.
-# NONFATAL:      debug_line[0x000000c9]
+# NONFATAL:      debug_line[0x000000c8]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      file_names[  2]:
 # NONFATAL-NEXT:            name: "file2"
 # NONFATAL-NEXT:       dir_index: 1
 # NONFATAL-NEXT:        mod_time: 0x00000002
 # NONFATAL-NEXT:          length: 0x00000003
 # NONFATAL-NOT:  file_names
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x1111222233334444 {{.*}} is_stmt end_sequence
 
 ## Case 6: Extended opcode with incorrect length versus expected.
 # NONFATAL:      debug_line[0x00000111]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL: 0x00000000abbadaba {{.*}} end_sequence
 # NONFATAL: 0x00000000babb1e45 {{.*}} 10 is_stmt end_sequence{{$}}
 
 ## Case 7: No end of sequence.
 # NONFATAL:      debug_line[0x0000016c]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      0x00000000deadfade {{.*}} is_stmt
 # NONFATAL-NOT:  end_sequence
 
 ## Case 8: Very short prologue length for V5 (ends during parameters).
 # NONFATAL:      debug_line[0x000001b2]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      standard_opcode_lengths[DW_LNS_set_isa] = 1
 # NONFATAL-NEXT: include_directories[  0] = "/tmp"
 # NONFATAL-NEXT: file_names[  0]:
 # NONFATAL-NEXT:            name: "a.c"
 # NONFATAL-NEXT:       dir_index: 1
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x0000babb1ebabb1e {{.*}} end_sequence
 
 ## Case 9: V5 prologue ends during file table.
 # NONFATAL:      debug_line[0x000001f2]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      include_directories[  0] = "/tmp"
 # NONFATAL-NEXT: file_names[  0]:
 # NONFATAL-NEXT:            name: "a.c"
 # NONFATAL-NEXT:       dir_index: 1
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x00000ab4acadab4a {{.*}} end_sequence
 
 ## Case 10: V5 prologue ends during directory table.
 # NONFATAL:      debug_line[0x00000232]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      include_directories[  0] = "/tmp"
 # NONFATAL-NEXT: file_names[  0]:
 # NONFATAL-NEXT:            name: "a.c"
 # NONFATAL-NEXT:       dir_index: 1
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x4444333322221111 {{.*}} end_sequence
 
 ## Case 11: V5 invalid MD5 hash form when there is still data to be read.
 # NONFATAL:      debug_line[0x00000272]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      include_directories[  0] = "/tmp"
 # NONFATAL-NOT:  file_names
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x1234123412341234 {{.*}} is_stmt end_sequence
 
 ## Case 12: V5 invalid MD5 hash form when data beyond the prologue length has
 ## been read before the MD5 problem is identified.
 # NONFATAL: debug_line[0x000002b5]
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      include_directories[  0] = "/tmp"
 # NONFATAL-NOT:  file_names
-# NONFATAL-NOT:  Address
+# NONFATAL:      0x4321432143214321 {{.*}} is_stmt epilogue_begin end_sequence
 
 # LAST:          debug_line[0x000002f8]
 # LAST:          0x00000000cafebabe {{.*}} end_sequence
 
 # RESERVED: warning: parsing line table prologue at offset 0x00000048 unsupported reserved unit length found of value 0xfffffffe
 
 # ALL-NOT:  warning:
 # ALL:      warning: parsing line table prologue at offset 0x00000048 found unsupported version 0x00
 # ALL-NEXT: warning: parsing line table prologue at offset 0x0000004e found unsupported version 0x01
 # ALL-NEXT: warning: parsing line table prologue at 0x00000054 found an invalid directory or file table description at 0x00000073
 # ALL-NEXT: warning: failed to parse entry content descriptions because no path was found
-# FIXME - The latter offset in the next line should be 0xad. The filename parsing code does not notice a missing terminating byte.
 # ALL-NEXT: warning: parsing line table prologue at 0x00000081 should have ended at 0x000000b9 but it ended at 0x000000ba
-# ALL-NEXT: warning: parsing line table prologue at 0x000000c9 should have ended at 0x00000104 but it ended at 0x00000103
+# ALL-NEXT: warning: parsing line table prologue at 0x000000c8 should have ended at 0x00000103 but it ended at 0x00000102
 # OTHER-NEXT: warning: unexpected line op length at offset 0x00000158 expected 0x02 found 0x01
 # OTHER-NEXT: warning: unexpected line op length at offset 0x0000015c expected 0x01 found 0x02
 # OTHER-NEXT: warning: last sequence in debug line table is not terminated!
 # ALL-NEXT: warning: parsing line table prologue at 0x000001b2 should have ended at 0x000001cd but it ended at 0x000001e4
 # ALL-NEXT: warning: parsing line table prologue at 0x000001f2 should have ended at 0x0000021d but it ended at 0x00000224
 # ALL-NEXT: warning: parsing line table prologue at 0x00000232 should have ended at 0x00000254 but it ended at 0x00000264
 # ALL-NEXT: warning: parsing line table prologue at 0x00000272 found an invalid directory or file table description at 0x000002a6
 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid
 # ALL-NEXT: warning: parsing line table prologue at 0x000002b5 found an invalid directory or file table description at 0x000002e9
 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid
+# ALL-NEXT: warning: parsing line table prologue at 0x000002b5 should have ended at 0x000002e0 but it ended at 0x000002e9
 # ALL-NOT:  warning:

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

LT.setCustomPrologue({
                              // zero).
       {0, LineTable::ULEB},  // directories count
       {0, LineTable::Byte},  // file name entry format count
       {0, LineTable::ULEB}   // file name entry count
   });
 
   generate();
 
-  checkGetOrParseLineTableEmitsFatalError(
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  EXPECT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+
+  checkError(
       {"parsing line table prologue at 0x00000000 found an invalid directory "
        "or file table description at 0x00000014",
-       "failed to parse entry content descriptions because no path was found"});
+       "failed to parse entry content descriptions because no path was found"},
+      std::move(Recoverable));
 }
 
 TEST_P(DebugLineParameterisedFixture, ErrorForTooLargePrologueLength) {
   if (!setupGenerator(Version))
     return;
 
   SCOPED_TRACE("Checking Version " + std::to_string(Version) + ", Format " +
                (Format == DWARF64 ? "DWARF64" : "DWARF32"));
 
   LineTable &LT = Gen->addLineTable(Format);
   DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
   ++Prologue.PrologueLength;
   LT.setPrologue(Prologue);
 
   generate();
 
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+  DWARFDebugLine::LineTable Result(**ExpectedLineTable);
+  // Undo the earlier modification so that it can be compared against a
+  // "default" prologue.
+  --Result.Prologue.PrologueLength;
+  checkDefaultPrologue(Version, Format, Result.Prologue, 0);
+
   uint64_t ExpectedEnd =
       Prologue.TotalLength + 1 + Prologue.sizeofTotalLength();
-  checkGetOrParseLineTableEmitsFatalError(
+  checkError(
       (Twine("parsing line table prologue at 0x00000000 should have ended at "
              "0x000000") +
        Twine::utohexstr(ExpectedEnd) + " but it ended at 0x000000" +
        Twine::utohexstr(ExpectedEnd - 1))
-          .str());
+          .str(),
+      std::move(Recoverable));
 }
 
 TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) {
   if (!setupGenerator(Version))
     return;
 
   SCOPED_TRACE("Checking Version " + std::to_string(Version) + ", Format " +
                (Format == DWARF64 ? "DWARF64" : "DWARF32"));
 
   LineTable &LT = Gen->addLineTable(Format);
   DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
   // FIXME: Ideally, we'd test for 1 less than expected, but the code does not
   // currently fail if missing only the terminator of a v2-4 file table.
   if (Version < 5)
     Prologue.PrologueLength -= 2;
   else
     Prologue.PrologueLength -= 1;
   LT.setPrologue(Prologue);
 
   generate();
 
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+  DWARFDebugLine::LineTable Result(**ExpectedLineTable);
+  // Undo the earlier modification so that it can be compared against a
+  // "default" prologue.
+  if (Version < 5)
+    Result.Prologue.PrologueLength += 2;
+  else
+    Result.Prologue.PrologueLength += 1;
+  checkDefaultPrologue(Version, Format, Result.Prologue, 0);
+
   uint64_t ExpectedEnd =
       Prologue.TotalLength - 1 + Prologue.sizeofTotalLength();
   if (Version < 5)
     --ExpectedEnd;
-  checkGetOrParseLineTableEmitsFatalError(
+  checkError(
       (Twine("parsing line table prologue at 0x00000000 should have ended at "
              "0x000000") +
        Twine::utohexstr(ExpectedEnd) + " but it ended at 0x000000" +
        Twine::utohexstr(ExpectedEnd + 1))
-          .str());
+          .str(),
+      std::move(Recoverable));
 }
 
 INSTANTIATE_TEST_CASE_P(
     LineTableTestParams, DebugLineParameterisedFixture,
     Values(std::make_pair(
                2, DWARF32), // Test lower-bound of v2-3 fields and DWARF32.
            std::make_pair(3, DWARF32), // Test upper-bound of v2-3 fields.
            std::make_pair(4, DWARF64), // Test v4 fields and DWARF64.
TEST_F(DebugLineBasicFixture, ParserSkipsCorrectly) {
   if (!setupGenerator())
     return;
 
   DWARFDebugLine::SectionParser Parser = setupParser();
 
   EXPECT_EQ(Parser.getOffset(), 0u);
   ASSERT_FALSE(Parser.done());
 
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
   EXPECT_EQ(Parser.getOffset(), 62u);
   ASSERT_FALSE(Parser.done());
 
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
   EXPECT_EQ(Parser.getOffset(), 136u);
   EXPECT_TRUE(Parser.done());
 
+  EXPECT_FALSE(Recoverable);
   EXPECT_FALSE(Unrecoverable);
 }
 
 TEST_F(DebugLineBasicFixture, ParserAlwaysDoneForEmptySection) {
   if (!setupGenerator())
     return;
 
   generate();
if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable();
   LT.setCustomPrologue({{0xfffffff0, LineTable::Long}});
   Gen->addLineTable();
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_EQ(Parser.getOffset(), 4u);
   EXPECT_TRUE(Parser.done());
+  EXPECT_FALSE(Recoverable);
 
   checkError("parsing line table prologue at offset 0x00000000 unsupported "
              "reserved unit length found of value 0xfffffff0",
              std::move(Unrecoverable));
 }
 
 TEST_F(DebugLineBasicFixture, ParserReportsFirstErrorInEachTableWhenParsing) {
   if (!setupGenerator())
TEST_F(DebugLineBasicFixture,
 
   LineTable &LT = Gen->addLineTable(DWARF32);
   LT.setCustomPrologue({{2, LineTable::Long}, {0, LineTable::Half}});
   LineTable &LT2 = Gen->addLineTable(DWARF32);
   LT2.setCustomPrologue({{2, LineTable::Long}, {1, LineTable::Half}});
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
   ASSERT_FALSE(Parser.done());
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_TRUE(Parser.done());
+  EXPECT_FALSE(Recoverable);
 
   checkError({"parsing line table prologue at offset 0x00000000 found "
               "unsupported version 0x00",
               "parsing line table prologue at offset 0x00000006 found "
               "unsupported version 0x01"},
              std::move(Unrecoverable));
 }
 
 TEST_F(DebugLineBasicFixture, ParserIgnoresNonPrologueErrorsWhenSkipping) {
   if (!setupGenerator())
     return;
 
   LineTable &LT = Gen->addLineTable(DWARF32);
   LT.addExtendedOpcode(42, DW_LNE_end_sequence, {});
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_TRUE(Parser.done());
+  EXPECT_FALSE(Recoverable);
   EXPECT_FALSE(Unrecoverable);
 }
 
 TEST_F(DebugLineBasicFixture, ParserPrintsStandardOpcodesWhenRequested) {
   if (!setupGenerator())
     return;
 
   using ValLen = dwarfgen::LineTable::ValueAndLength;