cmd/link: on wasm, number of functions limited to 2^16 #64856
Comments
HarikrishnanBalagopal
changed the title
HarikrishnanBalagopal
changed the title
seankhliao
changed the title
gopherbot
added
the
compiler/runtime
seankhliao
added
NeedsInvestigation
|
/cc @golang/wasm |
|
After some investigation this appears to be a bug with 1.22. I'm able to compile this without any errors with go 1.21. It happens for both This error prevents the compilation of large wasm programs with Go 1.22 without a workaround. /cc @golang/release |
@mauri870 I don't think the bug is only in v1.22 because we have not tried v1.22 yet. The errors we saw are all with Go v1.21 The line I linked in the first post is still there in the go/src/cmd/link/internal/wasm/asm.go Line 157 in 2184a39 The git blame shows that line has been there from the beginning when WASM got added f41dc71#diff-b189a5a4d6f377939c502fb248c71ab0f1febe60a54f283eaca1dad73c196604R85 WorkingBuilding the Non-workingAfter adding the DifferenceThe difference is that we added the |
|
Thanks, but I still can't reproduce on 1.21 for some reason |
I might be confused but the output you have shown includes the compile error. Are you trying to reproduce something else? |
|
Sorry, I think I was not clear enough. With 1.21.5 it built just fine, the error you see is for gotip(1.22). |
hmm, that's weird the line I linked hasn't changed very much go/src/cmd/link/internal/wasm/asm.go Line 157 in 2184a39 Update@mauri870 I just updated to v1.21.5 and I am still getting the error. |
|
I managed to reproduce it consistently in go 1.21 now, thanks. I would not trust on my initial bisecting results because the repro was not very consistent. I was able to reproduce the issue with: git clone https://github.com/HarikrishnanBalagopal/move2kube.git -b feat/starlark
cd move2kube
GOOS=js GOARCH=wasm go1.21.5 buildSince 1.20 does not have the wasip1 port, I tried with GOOS=js and it compiled just fine, so it appears to not be affected. It is unclear to me where the issue is, but since programs compile just fine in Go 1.20 and we have the same function size in 1.20, 1.21 and tip something changed between 1.20 and 1.21 that caused the error to appear. @gopherbot please backport to Go 1.21, this is a linker issue when compiling large wasm programs without a workaround. |
|
Backport issue(s) opened: #64867 (for 1.20), #64868 (for 1.21). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
|
I tried bisecting again and it points to CL 484856. I tried the parent commit and it works fine, but fails on the CL commit. I have honestly no idea why this change would cause relocations to overflow in large wasm programs. cc @randall77 @dr2chase (sorry about the double cc, I posted in the backport issue before by mistake) |
This line does not limit a WASM function's size to 65536 bytes. In our WASM backend, the "PC" is an index of a basic block, not a byte count or instruction count. To overflow that limit, the function needs to have more than 65536 basic blocks, which is possible but quite rare. |
hmm, the comment above that line makes it seem like it's a regular PC (but calculated from 2 values
|
|
It is not a regular PC. I'll look into what the cause of the error is. I'm not sure yet, but it is probably not the basic block number limit. |
|
It is not the basic block number limit, but the total number of (reachable) functions. As you saw in the comment above, We might be able to change the encoding scheme. E.g. we could change the method table to encode only the cc @neelance |
|
Change https://go.dev/cl/552835 mentions this issue: |
|
CL https://go.dev/cl/552835 changes the method table encoding to allow more than 65536 functions. Now https://github.com/HarikrishnanBalagopal/move2kube/tree/feat/starlark builds. Could you test if the resulting binary is correct? Thanks. I don't really like the CL, as it adds more special case for Wasm. But maybe it is fine... |
cherrymui
changed the title
cherrymui
changed the title
Is it necessary to keep the size to 32bit? The line shows an explicit cast to int64 go/src/cmd/link/internal/wasm/asm.go Line 90 in 649671b Both the input va and the return value are also uint64go/src/cmd/link/internal/wasm/asm.go Lines 91 to 92 in 649671b Since the WASM functions live in a separate address space to the WASM linear memory, it should not be affected by the same constraints when it comes to address bit size. |
Do I need to fetch the Update 1I was able to build the Go compiler with the changes. I was able to build the WASM app with the new compiler This is the compressed WASM binary Update 2@cherrymui Running the new WASM binary gives a very large stack trace and a fatal error at the end. This is the partial stack trace that we were able to capture https://gist.github.com/HarikrishnanBalagopal/80ca5e1d359ffdafd1692700b3370168 Steps:
Update 3@cherrymui So with the new compiler changes, even the https://gist.github.com/HarikrishnanBalagopal/d59b6f5a724dc49e207de967f2394e84 Update 4Managed to capture the beginning of the stack trace: |
|
@cherrymui The panic seems to be coming from Line 546 in 26ba75f and Line 553 in 26ba75f |
|
Thanks for trying the code and posting the error message. It seems the function table will need a similar update. I'll look into that.
Yes. It is not |
Thanks, please let me know when you make the changes to the function table and I will test them out as well. |
|
Any update on this? |
|
I was OOO last week and I'm just back. I'll work on the function table change. Thanks. |
Np, hope you had a good vacation. Please let me know if there's any update on this. |
|
Hi! Any update on this? now that v1.22 has been released. |
|
Not yet. Still working on it. I hope to get a new version of the fix by this week. |
|
@HarikrishnanBalagopal could you try the new version of CL https://go.dev/cl/552835 ? Thanks. |
Sorry, I went on vacation myself 😅 I will try it tomorrow and update here. Update 1@cherrymui I have tried the change and it failed to build the wasm webapp.
$ go version
go version go1.22.0 darwin/arm64
$ git remote -v
origin https://go.googlesource.com/go (fetch)
origin https://go.googlesource.com/go (push)
$ git log
commit 06039ae75fa4505de13184f520fe1c6bd0bb7bc9 (HEAD -> change-552835, tag: change-patchset-5)
Author: Cherry Mui <cherryyz@google.com>
Date: Tue Dec 26 15:35:56 2023 -0500
cmd/link, runtime: put only function index in method table and func table
.................
$ cd src/ && ./make.bash
Building Go cmd/dist using /opt/homebrew/Cellar/go/1.22.0/libexec. (go1.22.0 darwin/arm64)
Building Go toolchain1 using /opt/homebrew/Cellar/go/1.22.0/libexec.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for darwin/arm64.
---
Installed Go for darwin/arm64 in /Users/haribala/Documents/code/remote/go.googlesource.com/go
Installed commands in /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin
*** You need to add /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin to your PATH.
$ /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin/go version
go version devel go1.23-06039ae75f Thu Feb 15 13:53:32 2024 -0500 darwin/arm64
$ git log
commit 95c7fb33c04bb31d3d5e25a8553930693de86194 (HEAD -> feat/starlark, origin/feat/starlark)
Author: Harikrishnan Balagopal <harikrishmenon@gmail.com>
Date: Tue Dec 26 15:04:57 2023 +0530
feat: add the starlark transformer
.................
$ CGO_ENABLED=0 GOOS=wasip1 GOARCH=wasm /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin/go build -ldflags ' -X github.com/konveyor/move2kube-wasm/types/info.buildmetadata=unreleased -X github.com/konveyor/move2kube-wasm/types/info.gitCommit=95c7fb33c04bb31d3d5e25a8553930693de86194 -X github.com/konveyor/move2kube-wasm/types/info.gitTreeState=clean' -o "./bin/move2kube.wasm" .
# github.com/konveyor/move2kube-wasm
github.com/mholt/archiver/v3.(*Rar).Close.wrapinfo: non-pc-relative relocation R_ADDROFF address for github.com/mholt/archiver/v3.(*Rar).Close is too big: 0x1007b0000
github.com/mholt/archiver/v3.(*Tar).Close.wrapinfo: non-pc-relative relocation R_ADDROFF address for github.com/mholt/archiver/v3.(*Tar).Close is too big: 0x100a80000
github.com/mholt/archiver/v3.(*Zip).Close.wrapinfo: non-pc-relative relocation R_ADDROFF address for github.com/mholt/archiver/v3.(*Zip).Close is too big: 0x101420000
$ echo $?
1The wasm webapp build fails now. With the previous change we were able to build the wasm webapp at least #64856 (comment) One difference is that I used Update 2The new changed Go compiler $ CGO_ENABLED=0 GOOS=wasip1 GOARCH=wasm /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin/go build -ldflags ' -X github.com/konveyor/move2kube-wasm/types/info.buildmetadata=unreleased -X github.com/konveyor/move2kube-wasm/types/info.gitCommit=dec9d8da6e5f8882aacb46a8e5f784b96b42a3c6 -X github.com/konveyor/move2kube-wasm/types/info.gitTreeState=clean' -o "./bin/move2kube.wasm" .
$ echo $?
0Update 3@cherrymui I think there's a regression with v1.22.0 of Golang #65786 |
|
@cherrymui Please let me know if you need any more details. |
|
@HarikrishnanBalagopal thanks for the update. Could you try the latest version of CL https://go.dev/cl/552835 ? Thanks. |
Update 1Tested patchset 7 commit $ go version
go version go1.21.7 darwin/arm64
$ git remote -v
origin https://go.googlesource.com/go (fetch)
origin https://go.googlesource.com/go (push)
$ git log
commit 21cb3991a2076762670019b4e3c808ca38428281 (HEAD -> patchset-7)
Author: Cherry Mui <cherryyz@google.com>
Date: Tue Dec 26 15:35:56 2023 -0500
cmd/link, runtime: on Wasm, put only function index in method table and func table
In the type descriptor's method table, it contains relative PCs of
the methods (relative to the start of the text section) stored as
32-bit offsets. On Wasm, a PC is PC_F<<16 + PC_B, where PC_F is
the function index, and PC_B is the block index. When there are
more than 65536 functions, the PC will not fit into 32-bit (and
relative to the section start doesn't help). Since there are no
more bits for the function index, and the method table always
targets the entry of a method, we put just the PC_F there, and
rewrite back to a full PC at run time when we need the PC. This
way we can have more than 65536 functions.
The func table also contains 32-bit relative PCs, and it also
always points to function entries. Do the same there, as well
as other places where we use relative text offsets.
Also add the relocation type in the relocation overflow error
message.
Also add check for function too big on Wasm. If a function has
more than 65536 blocks, PC_B will overflow and PC = PC_F<<16 + PC_B
will points to the wrong function.
Fixes #64856.
Change-Id: If9c307e9fb1641f367a5f19c39f88f455805d0bb
................
$ git status
On branch patchset-7
nothing to commit, working tree clean
$ cd src/ && ./make.bash
Building Go cmd/dist using /opt/homebrew/Cellar/go@1.21/1.21.7/libexec. (go1.21.7 darwin/arm64)
Building Go toolchain1 using /opt/homebrew/Cellar/go@1.21/1.21.7/libexec.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for darwin/arm64.
---
Installed Go for darwin/arm64 in /Users/haribala/Documents/code/remote/go.googlesource.com/go
Installed commands in /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin
*** You need to add /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin to your PATH.
$ echo $?
0Building the wasm webapp $ /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin/go version
go version devel go1.23-21cb3991a2 Wed Feb 28 15:05:12 2024 -0500 darwin/arm64
$ git log
commit 95c7fb33c04bb31d3d5e25a8553930693de86194 (HEAD, origin/feat/starlark, feat/starlark)
Author: Harikrishnan Balagopal <harikrishmenon@gmail.com>
Date: Tue Dec 26 15:04:57 2023 +0530
feat: add the starlark transformer
chore: update dependencies
fix: issue with string not implementing a starlark interface
Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
.......................
$ CGO_ENABLED=0 GOOS=wasip1 GOARCH=wasm /Users/haribala/Documents/code/remote/go.googlesource.com/go/bin/go build -ldflags ' -X github.com/konveyor/move2kube-wasm/types/info.buildmetadata=unreleased -X github.com/konveyor/move2kube-wasm/types/info.gitCommit=95c7fb33c04bb31d3d5e25a8553930693de86194 -X github.com/konveyor/move2kube-wasm/types/info.gitTreeState=clean' -o "./bin/move2kube.wasm" .
$ echo $?
0Update 2At runtime there is an error Update 3The runtime error is on the end
local.get $var2
global.set $global1
local.get $var2
i32.wrap_i64
i64.load
local.get $var1
i32.const 8
i32.sub
local.tee $var1
global.set $global0
local.get $var1
i64.const 338362393
i64.store
i32.wrap_i64
i32.const 16
i32.shr_u
local.set $var0
i32.const 0
local.get $var0
call_indirect (param i32) (result i32)
global.get $global0
local.set $var1
br_if $label71
end $label10@cherrymui This line Here is the compiled wasm file (gzipped) btw Update 4The exact same runtime error occurs with the Since the |
|
Change https://go.dev/cl/567896 mentions this issue: |
|
Thanks for the update. The problem is this code: we extract the function index from the "PC". But we first truncate it to 32-bit then shift by 16 bits, so we only have 16 bits for the function index, which overflows. The fix is to do the shift first, then truncate. Wrote CL https://go.dev/cl/567896 for this. Could you try CL https://go.dev/cl/567896 and CL https://go.dev/cl/552835 together? You'll need both CLs. The easiest way is to check out the latter |
I was OOF past weeks, I will be back next week. Will try the change and post an update in the next few days. |
|
@cherrymui Sorry for the late reply, been a bit busy with work. I have tried the commit and it's failing with a panic. Golang repoMove2Kube repoMove2Kube WASM outputMove2Kube CodeThe panic seems to be on the UpdateIt fails with a panic even with a simple Update 2@cherrymui I managed to get an error about an invalid symbol table Full log |
Overview
It seems to me that the size of a WASM function is limited to 2^16 = 65536 bytes?
go/src/cmd/link/internal/wasm/asm.go
Line 90 in 649671b
This leads to a bunch of errors whenever using more than a few libraries in our WASM app
The code for our app is available in this branch https://github.com/konveyor/move2kube/blob/wasm/go.mod
The code is able to build using the official Go compiler WASIP1 support
https://github.com/konveyor/move2kube/blob/dec9d8da6e5f8882aacb46a8e5f784b96b42a3c6/Makefile#L45C2-L45C100
Issue
When we try to add the https://pkg.go.dev/go.starlark.net/starlark library then we get a bunch of compile errors
Fix
Please remove this arbitrary limit on function size or at least provide a way to work around it. This is a severe limit on any real world app that will use many different libraries to implement various features.
The WASM page size is 65536 bytes https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/Memory/Memory which is 2^16 and can be addressed with 16 bits. That might explain where this particular limit came from but it doesn't explain why a single function has to fit all inside of one WASM page. Especially since WASM functions live is a completely separate address space than the WASM linear memory.
Also see these comments:
Related
https://stackoverflow.com/questions/67294859/why-golang-limit-the-symbol-number-to-65535-while-compile-wasm-target
#7769
#7980
WebAssembly/design#1138
The text was updated successfully, but these errors were encountered: