Memory corruption issure for DenseStringElementsAttr

[rsuderman]

Summary

There was a memory corruption issue where the lifespan of the ArrayRef<StringRef> would fail. Directly passing the data will avoid the issue.

[mehdi_amini]

mlir/lib/IR/AttributeDetail.h

614

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?

[mehdi_amini]

mlir/lib/IR/AttributeDetail.h

614

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);

[mehdi_amini]

mlir/lib/IR/AttributeDetail.h

614

Actually nicer and equivalent I think:

return KeyTy(ty, data.take_front(), hashVal, /*isSplat=*/true);

mlir/lib/IR/AttributeDetail.h

static KeyTy getKey(ShapedType ty, ArrayRef<StringRef> data,
     auto hashVal = llvm::hash_value(firstElt);
 
     // Check to see if this storage represents a splat. If it doesn't then
     // combine the hash for the data starting with the first non splat element.
     for (size_t i = 1, e = data.size(); i != e; i++)
       if (!firstElt.equals(data[i]))
         return KeyTy(ty, data, llvm::hash_combine(hashVal, data.drop_front(i)));
 
-    // Otherwise, this is a splat so just return the hash of the first element.
-    return KeyTy(ty, {firstElt}, hashVal, /*isSplat=*/true);
+    // Otherwise, this is a splat.
+    return KeyTy(ty, data, hashVal, /*isSplat=*/true);
   }
 
   /// Hash the key for the storage.
   static llvm::hash_code hashKey(const KeyTy &key) {
     return llvm::hash_combine(key.type, key.hashCode);
   }
 
   /// Construct a new storage instance.