[PATCH] D75116: [DWARFDebugLine] Use new DWARFDataExtractor::getInitialLength

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:347-350
+    return createStringError(
+        errc::invalid_argument,
+        "parsing line table prologue at offset 0x%8.8" PRIx64 ": %s",
+        PrologueOffset, toString(std::move(Err)).c_str());
----------------
If I understand this correctly, this will now report cases where the unit length couldn't be read due to data running out. Should this change include some testing for that too?

Also, in that case, if I've followed the code correctly, the error message will be "parsing line table prologue at offset 0x00000000: unexpected end of data", which doesn't give particularly great precision as to where the unexpected end of data was. How about adding something like "parsing line table prologue at offset 0x00000000: unable to read unit_length: unexpected end of data"?


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:1218
 bool DWARFDebugLine::Prologue::totalLengthIsValid() const {
-  return TotalLength == dwarf::DW_LENGTH_DWARF64 ||
-         TotalLength < dwarf::DW_LENGTH_lo_reserved;
+  return TotalLength != 0u;
 }
----------------
Idle thought: If wonder if this should be unit tested (probably at a slightly higher level)? I could easily see a bug where a length equal to DW_LENGTH_lo_reserved in a DWARF64 file was not treated as valid.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp:810
 
 TEST_F(DebugLineBasicFixture, ParserMovesToEndForBadLengthWhenParsing) {
   if (!setupGenerator())
----------------
This and the following case might want renaming slightly, since the offset is no longer at the end. Perhaps "ParserMarkedAsDoneFor..."?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75116/new/

https://reviews.llvm.org/D75116