[PATCH] D156226: [DebugInfo] Fix crash when printing malformed DBG machine instructions

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

> Just to confirm my understanding: this is only suppressing printing the operands of debug instructions when there aren't enough operands, yes? It'd be unfortunate if we completely skipped printing an instruction because it was malformed, thus hiding it from developers.

Correct, it only suppresses pretty-printing of the debug variable info as a comment.

As an alternative approach maybe all the accessors like getDebugVariableOp should check getNumOperands and return a failure value if the operand does not exist. But I do not want to get too deeply involved in this, I just wanted to stop `llc -print-after-all` from crashing.



================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:1886-1889
+  // Print extra comments for DEBUG_VALUE and friends if they are well-formed.
+  if ((isNonListDebugValue() && getNumOperands() == 4) ||
+      (isDebugValueList() && getNumOperands() == 2) ||
+      (isDebugRef() && getNumOperands() == 3)) {
----------------
jmorse wrote:
> I believe these should be `>=` operators rather than `==` . The instructions that can be variadic may have several operands or none, and we want  the Variable and DebugLoc printed in those scenarios.
OK. I was copying from MachineVerifier which does this:
```
  // A fully-formed DBG_VALUE must have a location. Ignore partially formed
  // DBG_VALUEs: these are convenient to use in tests, but should never get
  // generated.
  if (MI->isDebugValue() && MI->getNumOperands() == 4)
    if (!MI->getDebugLoc())
      report("Missing DebugLoc for debug instruction", MI);
```
But I guess that is slightly wrong for DBG_VALUE (should be >= 4), very wrong for DBG_VALUE_LIST (should be >= 2) and completely misses DBG_INSTR_REF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156226