[Mlir-commits] [mlir] [MLIR][Tensor] Fix out-of-bounds FoldEmptyTensorWithDimOp crash #111270 (PR #112196)

[llvmlistbot at llvm.org]

================
@@ -979,7 +979,9 @@ struct FoldEmptyTensorWithDimOp : public OpRewritePattern<DimOp> {
     auto emptyTensorOp = dimOp.getSource().getDefiningOp<EmptyOp>();
     if (!emptyTensorOp || !maybeConstantIndex)
       return failure();
-    if (!emptyTensorOp.getType().isDynamicDim(*maybeConstantIndex))
+    auto emptyTensorType = emptyTensorOp.getType();
----------------
brod4910 wrote:

Okay, so you're suggesting to add `hasVerifier = 1` in ODS, then verify the condition? This doesn't seem to be inline with the spec of `tensor.dim` since an OOBs access is not a verification error but UB.

This comment from the `tensor.dim` op `fold` method seems to also suggest that as well:
```
Out of bound indices produce undefined behavior but are still valid IR.
Don't choke on them.
```

I do agree that if the index is truly dynamic, then this fold shouldn't be possible and the crash doesn't occur.

I may be totally confused here but handling the OOBs check in any of the code related to `tensor.dim` op seems to counter the spec of the op itself.

I found a similar check in the `Bufferization` dialect `FoldDimOfAllocTensorOp` pattern (`mlir/lib/Dialect/Bufferization/IR/BufferizationOps.cpp:329`):

```cpp
if (*maybeConstantIndex < 0 ||
    *maybeConstantIndex >= allocTensorOp.getType().getRank())
  return failure();
```

which also suggests I missed a check for when the index is `< 0`.

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