Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cmd/link: need to add buildids to wasm modules #25910

Closed
bradfitz opened this issue Jun 15, 2018 · 3 comments
Closed

cmd/link: need to add buildids to wasm modules #25910

bradfitz opened this issue Jun 15, 2018 · 3 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.11

Comments

@bradfitz
Copy link
Contributor

dist is failing for js/wasm right now (when run via make.bash, etc) due to missing buildids:

https://build.golang.org/log/4dfe5bd474996d98e85d77bfa4f63b9713b2ccbe

Repro:

$ GOOS=js GOARCH=wasm ./make.bash

@rsc says:

the reason things look stale is that cmd/go recomputes what the expected build id is and then looks in the binary to see if that's the recorded buildid. if so, not stale. if not, then it's stale
go tool buildid $GOROOT/pkg/tool/js_wasm/dist
prints an empty string
looks like the build id is not in the binary
...
what we do on systems with inflexible object formats (like darwin and windows) is arrange for the build id to appear early in the binary (in teh first few kilobytes) as string data
and we search for a magic indicator to find it.
src/cmd/link/internal/ld/data.go textbuildid
so wasm should do something like that too
we almost certainly want build ids generally. it's not just about dist
...
it's going to just keep rebuilding because it looks out of date
i mean, i don't think we'll run any of the binaries in pkg/js_wasm/tool
but still

/cc @neelance @ianlancetaylor
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. release-blocker arch-wasm WebAssembly issues labels Jun 15, 2018
@bradfitz
Copy link
Contributor Author

The linker's Link.textbuildid code is running and prepending to ctx.Textp, and then cmd/link/internal/wasm/asm.go has code that looks for it:

        for i, fn := range ctxt.Textp {
                wfn := new(bytes.Buffer)
                if fn.Name == "go.buildid" {
                        writeUleb128(wfn, 0) // number of sets of locals                                                                                           
                        writeI32Const(wfn, 0)
                        wfn.WriteByte(0x0b) // end                                                                                                                 

                } else {

I think we need to note the string value there and add a wasm data section early in the module. Not sure it's valid to have multiple data sections, though. (I'd considered a name, but names must be UTF-8, and the buildid is intentionally invalid UTF-8)

@bradfitz
Copy link
Contributor Author

Ah, we can use a custom section. Perfect.

CL forthcoming.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/119175 mentions this issue: cmd/link: add buildid to wasm modules

@golang golang locked and limited conversation to collaborators Jun 15, 2019
@dmitshur dmitshur added this to the Go1.11 milestone Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
4 participants
@bradfitz @neelance @dmitshur @gopherbot