[PATCH] D78850: Memory corruption issure for DenseStringElementsAttr

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/lib/IR/AttributeDetail.h:614
+    // Otherwise, this is a splat.
+    return KeyTy(ty, data, hashVal, /*isSplat=*/true);
   }
----------------
mehdi_amini wrote:
> I don't understand the fix, can you elaborate? Seems like the ArrayRef is passed by value to the KeyTy so I don't get the lifetime issue?
We were creating an ArrayRef pointing to the local variable firstElt before, but since it is a reference it should be fine. So there is likely a copy of the StringRef in the implicit conversion to ArrayRef.

The change here is making us taking the hash of the entire array instead of the first element, I don't think this is necessary, something like this should work as well:

```
    return KeyTy(ty, ArrayRef<StringRef>{&firstElt, 1}, hashVal, /*isSplat=*/true);
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78850