-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
wasm: duplicate import in wasip1 compiled binary #60525
Comments
Also happening for me with this code The duplicate imports for (module
(type (;0;) (func (param i32) (result i32)))
(type (;1;) (func (result i32)))
(type (;2;) (func (param i32)))
(type (;3;) (func (param i32 i32) (result i32)))
(type (;4;) (func (param i32 i64 i32) (result i32)))
(type (;5;) (func (param i32 i32 i32 i32) (result i32)))
(type (;6;) (func (param i32 i32 i32 i64 i32) (result i32)))
(type (;7;) (func (param i32 i32 i32 i32 i32) (result i32)))
(type (;8;) (func (param i32 i32 i32 i32 i32 i64 i64 i32 i32) (result i32)))
(type (;9;) (func (param i32 i32 i32) (result i32)))
(type (;10;) (func (param i64 i64 i64 i64) (result i64)))
(type (;11;) (func (param i64 i64 i64) (result i64)))
(type (;12;) (func (param i64) (result i64)))
(type (;13;) (func (result i64)))
(type (;14;) (func))
(type (;15;) (func (param i64 i64) (result i64)))
(type (;16;) (func (param f64) (result i64)))
(import "wasi_snapshot_preview1" "sched_yield" (func (;0;) (type 1)))
(import "wasi_snapshot_preview1" "proc_exit" (func (;1;) (type 2)))
(import "wasi_snapshot_preview1" "args_get" (func (;2;) (type 3)))
(import "wasi_snapshot_preview1" "args_sizes_get" (func (;3;) (type 3)))
(import "wasi_snapshot_preview1" "clock_time_get" (func (;4;) (type 4)))
(import "wasi_snapshot_preview1" "environ_get" (func (;5;) (type 3)))
(import "wasi_snapshot_preview1" "environ_sizes_get" (func (;6;) (type 3)))
(import "wasi_snapshot_preview1" "fd_write" (func (;7;) (type 5)))
(import "wasi_snapshot_preview1" "random_get" (func (;8;) (type 3)))
(import "wasi_snapshot_preview1" "poll_oneoff" (func (;9;) (type 5)))
(import "wasi_snapshot_preview1" "fd_close" (func (;10;) (type 0)))
(import "wasi_snapshot_preview1" "fd_read" (func (;11;) (type 5)))
(import "wasi_snapshot_preview1" "fd_readdir" (func (;12;) (type 6)))
(import "wasi_snapshot_preview1" "fd_filestat_get" (func (;13;) (type 3)))
(import "wasi_snapshot_preview1" "fd_write" (func (;14;) (type 5)))
(import "wasi_snapshot_preview1" "path_filestat_get" (func (;15;) (type 7)))
(import "wasi_snapshot_preview1" "path_open" (func (;16;) (type 8)))
(import "wasi_snapshot_preview1" "fd_fdstat_get" (func (;17;) (type 3)))
(import "wasi_snapshot_preview1" "fd_fdstat_set_flags" (func (;18;) (type 3)))
(import "wasi_snapshot_preview1" "fd_prestat_get" (func (;19;) (type 3)))
(import "wasi_snapshot_preview1" "fd_prestat_dir_name" (func (;20;) (type 9)))
(func $go_buildid (type 0) (param i32) (result i32) Compiled with
The Golang code package main
import (
"os"
"github.com/sirupsen/logrus"
)
func must(err error) {
if err != nil {
panic(err)
}
}
func main() {
logrus.Infof("start")
pwd, err := os.Getwd()
logrus.Infof("pwd: '%s'", pwd)
must(err)
fs, err := os.ReadDir(".")
must(err)
for _, f := range fs {
logrus.Infof("f: %+v", f)
}
b, err := os.ReadFile("./dep.json")
must(err)
logrus.Infof("The contents are:\n%s", string(b))
logrus.Infof("done")
} The go.mod file
The go.sum file
|
This is likely known, but it appears to be related to the double import definition between os_wasip1.go and fs_wasip1.go. Line 74 in d90ce58
Line 225 in d90ce58
Line 78 in d90ce58
Line 273 in d90ce58
This surfaced for me when trying to adapt a wasip1 go module to a waspi2 component. There are likely other challenges to get to a p2 component but this was the first error. |
Good spot, we might be able to fix that by having the runtime call the syscall package import instead. |
I have a fix for this by using go linkname and having syscall calling the runtime fs write. This felt backwards but was a pattern I saw in exec_libc.go I can push a PR for reference. |
Using linkname between runtime and syscall is a time honored tradition. Please do submit a PR (or better, a CL to gerrit). Although shouldn't the dependency be the other way around? 🤔 |
Will do! That is what I thought as well - but I found a similar pattern in Line 60 in 5fdadff
It's a small change if it makes sense to update! working on the CL now. |
That link just points to the |
Yeah, that makes sense. The exec_libc.go appeared to be pointing to https://github.com/golang/go/blob/master/src/runtime/syscall_aix.go#L232 my naive thought from the above was to say the runtime os ( wasip1 ) had an import fd_write that was linked through write1 or something similar and that the syscall pkg would leverage the runtime impl for it - this does feel backward tho! |
Change https://go.dev/cl/629857 mentions this issue: |
https://go-review.googlesource.com/c/go/+/629857 - happy to update this to reflect the above - but pushed what I have working to help outline the path so far. |
I think it’s important to point out that a duplicate import is not an error, as there are valid cases for a Go program having duplicate (The contrary would require coordination between all WebAssembly programs and the standard library, which would overly restrict the language from adding new capabilities to the standard library that are not exposed as exported symbols, without breaking backwards compatibility.) That said:
|
Thanks, @ydnar - makes sense. If I am following, instead of the above solution using Is it valid for multiple imports to show in the wasm binary that has the same function signature? i.e
Somewhat related is a PR in the wasm-tools repo bytecodealliance/wasm-tools#1787 and a related issue bytecodealliance/wasm-tools#1786 This PR leads me to believe that duplicates are acceptable but should be handled by the compiler and resolved to a single declaration. If that is the case, I will need to dig deeper but I can start tracking through the My near-term goal is to get past the above issue to continue attempting to adapt a wasip1 module to a wasip2 component using an adapter. Transparently, I am approaching this very narrowly and learning along the way. I'm happy to continue iterating on the above - feel free to assign the issue to me. I'll be following regardless. I threw together an example of this here https://github.com/elewis787/go-wasm-imports. The gist from my understanding using |
I'm sorry, but what's the actual error? The original issue describes a duplicate import but not an error that this causes. If the wasip1 to wasip2 adapter step is failing, then the issue is likely with wasm-tools and not Go. |
@ydnar here is a quick write-up from my end. I agree, calling this an error is a stretch. I updated my verbiage above to call out the issue described vs error. The wasm module produced is validating correctly (albeit, IMO, technically malformed). Initial FindingsHere are the steps I look that lead me to this issue package main
import "fmt"
func main() {
fmt.Println("hello world")
} Compiled to wasip1/wasm We can observe the duplicate fd_write imports by converting to wat
Ignoring them, using the wasmtime adapter pattern, I attempt to make a component This results in:
The above error is how I found this issue and the related wasm-tools pr/issue. Removing Duplicate ImportsWhen using the However, using wasmtime to execute the component fails This results in:
Looking in GoI started looking at how the imports were defined after reading the comment on the wasm-tools PR about removing duplicates: bytecodealliance/wasm-tools#1787 (comment) |
What version of Go are you using (
go version
)?What did you do?
The imports in the generated .wat file are:
Both
fd_write
andrandom_get
appear twice.What did you expect to see?
Each import appears only once
This isn't an invalid wasm file and doesn't affect the end result much, but there is clearly some part of the compiler that isn't weeding out duplicate imports for wasm.
The text was updated successfully, but these errors were encountered: