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.

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/go: go version to support non-executable Go binaries #48187

Closed
qmuntal opened this issue Sep 4, 2021 · 14 comments
Closed

cmd/go: go version to support non-executable Go binaries #48187

qmuntal opened this issue Sep 4, 2021 · 14 comments
Labels
Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
Proposals (old)
Milestone
Go1.20

Comments

@qmuntal
Copy link
Contributor

qmuntal commented Sep 4, 2021

The go version command only supports executable files, restriction that is stated in its documentation:

Go version reports the Go version used to build each of the named executable files.

This restriction is implemented by checking for a .exe file extension on Windows and by checking if the file has 0111 permissions.

go/src/cmd/go/internal/version/version.go

Lines 107 to 112 in 4d66d77

func isExe(file string, info fs.FileInfo) bool {
if runtime.GOOS == "windows" {
return strings.HasSuffix(strings.ToLower(file), ".exe")
}
return info.Mode().IsRegular() && info.Mode()&0111 != 0
}

I propose that go version should also support non-executable binaries, such as binaries compiled using buildmode=c-shared and buildmode=c-archive, as these binaries can also contain useful version data.

See #45234 for a detailed use-case.

@rsc
@gopherbot gopherbot added this to the Proposal milestone Sep 4, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Sep 5, 2021
@ianlancetaylor ianlancetaylor mentioned this issue Sep 5, 2021
Closed
@rsc
Copy link
Contributor

rsc commented Oct 13, 2021

It sounds like maybe the version info is there, and we don't look for it?

/cc @jayconrod @bcmills

@rsc
Copy link
Contributor

rsc commented Oct 13, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 13, 2021
@bcmills
Copy link
Contributor

bcmills commented Oct 13, 2021

If there is a straightforward fix to the go version subcommand, I think we should just fix it (with an accompanying regression test). I'm not sure that a formal proposal is really needed.

If the fix is not straightforward, then I think this will ultimately fall into a much-needed “buildmode cleanup” bucket, which is probably long overdue but also difficult to prioritize.

@jayconrod
Copy link
Contributor

Build info is stamped into files built in c-archive and c-shared mode, but go version doesn't attempt to read it.

It might be very simple to extend go version -m to work with files built in c-shared mode. The only thing I'm not sure about is whether we need to apply relocations when reading information out of the binary. I don't think so since we're not actually loading the binary into memory.

c-archive is a little harder: go version -m would need to understand the archive format enough to find the executable within it.

@ianlancetaylor
Copy link
Contributor

Note that we have archive reading code in cmd/link/internal/ld/ar.go.

@josharian
Copy link
Contributor

If we can tolerate extremely rare false positives, it may also be cheaper and easier not to parse the file at all. See e.g. tailscale/tailscale@a4e19f2.

@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

We already don't really parse the file, except to find the start of the data segment. Sometimes the data segment is very far into the binary, so we use the parsing to skip to at least the right approximate place. I suppose we could just read the entire file but that's going to be slower.

@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

It sounds like this is probably pretty easy and that we should just do it.

@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Oct 20, 2021
@gopherbot
Copy link

Change https://golang.org/cl/357569 mentions this issue: debug/buildinfo: add regression tests for different buildmodes

@qmuntal
Copy link
Contributor Author

qmuntal commented Oct 21, 2021

I've added some more debug/buildinfo tests in CL 357569 to ensure it can retrieve buildinfo from exe, pie and c-shared buildmodes.

I haven't added a test for c-archive because it fails on my computer (windows/amd64) with the error unrecognized file format.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Oct 27, 2021
@rsc
Copy link
Contributor

rsc commented Oct 27, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: cmd/go: go version to support non-executable Go binaries cmd/go: go version to support non-executable Go binaries Oct 27, 2021
@rsc rsc modified the milestones: Proposal, Backlog Oct 27, 2021
@gopherbot
Copy link

Change https://go.dev/cl/391855 mentions this issue: cmd/go: go version to support shared libraries

@qmuntal qmuntal mentioned this issue Jul 5, 2022
Closed
gopherbot pushed a commit that referenced this issue Aug 22, 2022
@qmuntal @thanm
debug/buildinfo: add regression tests for different buildmodes
761db3e 
Updates #48187

Change-Id: I2364f248520e77c2e3a4832b9769b52e7aa62f73
Reviewed-on: https://go-review.googlesource.com/c/go/+/357569
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
gopherbot pushed a commit that referenced this issue Oct 10, 2022
@qmuntal @gopherbot
cmd/go: support shared libraries in 'go version' on Windows
742e0a9 
This change modifies 'go version' to support shared windows libraries.

Updates #48187

Change-Id: I2e8436b8df84fe76677106fa9ca02dcd1fb90e77
Reviewed-on: https://go-review.googlesource.com/c/go/+/391855
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/442035 mentions this issue: doc/go1.20: go version supports non-executable Go binaries

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@qmuntal @romaindoumenc
cmd/go: support shared libraries in 'go version' on Windows
712fed7 
This change modifies 'go version' to support shared windows libraries.

Updates golang#48187

Change-Id: I2e8436b8df84fe76677106fa9ca02dcd1fb90e77
Reviewed-on: https://go-review.googlesource.com/c/go/+/391855
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@qmuntal @romaindoumenc
doc/go1.20: go version supports non-executable Go binaries
6695343 
Closes golang#48187

Change-Id: Ibb808654bab3b6602b8901423fd297ad1f6e6386
Reviewed-on: https://go-review.googlesource.com/c/go/+/442035
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
No open projects
Proposals (old)
Accepted
Milestone
Go1.20
Development

No branches or pull requests
8 participants
@josharian @rsc @jayconrod @dmitshur @ianlancetaylor @qmuntal @bcmills @gopherbot