[PATCH] D147922: [Assignment Tracking] Fix replaceVariableLocationOp for dbg.assign with DIArgList

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

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

The last time this function was updated DIArgLists were not supported for dbg.assigns. Without this patch it's possible to now dereference an invalid (the end) iterator. This occurs when the Address component gets replaced and there's a DIArgList for the Value component (the contained values are irrelevant).

The added unittest crashes without the code change in this patch.


https://reviews.llvm.org/D147922

Files:
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/unittests/IR/DebugInfoTest.cpp


Index: llvm/unittests/IR/DebugInfoTest.cpp
===================================================================
--- llvm/unittests/IR/DebugInfoTest.cpp
+++ llvm/unittests/IR/DebugInfoTest.cpp
@@ -370,6 +370,12 @@
   // Replace both.
   TEST_REPLACE(/*Old*/ P2, /*New*/ P1, /*Value*/ P1, /*Address*/ P1);
 
+  // Replace address only, value uses a DIArgList.
+  // Value = {DIArgList(V1)}, Addr = P1.
+  DAI->setRawLocation(DIArgList::get(C, ValueAsMetadata::get(V1)));
+  DAI->setExpression(DIExpression::get(
+      C, {dwarf::DW_OP_LLVM_arg, 0, dwarf::DW_OP_stack_value}));
+  TEST_REPLACE(/*Old*/ P1, /*New*/ P2, /*Value*/ V1, /*Address*/ P2);
 #undef TEST_REPLACE
 }
 
Index: llvm/lib/IR/IntrinsicInst.cpp
===================================================================
--- llvm/lib/IR/IntrinsicInst.cpp
+++ llvm/lib/IR/IntrinsicInst.cpp
@@ -135,14 +135,14 @@
   assert(NewValue && "Values must be non-null");
   auto Locations = location_ops();
   auto OldIt = find(Locations, OldValue);
-  assert((OldIt != Locations.end() || DbgAssignAddrReplaced) &&
-         "OldValue must be a current location");
+  if (OldIt == Locations.end()) {
+    assert(DbgAssignAddrReplaced &&
+           "OldValue must be dbg.assign addr if unused in DIArgList");
+    return;
+  }
+
+  assert(OldIt != Locations.end() && "OldValue must be a current location");
   if (!hasArgList()) {
-    // Additional check necessary to avoid unconditionally replacing this
-    // operand when a dbg.assign address is replaced (DbgAssignAddrReplaced is
-    // true).
-    if (OldValue != getVariableLocationOp(0))
-      return;
     Value *NewOperand = isa<MetadataAsValue>(NewValue)
                             ? NewValue
                             : MetadataAsValue::get(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147922.512098.patch
Type: text/x-patch
Size: 1758 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230410/1b9e0114/attachment.bin>