[Mlir-commits] [mlir] [mlir][func] Fix crash when a C interface wrapper cannot be built (PR #216489)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Hamza Qureshi (hamzaqureshi5)

<details>
<summary>Changes</summary>

Fixes #<!-- -->216268

`mlir-opt --gpu-to-llvm` crashes on this input:

```mlir
func.func @<!-- -->m0(%arg0: !async.token) attributes {llvm.emit_c_interface} {
  return
}
```

`llvm.emit_c_interface` asks for an extra wrapper function so the function can be called from C. The wrapper signature is built by `convertFunctionTypeCWrapper`, which requires every argument type to be LLVM compatible and returns nothing otherwise. That is stricter than the signature conversion, so it can fail for a function whose signature converted fine.

Nothing checked for that. The empty result was passed to `LLVMFuncOp::create`, which built a `TypeAttr` from a null type and crashed.

This checks the wrapper signature next to the existing varargs check, before any IR is created, so the pattern fails cleanly and the function is left unconverted.

Note the wrapper path for external functions only had an assert here, with a comment saying the conversion could not fail because it would have failed earlier. That assumption does not hold: the signature conversion and the C wrapper conversion do not accept the same types.

Testing: added `mlir/test/Conversion/GPUCommon/emit-c-interface-invalid.mlir` covering a function with a body, an external function, and a function that must still get its wrapper. On an assertions build the reproducer segfaults before the change and exits cleanly after, the new test fails without the change and passes with it, and `mlir/test/Conversion` stays at 413/413.

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


2 Files Affected:

- (modified) mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp (+9) 
- (added) mlir/test/Conversion/GPUCommon/emit-c-interface-invalid.mlir (+34) 


``````````diff
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 2686158f67e76..1b6bbf5fe910c 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -472,6 +472,15 @@ FailureOr<LLVM::LLVMFuncOp> mlir::convertFuncOpToLLVMFuncOp(
     return funcOp.emitError("C interface for variadic functions is not "
                             "supported yet.");
 
+  // The C wrapper signature requires all argument types to be LLVM compatible,
+  // which is stricter than the signature conversion above. Check it here, so
+  // that the pattern fails before any IR is created rather than while emitting
+  // the wrapper.
+  if (!useBarePtrCallConv && emitCWrapper &&
+      !converter.convertFunctionTypeCWrapper(funcTy).first)
+    return rewriter.notifyMatchFailure(
+        funcOp, "failed to convert C interface wrapper function type");
+
   // Lower function attrs
   FailureOr<LoweredLLVMFuncAttrs> loweredAttrs =
       lowerDiscardableAttrsForLLVMFunc(funcOp, *llvmType);
diff --git a/mlir/test/Conversion/GPUCommon/emit-c-interface-invalid.mlir b/mlir/test/Conversion/GPUCommon/emit-c-interface-invalid.mlir
new file mode 100644
index 0000000000000..3e77541e1ef5f
--- /dev/null
+++ b/mlir/test/Conversion/GPUCommon/emit-c-interface-invalid.mlir
@@ -0,0 +1,34 @@
+// RUN: mlir-opt %s --gpu-to-llvm -split-input-file | FileCheck %s
+
+// The C interface wrapper signature requires every argument type to be LLVM
+// compatible, which is stricter than the plain signature conversion: this
+// pass converts the function signature but cannot build a wrapper for it.
+// The function must be left unconverted instead of creating a wrapper with a
+// null function type.
+
+// CHECK-LABEL: func.func @ciface_unsupported_arg
+// CHECK-NOT:     _mlir_ciface_
+func.func @ciface_unsupported_arg(%arg0: !async.token)
+    attributes {llvm.emit_c_interface} {
+  return
+}
+
+// -----
+
+// Same for an external function, which takes the other wrapper code path.
+
+// CHECK-LABEL: func.func private @ciface_unsupported_arg_decl
+// CHECK-NOT:     _mlir_ciface_
+func.func private @ciface_unsupported_arg_decl(%arg0: !async.token)
+    attributes {llvm.emit_c_interface}
+
+// -----
+
+// A function whose arguments do convert must still get its wrapper.
+
+// CHECK-LABEL: llvm.func @ciface_supported_arg
+// CHECK:       llvm.func @_mlir_ciface_ciface_supported_arg
+func.func @ciface_supported_arg(%arg0: memref<4xf32>)
+    attributes {llvm.emit_c_interface} {
+  return
+}

``````````

</details>


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