[PATCH] D101373: [DebugInfo] Drop DBG_VALUE_LISTs with an excessive number of debug operands

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer created this revision.
StephenTozer added reviewers: djtodoro, aprantl, probinson, jmorse, Orlando, dblaikie.
StephenTozer added a project: debug-info.
Herald added a subscriber: hiraditya.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This patch addresses a crash that has been observed from rG791930d74087 <https://reviews.llvm.org/rG791930d74087b8ae8901172861a0fd21a211e436>, although the underlying error was introduced in patch rG7d0cafba962c <https://reviews.llvm.org/rG7d0cafba962c56d79b2f1c2caab695551d3be6df>. Currently, LiveDebugVariables asserts that no `DBG_VALUE_LIST` can have 64 or more debug operands. This assertion was added on the assumption that this limit was high enough that it would almost certainly be hit only as the result of an earlier compiler error, but a relatively simple reproducer has been found that triggers this assertion; this was noted in the latest review comments for D91722 <https://reviews.llvm.org/D91722>.

This patch fixes this issue by removing the assertion, and instead dropping the `DBG_VALUE_LIST`s. It may technically be possible to keep them, but we should make sure that such complex `DBG_VALUE_LIST`s will not introduce significant performance issues first. There may also be issues with the use of IntervalMap in LiveDebugVariables; the reason for the current limit of <64 operands is that supporting any greater number of operands would increase the size of the `DbgVariableValue` class, which is kept as small as possible to be used as the value in an IntervalMap.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101373

Files:
  llvm/lib/CodeGen/LiveDebugVariables.cpp


Index: llvm/lib/CodeGen/LiveDebugVariables.cpp
===================================================================
--- llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -119,17 +119,33 @@
             DIExpression::replaceArg(Expression, OpIdx, DuplicatingIdx);
       }
     }
-    // A debug value referencing 64+ unique machine locations is very likely
-    // to be the result of a bug earlier in the pipeline. If by some means this
-    // limit is validly reached, then we can add a byte to the size of
-    // LocNoCount.
-    assert(LocNoVec.size() < 64 &&
-           "debug value containing 64+ unique machine locations is not "
-           "supported by Live Debug Variables");
-    LocNoCount = LocNoVec.size();
-    if (LocNoCount > 0) {
+    // FIXME: Debug values referencing 64+ unique machine locations are rare and
+    // currently unsupported for performance reasons. If we can verify that
+    // performance is acceptable for such debug values, we can increase the
+    // bit-width of LocNoCount to 14 to enable up to 16384 unique machine
+    // locations. We will also need to verify that this does not cause issues
+    // with LiveDebugVariables' use of IntervalMap.
+    if (LocNoVec.size() < 64) {
+      LocNoCount = LocNoVec.size();
+      if (LocNoCount > 0) {
+        LocNos.reset(new unsigned[LocNoCount]());
+        std::copy(LocNoVec.begin(), LocNoVec.end(), loc_nos_begin());
+      }
+    } else {
+      LLVM_DEBUG(dbgs() << "Found debug value with 64+ unique machine "
+                           "locations, dropping...\n");
+      LocNoCount = 1;
+      // Turn this into an undef debug value list; right now, the simplest form
+      // of this is an expression with one arg, and an undef debug operand.
+      Expression =
+          DIExpression::get(Expr.getContext(), {dwarf::DW_OP_LLVM_arg, 0,
+                                                dwarf::DW_OP_stack_value});
+      if (auto FragmentInfoOpt = Expr.getFragmentInfo())
+        Expression = *DIExpression::createFragmentExpression(
+            Expression, FragmentInfoOpt->OffsetInBits,
+            FragmentInfoOpt->SizeInBits);
       LocNos.reset(new unsigned[LocNoCount]());
-      std::copy(LocNoVec.begin(), LocNoVec.end(), loc_nos_begin());
+      LocNos[0] = UndefLocNo;
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101373.340866.patch
Type: text/x-patch
Size: 2341 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210427/716206c3/attachment.bin>