cmd/compile: can't override variable with ldflags when initial value is a func

[sethvargo]

(This might be better categorized under the linker, please edit as appropriate)

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

$ go version
go version go1.21.4 darwin/arm64

Does this issue reproduce with the latest release?

Go 1.21.4 is the latest version as of this posting.

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

go env Output

$ go env
GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/sethvargo/Library/Caches/go-build'
GOENV='/Users/sethvargo/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/sethvargo/Development/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/sethvargo/Development/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.21.4/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.4/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/sethvargo/Development/project/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/cp/qb9vbbkx4w36f6dclng481br00gy5b/T/go-build715077767=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I tried to override a package variable (that has a default value) using ldflags, all using the same command:

go run \
  -a \
  -trimpath \
  -ldflags="-s -w -X main.Version=1.2.3. -extldflags=-static"

My expectation is that the Version value is set to "1.2.3" and therefore the program prints "1.2.3".

✅ Explicit string type

package main
import "fmt"
var Version string = "development"
func main() {
  fmt.Println(Version) // correctly prints "1.2.3"
}

✅ Implicit string type

package main
import "fmt"
var Version = "development"
func main() {
  fmt.Println(Version) // correctly prints "1.2.3"
}

✅ Explicit string type func() string

package main
import "fmt"
var Version string = func() string {
  return "development"
}()
func main() {
  fmt.Println(Version) // prints "1.2.3"
}

✅ Implicit string type func() string

package main
import "fmt"
var Version = func() string {
  return "development"
}()
func main() {
  fmt.Println(Version) // prints "1.2.3"
}

Run with -x output

❌ Explicit string type complex func() string

package main
import "fmt"
var Version string = func() string {
  if true {
    return "development"
  }
  return "production"
}()
func main() {
  fmt.Println(Version) // ❌ always prints "development"
}

❌ String concatenation with runtime variable

package main
import "fmt"
var Version string = "devel" + os.Getenv("FOO")
func main() {
  fmt.Println(Version) // ❌ always prints "devel"
}

Run with -x output

At first I thought this was because the compiler was optimizing away the function call, but surely the compiler should also optimize out the "always-true" branch.

What did you expect to see?

I expect the ldflags to override the variable.

What did you see instead?

ldflags conditionally overrides the variable, depending on other variables I don't fully understand.

What problem am I trying to solve?

I'd like to inject build information into the compiled binary, with sane fallback values if none are provided. Some of these values will be injected by the build process into the final binary, but if someone builds the binary themselves (or if they go install it), then I'd like some reasonable build information. Fortunately modern versions of Go expose this, but it seems to be incompatible with allowing the values to be overridden:

var Version string = func() string {
  if info, ok := debug.ReadBuildInfo(); ok {
    if v := info.Main.Version; v != "" {
      return v // e.g. "v0.0.1-alpha1.0.20231115..."
    }
  }

  return "source"
}()

I would like the default version to come from the debug package, but still provide a mechanism for builders to inject/override with their own values. With the function definition above, it's impossible for a builder to override Version. Since some of these functions could be called hundreds or thousands of times, I don't want to make this a pure function call. The best I could come up with was to move the package into internal and leverage a combination of private variables and once functions, but it's not pleasing:

package buildinfo

var version string
var Version string = sync.OnceValue(func() string {
  if v := version; v != "" {
    return v
  }

  if info, ok := debug.ReadBuildInfo(); ok {
    if v := info.Main.Version; v != "" {
      return v // e.g. "v0.0.1-alpha1.0.20231115..."
    }
  }

  return "source"
})()

[prattmic]

Without digging into the details, my guess is that the linker is changing the initial value of the variable, but for the complex expressions, the compiler has added an init() function which overrides the linker-specified default value. Why it works with some funcs but not others I'd guess comes down to whether the compiler manages to eliminate the function (I'm not sure how complex we get with that) and thus the init function.

cc @golang/compiler

[cherrymui]

I think it is understood that the ldflags can only overwrite a variable's static init value, not one with dynamic evaluation. I agree that the inconsistency is confusing. Maybe we want to disable some optimizations with string-typed global variables.

[sethvargo]

I think it is understood that the ldflags can only overwrite a variable's static init value...

Could you point me to the documentation for this please?

---

What is the preferred pattern for providing a complex computed default value but still permitting builders to override via an ldflag?

[seankhliao]

https://pkg.go.dev/cmd/link

-X importpath.name=value
	Set the value of the string variable in importpath named name to value.
	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.

var (
	Injected = ""
	Version  = func() string {
		if Injected != "" {
			return Injected
		}
		return "some default"
	}()
)

[sethvargo]

-X will not work if the initializer makes a function call or refers to other variables.

I think this is what @prattmic is referencing above, but it sometimes works for some function, depending on how the compiler does.

Also, it's strange that there's no linker error in this situation. Compare:

package main
import "fmt"

var commit int
func main() {
  fmt.Printf("%d\n", commit)
}

$ go run \
  -a \
  -trimpath \
  -ldflags="-s -w -X main.commit=abcdef1 -extldflags=-static" main.go

# command-line-arguments
❌ main.commit: cannot set with -X: not a var of type string (type:int)

to the function signature:

package main
import "fmt"

var commit = func() string {
  return "123"
}()

func main() {
  fmt.Printf("%d\n", commit)
}

which builds fine.

[cherrymui]

For the case above, the compiler inlines that function, so it becomes var commit = "123". We should probably disable that inlining for string-typed global variable.

[sethvargo]

Sorry, I mean even a more complex function that isn't inlined:

var commit = func() string {
  if time.Now().Unix()%3 == 0 {
    return "123"
  }
}()

Building that generates no warnings or errors. It seems odd that we return an error when linking against a non-string type... unless it's a function that returns a string type?

[cherrymui]

For this, the -X flag is ineffective (as documented). Currently it doesn't emit an error, partly because for the linker this is not much different from

var commit string
func init() {
  if time.Now().Unix()%3 == 0 {
    commit = "123"
  }
}

which is a valid program and the linker should not emit an error in any case. Technically the compiler is able to distinguish between the two and leave some metadata in the object file to inform the linker, but I'm not sure whether this is worth doing.

As documented, the -X flag applies only to string variable. It is an error if -X is applied to a non-string variable. So if the function returns a non-string type, it is not a string variable anymore, which causes the error.

[sethvargo]

Hi @cherrymui thank you for the reply. I understand from a technical perspective why it's behaving this way, but I'm asking you to look at it from the perspective of the user.

The help text says:

Set the value of the string variable in importpath named name to value.
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.

Specifically,

This is only effective if the variable is declared in the source code either uninitialized or initialized to a constant string expression.

When given a non-string expression, the user is presented an informative error that says "hey, you're doing it wrong." But with the second sentence in the help text,

-X will not work if the initializer makes a function call or refers to other variables.

When given a function expression [that returns a string], the user is provided no error or warning, and it silently goes unused.

I understand why the inconsistency exists across the compiler -> linker, but I'm trying to convey that it's unexpected from the user's perspective.

In the first sentence, "ineffective" means "will throw an error". In the second sentence, "ineffective" means "will be ignored". This seems like a really sharp edge case for users.

If it's not worth feeding this metadata from the compiler -> linker, then I think it's still worth updating the help output to note that this edge case will silently fail.

[ericlagergren]

We should probably disable that inlining for string-typed global variable.

Why?

[cherrymui]

We should probably disable that inlining for string-typed global variable.
Why?

Because otherwise this will cause confusion: the -ldflags=-X flag will be ineffective for

var X string = noninlineableCall()

but effective for

var X string = inlineableCall()

That is very confusing for the users.

We also already disable optimizations like rewriting

var A = "foo"; var B = A

to

var A = "foo"; var B = "foo"

for string-typed global variables (#34675).