[llvm] c95b322 - [Assignment Tracking] Fix replaceVariableLocationOp for dbg.assign with DIArgList

[via llvm-commits]

Author: OCHyams
Date: 2023-04-11T17:05:20+01:00
New Revision: c95b322b738b63abedb0483bafe08b92c3de8e31

URL: https://github.com/llvm/llvm-project/commit/c95b322b738b63abedb0483bafe08b92c3de8e31
DIFF: https://github.com/llvm/llvm-project/commit/c95b322b738b63abedb0483bafe08b92c3de8e31.diff

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

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.

Reviewed By: jmorse

Differential Revision: https://reviews.llvm.org/D147922

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index 1c78d4ba1a43b..9270d708a620a 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -135,14 +135,14 @@ void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue,
   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(

diff  --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index 3a00c3e5ea866..f038ff2fc7f31 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -370,6 +370,12 @@ TEST(DbgAssignIntrinsicTest, replaceVariableLocationOp) {
   // 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
 }