[Mlir-commits] [mlir] [mlir] [tensor] Crash in getPackOpResultTypeShape for tensor.pack/unpack ops. (PR #90641)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > Thanks for the fix. The issue is at the call site though. The `ArrayRef` indicates a "reference argument" so this seems to be an error in how the function is called. Do you have more information on that?
> 
> Thanks for reviewing.
> 
> TBH, this "fix" is a puzzle for me too since `ArrayRef` doesn't own the data. Debugging doesn't provide any information since the issue is only reproducible in `Release` build. `RelWithDebInfo` doesn't produce the crash either.
> 
> The callgraph leading to the crash is: `PackOp::verify -> commonVerifierPackAndUnPackOp -> inferPackedType -> getPackOpResultTypeShape`. All the call-sites starting from `inferPackedType` pass `innerDimsPos` as `ArrayRef` as the getter itself returns it as `ArrayRef`. This seems to be because `innerDimsPos` is a `DenseI64ArrayAttr` which gets cast as `ArrayRef` when returned in the getters.
> 
> What I've tried unsuccessfully:
> 
> 1. Convert the returned value from `getInnerDimsPos` to a `SmallVector` in `commonVerifierPackAndUnPackOp` and use that for all the subsequent calls.
> 2. Call `inferPackedType` as the first function from `commonVerifierPackAndUnPackOp` (just to ensure that the memory for `innerDimsPos` doesn't get affected by any other call -- though nothing in that code seemed to be doing that)

Thanks for the details.. I tracked the stack as you described, and I dont find any obvious footguns. I see in some places the conversion to `SmallVector` of the result of `getInnerDimsPos` (https://github.com/shark-infra/llvm-project/blob/b3291793f11924a3b62601aabebebdcfbb12a9a1/mlir/lib/Dialect/Tensor/IR/TensorOps.cpp#L3734) . My other suspicion is that depending on how you are using it, the pack operations itself might be deleted, and these methods are being called on a deleted op.

Maybe provide a repro for this? cc @jpienaar  or @joker-eph on this. Hopefully its not some corner case w.r.t properties.

https://github.com/llvm/llvm-project/pull/90641