cmd/go: invalid -ldflags is silently ignored

[hyangah]

go version go1.15.5 linux/amd64

$ cat main.go
package main
var X = ""
func main() {	println(X)  }

$ go run -ldflags='-X main.X=Hello' main.go
Hello
$ go run -ldflags='"-X main.X=Hello"' main.go

$

The -ldflags of the second go run is in invalid format, but the go command silently ignores it and keeps working.

$ go run -ldflags='"-X main.X=Hello"' -x main.go
WORK=/tmp/go-build569857493
mkdir -p $WORK/b001/
...
mkdir -p $WORK/b001/exe/
cd .
$GOROOT/pkg/tool/linux_amd64/link -o $WORK/b001/exe/main -importcfg $WORK/b001/importcfg.link -s -w -buildmode=exe -buildid=HYVxxilVrLgdctP3U_YF/h7WWNLZkY13bQM5oqdBD/IUwm0pzLWLHdmcshsYuR/HYVxxilVrLgdctP3U_YF -extld=clang $WORK/b001/_pkg_.a
$WORK/b001/exe/main

See above link command. That's missing -X main.X=Hello

[bcmills]

From what I can tell, that flag is here:

go/src/cmd/go/internal/load/flag.go

Line 17 in 64fae25

BuildLdflags PerPackageFlag // -ldflags

That triggers the parsing case here:

go/src/cmd/go/internal/load/flag.go

Lines 52 to 63 in 64fae25

	if !strings.HasPrefix(v, "-") {
	i := strings.Index(v, "=")
	if i < 0 {
	return fmt.Errorf("missing =<value> in <pattern>=<value>")
	}
	if i == 0 {
	return fmt.Errorf("missing <pattern> in <pattern>=<value>")
	}
	pattern := strings.TrimSpace(v[:i])
	match = MatchPackage(pattern, cwd)
	v = v[i+1:]
	}

...which succeeds because it finds =, and then attempts to match "-X main.X against a package path (which obviously fails to match anything because it's not a syntactically valid package path).

Before we call MatchPackage on the parsed pattern, we should first verify that it is a syntactically valid package pattern.

CC @jayconrod @matloob

[gopherbot]

Change https://golang.org/cl/339289 mentions this issue: load: fix ldflags invalid parameters

[mpictor]

Not sure if it's this exact issue, but I just noticed that with go1.16.4 the buildid I set is silently failing, and when it does it wipes out other ldflags.

go build -n -ldflags "'-buildid=my id' -X main.somevar='other value'" -o mybin cmd/go |&tail -n3|head -n1|sed 's/ -/\n -/g'

/usr/local/go/pkg/tool/linux_amd64/link
  -o $WORK/b001/exe/a.out
  -importcfg $WORK/b001/importcfg.link
  -X=cmd/internal/objabi.buildID=34hrmDhOa-Ykhyq3ZhCY/qBDvtQ-RAzWZwV2zip4h/qBDvtQ-RAzWZwV2zip4h/34hrmDhOa-Ykhyq3ZhCY
  -buildmode=exe
  -buildid=34hrmDhOa-Ykhyq3ZhCY/qBDvtQ-RAzWZwV2zip4h/qBDvtQ-RAzWZwV2zip4h/34hrmDhOa-Ykhyq3ZhCY
  -extld=gcc $WORK/b001/_pkg_.a

vs
go build -n -ldflags "-X main.somevar='other value'" -o mybin cmd/go |& tail -n3|head -n1|sed 's/ -/\n -/g'

/usr/local/go/pkg/tool/linux_amd64/link
  -o $WORK/b001/exe/a.out
  -importcfg $WORK/b001/importcfg.link
  -X=cmd/internal/objabi.buildID=ioJGL5vMyH4sJ99fxTWh/qBDvtQ-RAzWZwV2zip4h/qBDvtQ-RAzWZwV2zip4h/ioJGL5vMyH4sJ99fxTWh
  -buildmode=exe
  -buildid=ioJGL5vMyH4sJ99fxTWh/qBDvtQ-RAzWZwV2zip4h/qBDvtQ-RAzWZwV2zip4h/ioJGL5vMyH4sJ99fxTWh
  -X main.somevar='other value'
  -extld=gcc $WORK/b001/_pkg_.a

In the past I was able to set a build id containing spaces using the syntax in the first command, i.e. '-buildid=some value' - so disallowing build id's containing spaces is a regression.

[mpictor]

It seems the order of args matters:
go build -n -ldflags "-X main.somevar='other value' '-buildid=my id'" -o mybin cmd/go |&tail -n3|head -n1|sed 's/ -/\n -/g'

/usr/local/go/pkg/tool/linux_amd64/link
  -o $WORK/b001/exe/a.out
  -importcfg $WORK/b001/importcfg.link
  -X=cmd/internal/objabi.buildID=g6Bfka0tA1jniKMcsnRZ/-IMjHA-s38sooeR9yuWL/QLyQO3HmDx9HHhrCDZtt/g6Bfka0tA1jniKMcsnRZ
  -buildmode=exe
  -buildid=g6Bfka0tA1jniKMcsnRZ/-IMjHA-s38sooeR9yuWL/QLyQO3HmDx9HHhrCDZtt/g6Bfka0tA1jniKMcsnRZ
  -X main.somevar='other value' "-buildid=my id"
  -extld=gcc /home/mark/.cache/go-build/dc/dc883a0241b3ab0a931f8972d7457b4f52681d8efc982c5537dbef0f34fc766f-d

However, this command results in an error if actualy executed (by removing -r:
go build -ldflags "-X main.somevar='other value' '-buildid=my id'" -o mybin cmd/go

# cmd/go
usage: link [options] main.o
  -B note
        add an ELF NT_GNU_BUILD_ID note when using ELF
...

[gopherbot]

Change https://go.dev/cl/407314 mentions this issue: misc/cgo/testplugin: set the package name in TestIssue19534

[gopherbot]

Change https://go.dev/cl/419594 mentions this issue: .github/workflows: add go1.19rc2 to long/smoke tests