[PATCH] D81570: [DebugInfo] Don't print extended opcode operands if invalid

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added a comment.

You may also want to add a couple of tests with an unrecognized opcode and premature termination, like these:

  std::make_tuple(
      3, 9, 99,
      ValueAndLengths{{0x12345678, LineTable::Quad}}, ...
  std::make_tuple(
      4, 9, 99,
      ValueAndLengths{{0x12345678, LineTable::Quad}}, ...



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:814
         State.Row.EndSequence = true;
-        if (Verbose) {
+        if (Cursor && Verbose) {
           *OS << "\n";
----------------
It seems it is not possible to get here with an invalidated `Cursor`, no?


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp:1468
         std::make_tuple(
             2, 9, DW_LNE_set_address,
             ValueAndLengths{{0x12345678, LineTable::Quad}},
----------------
Maybe it is better to change `DW_LNE_set_address` to something different to emphasize that this test case goes to the `default` branch of `switch (SubOpcode)`?


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp:1470
             ValueAndLengths{{0x12345678, LineTable::Quad}},
-            "Unrecognized extended op 0x00 length 9",
+            " (<parsing error>)",
             "unexpected end of data at offset 0x31 while reading [0x31, 0x32)"),
----------------
Should we add this if there are no raw bytes to report, especially when even opcode is missing? That space at the beginning looks quite untidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81570