[PATCH] D68816: [NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap

Bjorn Pettersson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 29 03:19:05 PDT 2019


bjope added a subscriber: uabelho.
bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1141
+  if (auto *UserVals = lookupVirtReg(OldReg))
+    for (auto *UV : *UserVals)
+      for (unsigned i = 0; i != NewRegs.size(); ++i)
----------------
The mapVirtReg call below adds elements to the DenseMap. When a DenseMap grows/shrinks it might be re-allocated. So the pointer to the SmallVector (stored inside the map elements), called `UserVals` here, may be a dangling pointer after the first call to mapVirtReg and the vector it points to may have been moved.

So the iteration over *UserVals here is not safe, and asan complains about accessing deallocated memory.

A quick workaround that I'm trying downstream is to copy the vector like this:

```
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 6032bfe48af..4659ebea254 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -1144,10 +1144,16 @@ void LDVImpl::splitRegister(unsigned OldReg, ArrayRef<unsigned> NewRegs) {
     return;
 
   // Map all of the new virtual registers.
-  if (auto *UserVals = lookupVirtReg(OldReg))
-    for (auto *UV : *UserVals)
+  if (auto *UserVals = lookupVirtReg(OldReg)) {
+    // Make a copy of the vector. This is needed because VirtRegToUserVals is
+    // updated in the loop below, so the pointer into the VirtRegToUserVals
+    // DenseMap may become invalid in case the map is reallocated.
+    SmallVector<UserValue *, 4> UserValsCopy;
+    UserValsCopy.assign(UserVals->begin(), UserVals->end());
+    for (auto *UV : UserValsCopy)
       for (unsigned i = 0; i != NewRegs.size(); ++i)
         mapVirtReg(NewRegs[i], UV);
+  }
 }
```

No idea if this impacts the performance negatively (considering that the idea with the refactoring into using a DenseMap seem to be to improve compile speed maybe there are better solutions). But right now we the code is broken so I guess we need to submit a workaround or a revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68816





More information about the llvm-commits mailing list