[PATCH] D105129: [DebugInfo] Prevent error when updating location operands for a dbg.value

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer created this revision.
StephenTozer added reviewers: djtodoro, aprantl, dblaikie, jmorse, probinson, thakis.
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 fixes the issue observed at https://bugs.chromium.org/p/chromium/issues/detail?id=1224338, mentioned in D91722 <https://reviews.llvm.org/D91722>.

The above issue occurs in `CodeGenPrepare::fixupDbgValue`, where we attempt to update the location operands of a `dbg.value`. The error occurs when the `dbg.value` uses a `DIArgList` that contains the same value multiple times; currently the update is performed by iterating over the location operands, and updating them within that loop by calling `replaceVariableLocationOp`, which invalidates the iterator; specifically, it continues to point to the old list of values, and so the loop attempts to replace the same value again when it isn't present in the list anymore, resulting in the observed error.

This has been fixed by first collecting the `dbg.value`'s location operands into a set and then iterating over that, ensuring that we don't invalidate our loop iterator or attempt to replace a non-existing operand. Upon investigating, I discovered that the same issue exists in `HWAddressSanitizer::sanitizeFunction`, and have added the same fix as part of this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105129

Files:
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1348,7 +1348,9 @@
     for (auto &BB : F) {
       for (auto &Inst : BB) {
         if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst)) {
-          for (Value *V : DVI->location_ops()) {
+          SmallDenseSet<Value *> LocationOps(DVI->location_ops().begin(),
+                                             DVI->location_ops().end());
+          for (Value *V : LocationOps) {
             if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
               if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI))
                 DVI->replaceVariableLocationOp(V, NewAI);
Index: llvm/lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -7994,7 +7994,9 @@
 
   // Does this dbg.value refer to a sunk address calculation?
   bool AnyChange = false;
-  for (Value *Location : DVI.getValues()) {
+  SmallDenseSet<Value *> LocationOps(DVI.location_ops().begin(),
+                                     DVI.location_ops().end());
+  for (Value *Location : LocationOps) {
     WeakTrackingVH SunkAddrVH = SunkAddrs[Location];
     Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr;
     if (SunkAddr) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105129.355264.patch
Type: text/x-patch
Size: 1533 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210629/c6c9bd2c/attachment.bin>