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/link: "-X main.version" when version has a value and is later used in a global struct literal doesn't work #34675

Closed
segevfiner opened this issue Oct 3, 2019 · 4 comments
Closed

cmd/link: "-X main.version" when version has a value and is later used in a global struct literal doesn't work #34675

segevfiner opened this issue Oct 3, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@segevfiner
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.13.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="<snip>/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/segev/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go@1.12/1.12.9/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go@1.12/1.12.9/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/b_/c6yh0ksn63d1yy192x2p9f4c0000gn/T/go-build796234501=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Steps

  1. Save main.go, go build and run it.
  2. You get "Version: dev"
  3. Run go build -ldflags "-X main.version=foo"
  4. You get "Version: dev", instead of "Version: foo".

If you change var version = "dev" to var version string, it works. It also works if you just fmt.Println("Version:", version) without going through the struct.

The documentation says:

	This is only effective if the variable is declared in the source code either uninitialized
	or initialized to a constant string expression. -X will not work if the initializer makes
	a function call or refers to other variables.

Which doesn't suggest to me that this should somehow not work.

This will be a common scenario when using https://github.com/spf13/cobra which uses a global struct like that with a Version field, and is where I encountered this originally.

main.go

package main

import "fmt"

var version = "dev"

type cmd struct {
	Version string
}

var rootCmd = cmd{
	Version: version,
}

func main() {
	fmt.Println("Version:", rootCmd.Version)
}

What did you expect to see?

"Version: foo" when running go build -ldflags "-X main.version=foo".

What did you see instead?

"Version: dev" when running go build -ldflags "-X main.version=foo".
@andybons andybons changed the title "-X main.version" when version has a value and is later used in a global struct literal doesn't work cmd/link: "-X main.version" when version has a value and is later used in a global struct literal doesn't work Oct 3, 2019
@andybons
Copy link
Member

andybons commented Oct 3, 2019

@cherrymui @rsc @mdempsky @ianlancetaylor

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 3, 2019
@andybons andybons added this to the Unplanned milestone Oct 3, 2019
@cuonglm
Copy link
Member

cuonglm commented Oct 3, 2019

@ianlancetaylor explained in #26042 (comment)

@mdempsky
Copy link
Member

mdempsky commented Oct 3, 2019 •
edited

The issue is that for something like:

var A = "foo"
var B = A

the compiler recognizes that A is statically initialized to a constant and B is statically initialized to a copy of A, so it's compiled as:

var A = "foo"
var B = "foo"    // optimized by cmd/compile

Then later when we link with -X mypkg.A=bar, it only changes it to:

var A = "bar"    // modified by cmd/link -X
var B = "foo"

because the linker doesn't know it needs to modify B as well.

I see at least a few options here:

  1. Change cmd/link's documentation to only guarantee -X works for uninitialized string variables. cmd/compile already pessimistically optimizes references to uninitialized package-scope variables, assuming that they might be initialized externally (e.g., assembly code or the linker).

  2. Change cmd/compile to not apply the optimization mentioned above to string-typed variables. This might produce slightly worse code in some cases, but I think should be limited to emitting slightly less efficient program initialization code (i.e., we need to dynamically initialized some variables instead of statically initialize them).

  3. Change cmd/compile to track how string-typed variables are used and where the values get copied so that cmd/link can patch up everywhere it's copied to. I'm worried this will be a lot of complexity for little value.

@gopherbot
Copy link

Change https://golang.org/cl/198657 mentions this issue: cmd/compile: don't statically copy string-typed variables

@cherrymui cherrymui mentioned this issue Feb 21, 2020
Open
@golang golang locked and limited conversation to collaborators Oct 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@mdempsky @andybons @cuonglm @gopherbot @segevfiner