wasi preview 2 support

[dgryski]

trafficstars

This PR adds -target=wasip2 support to TinyGo. It no longer depends on wasi-libc, but. has its own miniature libc that turns the calls into appropriate component calls.

[aykevl]

Interesting! Suggestion: use -target=wasip2 to more clearly specify the WASI version.

[dgryski]

Now we just need to unstub the 29 calls in libc_wasip2.go with Go implementations of the associated C calls using preview2 calls under the hood.

[ydnar]

@dgryski the HOWTO in this PR might need to be updated to reflect the new build steps (and no need for a Preview 1 adapter!):

tinygo build -target=wasip2 -x -o main.wasm ./cmd/wasip2-test
wasm-tools component embed -w wasi:cli/command $(tinygo env TINYGOROOT)/lib/wasi-cli/wit/ main.wasm -o embedded.wasm
wasm-tools component new embedded.wasm -o component.wasm
wasmtime run --wasm component-model component.wasm

[dgryski]

This is now in a state where it's roughly equivalent to wasip1 support. I still plan to add networking support, but that can come in a later PR.

[deadprogram]

Seems like this PR has some actual fail on the CI https://github.com/tinygo-org/tinygo/actions/runs/8623266604/job/23785693630

[deadprogram]

@dgryski this PR has a merge conflict that needs to be resolved, if you please.

[dgryski]

Just rebased against dev with no conflicts; let see what happes...

[deadprogram]

@dgryski looks like actual test failure of some kind here https://github.com/tinygo-org/tinygo/actions/runs/8788784842/job/24119419435#step:18:36

[deadprogram]

@dgryski please see this error:

main.go:1344: error: wasm-tools failed: exec: "wasm-tools": executable file not found in $PATH

[dgryski]

@deadprogram Yup, I'm looking at the linux.yml workflow now..

[dgryski]

@aykevl PTAL. I think we've addressed everything.

[deadprogram]

@dgryski see https://github.com/tinygo-org/tinygo/pull/4027/files#r1589100941

[deadprogram]

@aykevl any other comments before we merge?

[dgryski]

Is there anything else left unaddressed before we merge?

[aykevl]

@dgryski yes there is, namely with //go:wasmimport. We are currently incompatible with the upstream proposal for loosening //go:wasmimport:

• The upstream proposal doesn't allow passing structs as-is.
• Also, the proposal now requires structs.HostLayout markers in every struct to ensure they match the underlying platform. The only thing TinyGo needs to do here is to check that the field is present: I haven't checked specifically but I'm pretty sure we already match the Canonical ABI for the in-memory layout.
• We don't validate whether struct fields are allowed. So for example the following code would be allowed, even though it clearly is invalid:

  //go:wasmimport foo bar
  func bar(x *struct{a func()})
  

We could decide to deviate from the upstream proposal (especially when it comes to passing structs by value) but if we do that I'd like to see at least a good explanation for that (and the comment should be more than just "this reflects the relaxed type restrictions proposed here" because it doesn't).

[deadprogram]

@dgryski can you please rebase against the latest dev so this branch can build using LLVM 18?

[dgryski]

Rebased against dev and squashed.

[dgryski]

@aykevl So what's the easiest set of short-term hacks to get this merged? Merge with a note indicating the current incompatibilities but that we will update it to match upstream Go before the next Go release that properly includes support for wasip2?

[deadprogram]

@dgryski please rebase against dev again and correct the merge conflict.

Merge with a note indicating the current incompatibilities but that we will update it to match upstream Go before the next Go release that properly includes support for wasip2?

IMO if we do that, we can merge this right now. wasi preview in the naming even is a signal to developers that they may have to deal with at least some breaking changes along the way. And as long as we are compatible with the lastest shipping Go releases then we have done the correct thing.

[deadprogram]

@dgryski what did you think about my last request/comment?

[dgryski]

@deadprogram Yeah, I think that's fine. Big Go doesn't have wasip2 support yet, so we're leading the charge here. There are proposals (some of whic have been accepted) but we're still in unknown territory. "Nobody" should be using this yet for any real workload.

[aykevl]

@dgryski but you said yourself that passing structs wasn't going to work with WASI and that more work was needed to lower to primitive types?

What about a different solution: the relaxed //go:wasmimport rules are reverted and instead the specific packages in-tree (internal/wasi/*) are whitelisted to allow any type?

To be clear, it's these two lines that are a problem:

	case *types.Struct:
		return true

This needs to be more strict. We can't just pass any struct, because there is no defined ABI for passing structs and there is no checking whether these fields are allowed. The other changes (loosening integers and pointers) seem safe to me.

So what's the easiest set of short-term hacks to get this merged? Merge with a note indicating the current incompatibilities but that we will update it to match upstream Go before the next Go release that properly includes support for wasip2?

The relaxed constraints of //go:wasmimport aren't limited to wasip2, but apply to any wasm target. So then we'd be allowing things now and possibly breaking programs again in the future once we (inevitably) restrict things again. A short term hack would be to revert these changes to //go:wasmimport and instead disable checking for internal/wasi/* packages (they're internal so we can do with them whatever we want).

[deadprogram]

@dgryski @aykevl from what I can tell, all of the outstanding comments have been addressed, the branch is rebased, and is passing all tests.

Are we ready to merge?

[dgryski]

@deadprogram I still need to merge @ydnar 's https://github.com/dgryski/tinygo/pull/16 PR and rebase, then I think we're good.

[aykevl]

I'd like to take another look once that PR is merged, but initially it seems to resolve most of my concerns.

[deadprogram]

@aykevl please take a look now.

[deadprogram]

@dgryski please resolve merge conflicts. Hopefully for the last time!

[rajatjindal]

hello folks, thank you for your amazing work on this project as well as this feature. 👍

I was trying out the latest changes in this branch, and ran into a problem that I thought I will mention here.

when building a project for target wasip2, I am getting following error:

# os
/Users/rajatjindal/go/src/github.com/tinygo-org/tinygo/build/release/tinygo/src/os/stat_unix.go:28:6: statNolog redeclared in this block
/Users/rajatjindal/go/src/github.com/tinygo-org/tinygo/build/release/tinygo/src/os/stat_other.go:15:6:  other declaration of statNolog
/Users/rajatjindal/go/src/github.com/tinygo-org/tinygo/build/release/tinygo/src/os/stat_unix.go:41:6: lstatNolog redeclared in this block
/Users/rajatjindal/go/src/github.com/tinygo-org/tinygo/build/release/tinygo/src/os/stat_other.go:20:6:  other declaration of lstatNolog
/Users/rajatjindal/go/src/github.com/tinygo-org/tinygo/build/release/tinygo/src/os/stat_unix.go:15:16: method File.Stat already declared at /Users/rajatjindal/go/src/github.com/tinygo-org/tinygo/build/release/tinygo/src/os/stat_other.go:10:16

which I was able to fix by making following change to stat_other.go:

diff --git a/src/os/stat_other.go b/src/os/stat_other.go
index d3e0af6e..234c59db 100644
--- a/src/os/stat_other.go
+++ b/src/os/stat_other.go
@@ -1,4 +1,4 @@
-//go:build baremetal || (tinygo.wasm && !wasip1)
+//go:build baremetal || (tinygo.wasm && !wasip1 && !wasip2)
 
 // Copyright 2016 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style

could you please confirm if that is the right fix (and if so, if you could pls include that fix in changes) or kindly point me in the right direction.

thank you

[dgryski]

@rajatjindal Thanks. Those build tags must have broken on one my rebases. Fixed now.

[ydnar]

I did an in-depth review and it looks good to me.

This is probably far too late (I don't want to delay this PR any further), but what about not checking in all those generated files? Alternatives:

• generating the files on build, like with gen-device
• putting those files in a separate repository

It's not a big issue, but I feel like adding so many generated files in the repo leads to churn. But maybe that's just me.

The generated files are part of (and compiled with every program that uses) the standard library. I guess we could generate them in CI and build. We’d need to add Rust dependencies (wasm-tools), and the standard library wouldn’t build for GOOS=wasip2 unless those files are present.

On the other hand, the diff history of the generated files might be useful over time.

Do you have an opinion on whether package cm should be copied into src/vendor, rather than a submodule?