Skip to content

Impossible to use wasm:js-string functions in webassembly #136594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
socketpair opened this issue Apr 21, 2025 · 20 comments
Open

Impossible to use wasm:js-string functions in webassembly #136594

socketpair opened this issue Apr 21, 2025 · 20 comments
Labels
backend:WebAssembly enhancement Improving things as opposed to bug fixing, e.g. new or missing feature extension:clang

Comments

@socketpair
Copy link

Here you can see an example of WASM code how to use concat JS builtin:

https://developer.mozilla.org/en-US/docs/WebAssembly/Guides/JavaScript_builtins#wasm_module

(func $concat (import "wasm:js-string" "concat")
    (param externref externref) (result (ref extern)))

So, in C I defined it in the following way:

__externref_t js_concat(__externref_t, __externref_t) 
__attribute__((import_module("wasm:js-string"), import_name("concat")));

Next, I'm trying to run the code that uses definition above in the following way:

const { instance } = await WebAssembly.instantiateStreaming(
  fetch('trie_search.wasm'),
  {
      env: {  },
  },
  {
      builtins: ["js-string"],
      importedStringConstants: "Something",
  },
);
instance.exports.my_function();

Browser console says:

Uncaught (in promise) CompileError: WebAssembly.instantiateStreaming():
Imported builtin function "wasm:js-string" "concat" has incorrect signature

I guess, the problem in return value. in WAT language it should look like (ref extern). How to express it in C (Clang)?

https://github.com/WebAssembly/js-string-builtins/blob/main/proposals/js-string-builtins/Overview.md#wasmjs-string-concat
https://github.com/WebAssembly/function-references/blob/master/proposals/function-references/Overview.md#binary-format

(module
  (global $h (import "string_constants" "hello ") externref)
  (global $w (import "string_constants" "world!") externref)

  (func $concat (import "wasm:js-string" "concat")
    (param externref externref) (result (ref extern)))

  (func $log (import "m" "log") (param externref))
  (func (export "main")
    (call $log (call $concat (global.get $h) (global.get $w))))
)

Compiles perfectly well using binryen, but how to write an equivalent using C and clang ?

@EugeneZelenko EugeneZelenko added question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! backend:WebAssembly and removed new issue labels Apr 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 21, 2025

@llvm/issue-subscribers-backend-webassembly

Author: Коренберг Марк (socketpair)

Here you can see an example of WASM code how to use `concat` JS builtin:

https://developer.mozilla.org/en-US/docs/WebAssembly/Guides/JavaScript_builtins#wasm_module

(func $concat (import "wasm:js-string" "concat")
    (param externref externref) (result (ref extern)))

So, in C I defined it in the following way:

__externref_t js_concat(__externref_t, __externref_t) 
__attribute__((import_module("wasm:js-string"), import_name("concat")));

Next, I'm trying to run the code that uses definition above in the following way:

const { instance } = await WebAssembly.instantiateStreaming(
  fetch('trie_search.wasm'),
  {
      env: {  },
  },
  {
      builtins: ["js-string"],
      importedStringConstants: "Something",
  },
);
instance.exports.my_function();

Browser console says:

Uncaught (in promise) CompileError: WebAssembly.instantiateStreaming():
Imported builtin function "wasm:js-string" "concat" has incorrect signature

I guess, the problem in return value. in WAT language it should look like (ref extern). How to express it in C (Clang)?

https://github.com/WebAssembly/js-string-builtins/blob/main/proposals/js-string-builtins/Overview.md#wasmjs-string-concat
https://github.com/WebAssembly/function-references/blob/master/proposals/function-references/Overview.md#binary-format

(module
  (global $h (import "string_constants" "hello ") externref)
  (global $w (import "string_constants" "world!") externref)

  (func $concat (import "wasm:js-string" "concat")
    (param externref externref) (result (ref extern)))

  (func $log (import "m" "log") (param externref))
  (func (export "main")
    (call $log (call $concat (global.get $h) (global.get $w))))
)

Compiles perfectly well using binryen, but how to write an equivalent using C and clang ?

@sbc100
Copy link
Collaborator

sbc100 commented Apr 21, 2025

When you compile the code using clang, what is the type of the resulting import? I.e. what does wasm-dis to wasm-objdump say about the import?

@hoodmane
Copy link
Contributor

hoodmane commented Apr 21, 2025

I think the problem is that the return type of the import is ref extern (a non nullable reference to extern) whereas __externref_t is externref aka ref null extern. ref null extern is not expressible from C/assembly.

@hoodmane
Copy link
Contributor

It looks like test, charCodeAt, codePointAt, length, equals, and compare have types that make them usable from C/llvm asm whereas cast, fromCharCodeArray, intoCharCodeArray, fromCharCode, fromCodePoint, concat, and substring all have inexpressible types and would require an extra adapter to use.

@socketpair
Copy link
Author

@hoodmane who can work on the issue ? Possibly introduce a new built-in type ? Add a new attribute?

@socketpair
Copy link
Author

And also, please someone change tags for the issue. It's not a question. Actually, it is a bug or a feature request.

@dschuff dschuff added enhancement Improving things as opposed to bug fixing, e.g. new or missing feature extension:clang and removed question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! labels Apr 22, 2025
@dschuff
Copy link
Member

dschuff commented Apr 22, 2025

I don't think anyone is currently working on the extension you are using. Actually I'm not aware of anyone really using either, so I'm interested in what your use case is. Can you say more about that?

@socketpair
Copy link
Author

socketpair commented Apr 22, 2025

Okay. I just wanted to build logging system from wasm to js. I don't use emscripten, just bare clang and a memory manager. I can share sources, actually.

Now, I just export from js (and import in wasm) own concat function. And it works. I guess, importing builtin is better.

Actually, nothing special. But wasm support in Clang looks incomplete, just faced it. Why not to improve ? JS builtins is finalized standard. Why not to support it? I guess clang changes will be minimal.

Also, I would distribute in Clang additional header file with these definitions. For c++ I would add these functions to wasm::js_string namespace. In C, I would import as _js_string..... Or so.

@dschuff
Copy link
Member

dschuff commented Apr 22, 2025

Yeah; wasm support in clang is actually quite mature, but primarily for applications which only use linear memory (and therefore use C or C++ strings rather than JS strings). As you've seen there is some limited support for externref (which never really advanced beyond the experimental stage), but the use cases for externref are somewhat limited, since you can't really do anything with them in C/C++, you would just have to pass them back out to JS. So there just hasn't been a lot of user demand for this support, and therefore we've prioritized other things. Having said that, I don't think we'd be opposed to seeing it added. I think it would be reasonable to add something like __nullexternref_t for this; perhaps you could imagine going even further and adding a C type qualifier (like const or volatile) that would compose with externref, but I suspect that would be more involved in the frontend, and IIRC we don't support any other GC types yet, so it may not be worth it. And as you said, once we had that, it should just be a matter of declaring the JS string builtin functions in a header, to make it work.
As for the "who would work on it" part, I don't think we (i.e. the Google wasm tools folks) are likely to prioritize this soon, but if you wanted to give it a try, we (and maybe @pmatos ?) can probably help you with reviews and advice.

@hoodmane
Copy link
Contributor

It would be really great to have more of these types added to llvm assembly first. Then it would be possible to make your own wrappers for functions like this in assembly. It should be possible to cast everything to externref to pass it to C and downcast arguments from externref to the appropriate type. But even assembly level support is quite a lot of work and someone would have to actually do it.

@socketpair
Copy link
Author

socketpair commented Apr 23, 2025

Actually I don't quite understand what means nullable here. Is it a reference to JS null object ?

https://webassembly.github.io/gc/core/syntax/types.html#reference-types

Seems __nullexternref_t is non-nullable and existing __externref_t IS nullable.

So, the better name would be something like __nonnull_externref_t.

Also we need to cast from non-nullable type to nullable one. And possibly vice versa after checking for null.

And I'm just curious about nullablity:

Is it possible in C to check if __externref_t is null ? Is it possible to cast 0 (nullptr) to __externref_t ? Are JS Null, Undefined and Number(0) distinguishable?

How null for nullable type is interpreted in JS ? As Null or Undefined ?

@pmatos
Copy link
Contributor

pmatos commented Apr 23, 2025

I don't think we (i.e. the Google wasm tools folks) are likely to prioritize this soon, but if you wanted to give it a try, we (and maybe @pmatos ?) can probably help you with reviews and advice.

I can definitely help here, but I am unlikely to implement this myself in the near future. However, it shouldn't be a large patch and if you are interested in getting into Wasm LLVM, this might be a good issue to tackle.

@pmatos
Copy link
Contributor

pmatos commented Apr 23, 2025

Actually I don't quite understand what means nullable here. Is it a reference to JS null object ?

https://webassembly.github.io/gc/core/syntax/types.html#reference-types

Seems __nullexternref_t is non-nullable and existing __externref_t IS nullable.

So, the better name would be something like __nonnull_externref_t.

I think you're correct here, yes.

Also we need to cast from non-nullable type to nullable one. And possibly vice versa after checking for null.

And I'm just curious about nullablity:

Is it possible in C to check if __externref_t is null ? Is it possible to cast 0 (nullptr) to __externref_t ? Are JS Null, Undefined and Number(0) distinguishable?

At the moment, I don't think you can do anything like this in C. Externref is an opaque type which you can't do much with.

@socketpair
Copy link
Author

socketpair commented Apr 23, 2025

@pmatos

https://webassembly.github.io/spec/core/syntax/instructions.html#reference-instructions

Image

Seems we CAN cast nullptr to reference type, CAN check if it is null. And possibly (if I understand right) convert function pointer to funcref.

@pmatos
Copy link
Contributor

pmatos commented Apr 23, 2025

@pmatos

https://webassembly.github.io/spec/core/syntax/instructions.html#reference-instructions

Image

Actually we CAN cast nullptr to reference type, CAN check if it is null. And possibly (if I understand right) convert function pointer to funcref.

Ah. Yes, you're right. I forgot. Haven't worked on this for too long. I think I implemented is_null at least. Would have to check. Give me a few hours to get back to my laptop.

@hoodmane
Copy link
Contributor

@pmatos is_null does not seem to be a thing, the relevant builtins so far are:

__builtin_wasm_ref_null_extern
__builtin_wasm_ref_null_func
__builtin_wasm_table_copy
__builtin_wasm_table_fill
__builtin_wasm_table_get
__builtin_wasm_table_grow
__builtin_wasm_table_set
__builtin_wasm_table_size

So adding __builtin_wasm_ref_extern_is_null seems like a good starting point.

@socketpair
Copy link
Author

I'm not into, but I think comparison with nullptr looks natural. And for me, is better than calling Intrinsic

@pmatos
Copy link
Contributor

pmatos commented Apr 25, 2025

@pmatos is_null does not seem to be a thing, the relevant builtins so far are:

__builtin_wasm_ref_null_extern
__builtin_wasm_ref_null_func
__builtin_wasm_table_copy
__builtin_wasm_table_fill
__builtin_wasm_table_get
__builtin_wasm_table_grow
__builtin_wasm_table_set
__builtin_wasm_table_size

So adding __builtin_wasm_ref_extern_is_null seems like a good starting point.

The IR already has this at

def int_wasm_ref_is_null_extern :

so I just forgot to plug it into Clang. As mentioned, I can point someone to do it but I want be able to dedicate anytime at the moment to this.

@pmatos
Copy link
Contributor

pmatos commented Apr 25, 2025

I'm not into, but I think comparison with nullptr looks natural. And for me, is better than calling Intrinsic

The current implementation won't allow comparisons to null atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:WebAssembly enhancement Improving things as opposed to bug fixing, e.g. new or missing feature extension:clang
Projects
None yet
Development

No branches or pull requests

7 participants