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

Igor Kudrin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 22 06:57:47 PDT 2020


ikudrin accepted this revision.
ikudrin added a comment.
This revision is now accepted and ready to land.

Overall looks great.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:814
         State.Row.EndSequence = true;
-        if (Verbose) {
+        if (Cursor && Verbose) {
           *OS << "\n";
----------------
ikudrin wrote:
> It seems it is not possible to get here with an invalidated `Cursor`, no?
Maybe it's worth to drop here a note about that `Cursor` is always valid here? So that a programmer coming across these lines would not be surprised why `Cursor` is checked in other branches, but not here?


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:937-946
+        while (ByteCursor && ByteCursor.tell() < End) {
+          uint8_t Byte = TableData.getU8(ByteCursor);
+          if (ByteCursor) {
+            if (ByteCursor.tell() == OperandOffset + 1)
+              *OS << " (<parsing error>";
+            *OS << format(" %2.2" PRIx8, Byte);
+          }
----------------
I suppose that this might be simplified a bit using a `do-while` loop. Something like this:
```
uint8_t Byte = TableData.getU8(ByteCursor);
if (ByteCursor) {
  *OS << " (<parsing error>";
  do {
    *OS << format(" %2.2" PRIx8, Byte);
    Byte = TableData.getU8(ByteCursor);
  } while (ByteCursor);
  *OS << ")";
}
```
WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81570





More information about the llvm-commits mailing list