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: handle space in path to C compiler #43808

Closed
apmckinlay opened this issue Jan 20, 2021 · 15 comments
Closed

cmd/go: handle space in path to C compiler #43808

apmckinlay opened this issue Jan 20, 2021 · 15 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.16

Comments

@apmckinlay
Copy link

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

$ go version
go version go1.15.7 windows/amd64

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

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\andrew\AppData\Local\go-build
set GOENV=C:\Users\andrew\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\andrew\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\andrew\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=V:\gsuneido\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\andrew\AppData\Local\Temp\go-build158816006=/tmp/go-build -gno-record-gcc-switches
GOROOT/bin/go version: go version go1.15.7 windows/amd64
GOROOT/bin/go tool compile -V: compile version go1.15.7
gdb --version: GNU gdb (GDB) 8.1

What did you do?

> go build -v -ldflags "-s -w -X 'main.builtDate=Jan 20 2021 11:17:27' -X main.mode=gui -H windowsgui"

What did you expect to see?

Successful build

What did you see instead?

# github.com/apmckinlay/gsuneido/builtin/goc
cgo: exec c:\Program: exec: "c:\\Program": file does not exist

It builds successfully on 1.15.6 (and 1.16beta1)

I cut my path down to:

PATH=C:\WINDOWS\System32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\system32;C:\Program Files\mingw-w64\x86_64-8.1.0-posix-seh-rt_v6-rev0\mingw64\bin;C:\Go\bin;C:\Users\andrew\go\bin

In particular, mingw-w64 is in c:\program files
I think that was the default, but I can't remember for sure.

Cmd finds g++ using this path

> g++ --version
g++ (x86_64-posix-seh-rev0, Built by MinGW-W64 project) 8.1.0
@seankhliao seankhliao changed the title 1.15.7 on Windows doesn't handle space in path to C compiler cmd/go: handle space in path to C compiler on windows Jan 20, 2021
@seankhliao seankhliao added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Jan 20, 2021
@AlexRouSg

This comment has been minimized.

Sign in to view
@apmckinlay
Copy link
Author

I'm not sure this is the same as #41400 although it could be related.
My problem just started with 1.15.7.
That issue appears to affect older versions.
I don't have an external CC environment variable
My go env CC is just "gcc", no spaces.

@AlexRouSg
Copy link
Contributor

@apmckinlay sorry for missing that it started with 1.15.7

Then this looks due to the security fix in #43785

@apmckinlay
Copy link
Author

@AlexRouSg That's what I was assuming

@AlexRouSg
Copy link
Contributor

After reading the patches ... it looks like it sets the CC env var to the full path which contains spaces and thus triggering #41400
e8e7fac#diff-fec5b487b6c6e4cfdbf1e8af384e463d03693a05feb2f9b9a757feb0ab12e1e4R2028

So it does look like fixing #41400 would fix this too. Tho I'm not too sure what route it would eventually take.

cc @bcmills @rolandshoemaker

@rsc
Copy link
Contributor

rsc commented Jan 20, 2021

Apologies. The workaround is to use c:\progra~1 in your path for now.

@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 20, 2021
@rsc rsc added this to the Go1.16 milestone Jan 20, 2021
@apmckinlay
Copy link
Author

@rsc No worries, thanks.

@zzwx
Copy link

zzwx commented Jan 21, 2021 •
edited

Imagining a new user downloading fresh go version and bumping into inability to compile anything that requires "cgo"

@dmitshur
Copy link
Contributor

@rsc This is a release blocker and doesn't have an assignee right now. Are you planning to work on it, or should someone else? Thanks.

@jayconrod
Copy link
Contributor

I can take a look at this.

I was able to reproduce it on macOS by the way. It looks like it's an issue with spaces in the CC variable passed to cgo, not specifically a Windows issue.

This will likely need to be backported, so I'd rather not attempt a complete fix for #41400 yet.

@jayconrod jayconrod changed the title cmd/go: handle space in path to C compiler on windows cmd/go: handle space in path to C compiler Jan 21, 2021
@jayconrod jayconrod self-assigned this Jan 21, 2021
@jayconrod jayconrod mentioned this issue Jan 21, 2021
Closed
@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2021

@jayconrod, if the alternative approach in #41400 (comment) doesn't work out, another interim mitigation on Windows in particular might be to use GetShortPathName to obtain the space-free version of the path that Russ suggested as a workaround in #43808 (comment).

@gopherbot
Copy link

Change https://golang.org/cl/285873 mentions this issue: cmd/go: don't lookup the path for CC when invoking cgo

@jayconrod
Copy link
Contributor

It looks like the alternative approach works out. It just means we revert CL 284780. That was part of the security fix, but it was a "belt and suspenders" fix, and we still have the belt: internal/execabs prevents cgo from using CC with relative paths in PATH.

@jayconrod
Copy link
Contributor

@gopherbot Please backport to 1.14 and 1.15. This was a regression.

@gopherbot gopherbot mentioned this issue Jan 22, 2021
Closed
@gopherbot
Copy link

Backport issue(s) opened: #43859 (for 1.14), #43860 (for 1.15).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Jan 22, 2021
Closed
This was referenced Jan 26, 2021
Closed
Closed
Closed
@tmthrgd tmthrgd mentioned this issue Jan 27, 2021
Closed
@jayconrod jayconrod mentioned this issue Feb 1, 2021
Closed
@ianlancetaylor ianlancetaylor mentioned this issue Feb 1, 2021
Closed
@golang golang locked and limited conversation to collaborators Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.16
Development

No branches or pull requests
9 participants
@rsc @jayconrod @dmitshur @apmckinlay @bcmills @AlexRouSg @zzwx @gopherbot @seankhliao