[WebAssembly,clang] Add __builtin_wasm_test_function_pointer_signature#150201
[WebAssembly,clang] Add __builtin_wasm_test_function_pointer_signature#150201
Conversation
…e clang intrinsic Tests if the runtime type of the function pointer matches the static type. If this returns false, calling the function pointer will trap. Uses `@llvm.wasm.ref.test.func` added in llvm#147486.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Hood Chatham (hoodmane) ChangesTests if the runtime type of the function pointer matches the static type. If this returns false, calling the function pointer will trap. Uses cc @sbc100 @dschuff @tlively 7 Files Affected:
diff --git a/clang/include/clang/Basic/BuiltinsWebAssembly.def b/clang/include/clang/Basic/BuiltinsWebAssembly.def
index e2afcc08064b2..1e03a40b1a22b 100644
--- a/clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ b/clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -199,6 +199,12 @@ TARGET_BUILTIN(__builtin_wasm_ref_is_null_extern, "ii", "nct", "reference-types"
// return type.
TARGET_BUILTIN(__builtin_wasm_ref_null_func, "i", "nct", "reference-types")
+// Check if the static type of a function pointer matches its static type. Used
+// to avoid "function signature mismatch" traps. Takes a function pointer, uses
+// table.get to look up the pointer in __indirect_function_table and then
+// ref.test to test the type.
+TARGET_BUILTIN(__builtin_wasm_test_function_pointer_signature, "i.", "nct", "reference-types")
+
// Table builtins
TARGET_BUILTIN(__builtin_wasm_table_set, "viii", "t", "reference-types")
TARGET_BUILTIN(__builtin_wasm_table_get, "iii", "t", "reference-types")
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b2ea65ae111be..745ac8cb5dc2e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7575,6 +7575,8 @@ def err_typecheck_illegal_increment_decrement : Error<
"cannot %select{decrement|increment}1 value of type %0">;
def err_typecheck_expect_int : Error<
"used type %0 where integer is required">;
+def err_typecheck_expect_function_pointer
+ : Error<"used type %0 where function pointer is required">;
def err_typecheck_expect_hlsl_resource : Error<
"used type %0 where __hlsl_resource_t is required">;
def err_typecheck_arithmetic_incomplete_or_sizeless_type : Error<
@@ -13202,6 +13204,10 @@ def err_wasm_builtin_arg_must_match_table_element_type : Error <
"%ordinal0 argument must match the element type of the WebAssembly table in the %ordinal1 argument">;
def err_wasm_builtin_arg_must_be_integer_type : Error <
"%ordinal0 argument must be an integer">;
+def err_wasm_builtin_test_fp_sig_cannot_include_reference_type
+ : Error<"not supported for "
+ "function pointers with a reference type %select{return "
+ "value|parameter}0">;
// OpenACC diagnostics.
def warn_acc_routine_unimplemented
diff --git a/clang/include/clang/Sema/SemaWasm.h b/clang/include/clang/Sema/SemaWasm.h
index 2123e073516cb..8c0639fd7e76f 100644
--- a/clang/include/clang/Sema/SemaWasm.h
+++ b/clang/include/clang/Sema/SemaWasm.h
@@ -37,6 +37,7 @@ class SemaWasm : public SemaBase {
bool BuiltinWasmTableGrow(CallExpr *TheCall);
bool BuiltinWasmTableFill(CallExpr *TheCall);
bool BuiltinWasmTableCopy(CallExpr *TheCall);
+ bool BuiltinWasmTestFunctionPointerSignature(CallExpr *TheCall);
WebAssemblyImportNameAttr *
mergeImportNameAttr(Decl *D, const WebAssemblyImportNameAttr &AL);
diff --git a/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp b/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp
index b7fd70e855d40..5644792028013 100644
--- a/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp
@@ -12,7 +12,10 @@
#include "CGBuiltin.h"
#include "clang/Basic/TargetBuiltins.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/IR/Constants.h"
#include "llvm/IR/IntrinsicsWebAssembly.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace clang;
using namespace CodeGen;
@@ -218,6 +221,60 @@ Value *CodeGenFunction::EmitWebAssemblyBuiltinExpr(unsigned BuiltinID,
Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_ref_null_func);
return Builder.CreateCall(Callee);
}
+ case WebAssembly::BI__builtin_wasm_test_function_pointer_signature: {
+ Value *FuncRef = EmitScalarExpr(E->getArg(0));
+
+ // Get the function type from the argument's static type
+ QualType ArgType = E->getArg(0)->getType();
+ const PointerType *PtrTy = ArgType->getAs<PointerType>();
+ assert(PtrTy && "Sema should have ensured this is a function pointer");
+
+ const FunctionType *FuncTy = PtrTy->getPointeeType()->getAs<FunctionType>();
+ assert(FuncTy && "Sema should have ensured this is a function pointer");
+
+ // In the llvm IR, we won't have access anymore to the type of the function
+ // pointer so we need to insert this type information somehow. We gave the
+ // @llvm.wasm.ref.test.func varargs and here we add an extra 0 argument of
+ // the type corresponding to the type of each argument of the function
+ // signature. When we lower from the IR we'll use the types of these
+ // arguments to determine the signature we want to test for.
+
+ // Make a type index constant with 0. This gets replaced by the actual type
+ // in WebAssemblyMCInstLower.cpp.
+ llvm::FunctionType *LLVMFuncTy =
+ cast<llvm::FunctionType>(ConvertType(QualType(FuncTy, 0)));
+
+ uint NParams = LLVMFuncTy->getNumParams();
+ std::vector<Value *> Args;
+ Args.reserve(NParams + 2);
+ // The only real argument is the FuncRef
+ Args.push_back(FuncRef);
+
+ // Add the type information
+ auto addType = [&Args](llvm::Type *T) {
+ if (T->isVoidTy()) {
+ // Do nothing
+ } else if (T->isFloatingPointTy()) {
+ Args.push_back(ConstantFP::get(T, 0));
+ } else if (T->isIntegerTy()) {
+ Args.push_back(ConstantInt::get(T, 0));
+ } else {
+ // TODO: Handle reference types here. For now, we reject them in Sema.
+ llvm_unreachable("Unhandled type");
+ }
+ };
+
+ addType(LLVMFuncTy->getReturnType());
+ // The token type indicates the boundary between return types and param
+ // types.
+ Args.push_back(
+ PoisonValue::get(llvm::Type::getTokenTy(getLLVMContext())));
+ for (uint i = 0; i < NParams; i++) {
+ addType(LLVMFuncTy->getParamType(i));
+ }
+ Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_ref_test_func);
+ return Builder.CreateCall(Callee, Args);
+ }
case WebAssembly::BI__builtin_wasm_swizzle_i8x16: {
Value *Src = EmitScalarExpr(E->getArg(0));
Value *Indices = EmitScalarExpr(E->getArg(1));
diff --git a/clang/lib/Sema/SemaWasm.cpp b/clang/lib/Sema/SemaWasm.cpp
index 6faea24a46b09..8998492a71619 100644
--- a/clang/lib/Sema/SemaWasm.cpp
+++ b/clang/lib/Sema/SemaWasm.cpp
@@ -227,6 +227,53 @@ bool SemaWasm::BuiltinWasmTableCopy(CallExpr *TheCall) {
return false;
}
+bool SemaWasm::BuiltinWasmTestFunctionPointerSignature(CallExpr *TheCall) {
+ if (SemaRef.checkArgCount(TheCall, 1))
+ return true;
+
+ Expr *FuncPtrArg = TheCall->getArg(0);
+ QualType ArgType = FuncPtrArg->getType();
+
+ // Check that the argument is a function pointer
+ const PointerType *PtrTy = ArgType->getAs<PointerType>();
+ if (!PtrTy) {
+ return Diag(FuncPtrArg->getBeginLoc(),
+ diag::err_typecheck_expect_function_pointer)
+ << ArgType << FuncPtrArg->getSourceRange();
+ }
+
+ const FunctionProtoType *FuncTy =
+ PtrTy->getPointeeType()->getAs<FunctionProtoType>();
+ if (!FuncTy) {
+ return Diag(FuncPtrArg->getBeginLoc(),
+ diag::err_typecheck_expect_function_pointer)
+ << ArgType << FuncPtrArg->getSourceRange();
+ }
+
+ // Check that the function pointer doesn't use reference types
+ if (FuncTy->getReturnType().isWebAssemblyReferenceType()) {
+ return Diag(
+ FuncPtrArg->getBeginLoc(),
+ diag::err_wasm_builtin_test_fp_sig_cannot_include_reference_type)
+ << 0 << FuncTy->getReturnType() << FuncPtrArg->getSourceRange();
+ }
+ auto NParams = FuncTy->getNumParams();
+ for (unsigned I = 0; I < NParams; I++) {
+ if (FuncTy->getParamType(I).isWebAssemblyReferenceType()) {
+ return Diag(
+ FuncPtrArg->getBeginLoc(),
+ diag::
+ err_wasm_builtin_test_fp_sig_cannot_include_reference_type)
+ << 1 << FuncPtrArg->getSourceRange();
+ }
+ }
+
+ // Set return type to int (the result of the test)
+ TheCall->setType(getASTContext().IntTy);
+
+ return false;
+}
+
bool SemaWasm::CheckWebAssemblyBuiltinFunctionCall(const TargetInfo &TI,
unsigned BuiltinID,
CallExpr *TheCall) {
@@ -249,6 +296,8 @@ bool SemaWasm::CheckWebAssemblyBuiltinFunctionCall(const TargetInfo &TI,
return BuiltinWasmTableFill(TheCall);
case WebAssembly::BI__builtin_wasm_table_copy:
return BuiltinWasmTableCopy(TheCall);
+ case WebAssembly::BI__builtin_wasm_test_function_pointer_signature:
+ return BuiltinWasmTestFunctionPointerSignature(TheCall);
}
return false;
diff --git a/clang/test/CodeGen/builtins-wasm.c b/clang/test/CodeGen/builtins-wasm.c
index d8aff82b0c140..d52f5f88ed8cb 100644
--- a/clang/test/CodeGen/builtins-wasm.c
+++ b/clang/test/CodeGen/builtins-wasm.c
@@ -751,3 +751,27 @@ void *tp (void) {
return __builtin_thread_pointer ();
// WEBASSEMBLY: call {{.*}} @llvm.thread.pointer.p0()
}
+
+
+typedef void (*funcref_t)();
+typedef int (*funcref_int_t)(int);
+typedef float (*F1)(float, double, int);
+typedef int (*F2)(float, double, int);
+typedef int (*F3)(int, int, int);
+typedef void (*F4)(int, int, int);
+typedef void (*F5)(void);
+
+void use(int);
+
+void test_function_pointer_signature_void(F1 func) {
+ // WEBASSEMBLY: %0 = tail call i32 (ptr, ...) @llvm.wasm.ref.test.func(ptr %func, float 0.000000e+00, token poison, float 0.000000e+00, double 0.000000e+00, i32 0)
+ use(__builtin_wasm_test_function_pointer_signature(func));
+ // WEBASSEMBLY: %1 = tail call i32 (ptr, ...) @llvm.wasm.ref.test.func(ptr %func, i32 0, token poison, float 0.000000e+00, double 0.000000e+00, i32 0)
+ use(__builtin_wasm_test_function_pointer_signature((F2)func));
+ // WEBASSEMBLY: %2 = tail call i32 (ptr, ...) @llvm.wasm.ref.test.func(ptr %func, i32 0, token poison, i32 0, i32 0, i32 0)
+ use(__builtin_wasm_test_function_pointer_signature((F3)func));
+ // WEBASSEMBLY: %3 = tail call i32 (ptr, ...) @llvm.wasm.ref.test.func(ptr %func, token poison, i32 0, i32 0, i32 0)
+ use(__builtin_wasm_test_function_pointer_signature((F4)func));
+ // WEBASSEMBLY: %4 = tail call i32 (ptr, ...) @llvm.wasm.ref.test.func(ptr %func, token poison)
+ use(__builtin_wasm_test_function_pointer_signature((F5)func));
+}
diff --git a/clang/test/Sema/builtins-wasm.c b/clang/test/Sema/builtins-wasm.c
index 31e5291d3ae5e..a3486b1aedb13 100644
--- a/clang/test/Sema/builtins-wasm.c
+++ b/clang/test/Sema/builtins-wasm.c
@@ -54,3 +54,27 @@ void test_table_copy(int dst_idx, int src_idx, int nelem) {
__builtin_wasm_table_copy(table, table, dst_idx, src_idx, table); // expected-error {{5th argument must be an integer}}
__builtin_wasm_table_copy(table, table, dst_idx, src_idx, nelem);
}
+
+typedef void (*F1)(void);
+typedef int (*F2)(int);
+typedef int (*F3)(__externref_t);
+typedef __externref_t (*F4)(int);
+
+void test_function_pointer_signature() {
+ // Test argument count validation
+ (void)__builtin_wasm_test_function_pointer_signature(); // expected-error {{too few arguments to function call, expected 1, have 0}}
+ (void)__builtin_wasm_test_function_pointer_signature((F1)0, (F2)0); // expected-error {{too many arguments to function call, expected 1, have 2}}
+
+ // // Test argument type validation - should require function pointer
+ (void)__builtin_wasm_test_function_pointer_signature((void*)0); // expected-error {{used type 'void *' where function pointer is required}}
+ (void)__builtin_wasm_test_function_pointer_signature((int)0); // expected-error {{used type 'int' where function pointer is required}}
+ (void)__builtin_wasm_test_function_pointer_signature((F3)0); // expected-error {{not supported for function pointers with a reference type parameter}}
+ (void)__builtin_wasm_test_function_pointer_signature((F4)0); // expected-error {{not supported for function pointers with a reference type return value}}
+
+ // // Test valid usage
+ int res = __builtin_wasm_test_function_pointer_signature((F1)0);
+ res = __builtin_wasm_test_function_pointer_signature((F2)0);
+
+ // Test return type
+ _Static_assert(EXPR_HAS_TYPE(__builtin_wasm_test_function_pointer_signature((F1)0), int), "");
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
sbc100
left a comment
There was a problem hiding this comment.
Nice! I think we might be able to use this in a few places in emscripten, to avoid jumping through JS.
Kind of interesting that this only takes a single argument.. intuitively I would expect two, but I guess this makes sense.
Would the name __builtin_wasm_test_function_pointer_type also make sense? I'm not sure about type or signature is commonly used here.
…#150116) PR #147486 broke the sanitizer and expensive-checks buildbot. These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. This removes the unneeded captures and should fix the sanitizer-buildbot. I also fixed the codegen in the wasm64 target: table.get requires an i32 but in wasm64 the function pointer is an i64. We need an additional `i32.wrap_i64` to convert it. I also added `-verify-machineinstrs` to the tests so that the test suite validates this fix. Finally, I noticed that #150201 uses a feature of the intrinsic that is not covered by the tests, namely `ptr` arguments. So I added one additional test case to ensure that it works properly. cc @dschuff
|
I'm neutral on the name, happy to change it if you prefer. |
|
|
|
I'd also like to add |
Where would the return value go here? Would end up in &success? What happens if none of the signatures match? Could you write this in terms of |
|
My thought is that it returns the return value. If one of the signatures matches it sets |
How would it work for functions that return different types? i.e. how would |
|
It seems that builtins can do that. In |
Thats seems really magical.. |
|
Well it would also be possible to make success the return value, and make the first argument a |
|
Other builtins with dynamic return type:
|
|
This change looks fine. |
|
If this change looks good can we merge it? |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24106 Here is the relevant piece of the build log for the reference |
|
Thanks again @dschuff ! |
…llvm#150116) PR llvm#147486 broke the sanitizer and expensive-checks buildbot. These captures were needed when toWasmValType emitted a diagnostic but are no longer needed since we changed it to an assertion failure. This removes the unneeded captures and should fix the sanitizer-buildbot. I also fixed the codegen in the wasm64 target: table.get requires an i32 but in wasm64 the function pointer is an i64. We need an additional `i32.wrap_i64` to convert it. I also added `-verify-machineinstrs` to the tests so that the test suite validates this fix. Finally, I noticed that llvm#150201 uses a feature of the intrinsic that is not covered by the tests, namely `ptr` arguments. So I added one additional test case to ensure that it works properly. cc @dschuff
llvm#150201) Tests if the runtime type of the function pointer matches the static type. If this returns false, calling the function pointer will trap. Uses `@llvm.wasm.ref.test.func` added in llvm#147486. Also adds a "gc" wasm feature to gate the use of the ref.test instruction.
| if (Feature == "-gc") { | ||
| HasGC = false; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When we add new feature like this don't we normally add them to WebAssemblyTargetInfo::initFeatureMap in addBleedingEdgeFeatures.. I would also expect the test for bleeding edge CPU to be updated, right?
There was a problem hiding this comment.
the test for llvm/test/CodeGen/WebAssembly/target-features-cpus.ll below does seem to include gc in bleeding-edge. But you're right, that we've been also adding things with addBleedingEdgeFeatures so it's weird that that test changes. We should look into what's going on there.
…ures (#151107) See suggestion here: llvm/llvm-project#150201 (comment)
Also alphebetize feature list, add `-mgc` and `-mno-gc` flags, and add some missing feature tests. Reland of llvm#151107. llvm#150201 (comment)
Also alphebetize feature list, add `-mgc` and `-mno-gc` flags, and add some missing feature tests. Reland of #151107. #150201 (comment)
…ures (#151294) Also alphebetize feature list, add `-mgc` and `-mno-gc` flags, and add some missing feature tests. Reland of #151107. llvm/llvm-project#150201 (comment)
…or Emscripten trampolines Since llvm/llvm-project#150201 was merged, there is now a better way to do this. Requires Emscripten 4.0.12.
…or Emscripten trampolines Since llvm/llvm-project#150201 was merged, there is now a better way to do this. Requires Emscripten 4.0.12.
…cripten trampoline (#137470) With llvm/llvm-project#150201 being merged, there is now a better way to generate the Emscripten trampoline, instead of including hand-generated binary WASM content. Requires Emscripten 4.0.12.
…or Emscripten trampoline (pythonGH-137470) With llvm/llvm-project#150201 being merged, there is now a better way to generate the Emscripten trampoline, instead of including hand-generated binary WASM content. Requires Emscripten 4.0.12. (cherry picked from commit 2629ee4) Co-authored-by: Hood Chatham <[email protected]>
…for Emscripten trampoline (GH-137470) (#139039) gh-128627: Use __builtin_wasm_test_function_pointer_signature for Emscripten trampoline (GH-137470) With llvm/llvm-project#150201 being merged, there is now a better way to generate the Emscripten trampoline, instead of including hand-generated binary WASM content. Requires Emscripten 4.0.12. (cherry picked from commit 2629ee4) Co-authored-by: Hood Chatham <[email protected]>
Tests if the runtime type of the function pointer matches the static type. If this returns false, calling the function pointer will trap. Uses
@llvm.wasm.ref.test.funcadded in #147486.Also adds a "gc" wasm feature to gate the use of the ref.test instruction.