[mlir][Parser] Fix crash when resolving invalid operands with missing location

[matthias-springer]

When resolveOperands reports an error and no valid SMLoc was provided, report the error at the beginning of the op instead of crashing.

Assert `Ptr >= BufStart && Ptr <= Buffer->getBufferEnd()' in llvm/lib/Support/SourceMgr.cpp:llvm::SourceMgr::SrcBuffer::getLineNumberSpecialized failed

E.g., this is currently the case when parsing the following op with a type but without any operands:

let assemblyFormat = "$str (`,` $args^)? attr-dict (`:` type($args)^)?";

Reported error (with this PR):

within split at mlir/test/IR/invalid-ops.mlir:122 offset :4:1: error: custom op 'test.variadic_args_types_split' number of operands and types do not match: got 0 operands and 1 types
test.variadic_args_types_split "hello_world" : i32
^

In the ODS-generated C++, the SMLoc is populated when parsing the optional group containing $args. However, this group is missing in the test case.

There are likely additional hand-written parsers that suffer from the same problem.

Note: I tried emitting a second SMLoc for the optional type group in the OpFormatGen.cpp, but this adds quite a bit of complexity in the code base for little improvement in user experience.

[llvmbot]

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

When resolveOperands reports an error and no valid SMLoc was provided, fall back to the beginning of the op instead of crashing.

E.g., this is currently the case when parsing the following op with a type but without any operands:

let assemblyFormat = "$str (`,` $args^)? attr-dict (`:` type($args)^)?";

In the ODS-generated C++, the SMLoc is populated when parsing the optional group containing $args. However, this group is missing in the test case.

There are likely additional hand-written parsers that suffer from the same problem.

Note: I tried emitting a second SMLoc for the optional type group in the OpFormatGen.cpp, but this adds quite a bit of complexity in the code base for little improvement in user experience.

---

Full diff: https://github.com/llvm/llvm-project/pull/128163.diff

3 Files Affected:

• (modified) mlir/include/mlir/IR/OpImplementation.h (+4-2)
• (modified) mlir/test/IR/invalid-ops.mlir (+5)
• (modified) mlir/test/lib/Dialect/Test/TestOpsSyntax.td (+5)

diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index a863e881ee7c8..d80bff2d78217 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -1603,10 +1603,12 @@ class OpAsmParser : public AsmParser {
                   SmallVectorImpl<Value> &result) {
     size_t operandSize = llvm::range_size(operands);
     size_t typeSize = llvm::range_size(types);
-    if (operandSize != typeSize)
-      return emitError(loc)
+    if (operandSize != typeSize) {
+      // If no location was provided, report errors at the beginning of the op.
+      return emitError(loc.isValid() ? loc : getNameLoc())
              << "number of operands and types do not match: got " << operandSize
              << " operands and " << typeSize << " types";
+    }
 
     for (auto [operand, type] : llvm::zip_equal(operands, types))
       if (resolveOperand(operand, type, result))
diff --git a/mlir/test/IR/invalid-ops.mlir b/mlir/test/IR/invalid-ops.mlir
index 6672c8840ffde..c7cef97c246e8 100644
--- a/mlir/test/IR/invalid-ops.mlir
+++ b/mlir/test/IR/invalid-ops.mlir
@@ -118,3 +118,8 @@ func.func @invalid_splat(%v : f32) { // expected-note {{prior use here}}
 
 // expected-error@+1 {{expected ':' after block name}}
 "g"()({^a:^b })
+
+// -----
+
+// expected-error@+1 {{number of operands and types do not match: got 0 operands and 1 types}}
+test.variadic_args_types_split "hello_world" : i32
diff --git a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
index 2848cb994231b..9c199f0c3b6fc 100644
--- a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
+++ b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
@@ -767,4 +767,9 @@ def FormatInferTypeVariadicOperandsOp
   }];
 }
 
+def VariadicArgsTypesSplit : TEST_Op<"variadic_args_types_split"> {
+  let arguments = (ins StrAttr:$str, Variadic<AnyType>:$args);
+  let assemblyFormat = "$str (`,` $args^)? attr-dict (`:` type($args)^)?";
+}
+
 #endif  // TEST_OPS_SYNTAX