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/compile: GOEXPERIMENT=unified on macOS fails fixedbugs/issue27836.go #53954

Closed
dr2chase opened this issue Jul 19, 2022 · 8 comments
Closed

cmd/compile: GOEXPERIMENT=unified on macOS fails fixedbugs/issue27836.go #53954

dr2chase opened this issue Jul 19, 2022 · 8 comments
Assignees
@dr2chase
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.20

Comments

@dr2chase
Copy link
Contributor

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

$ go version
go version devel go1.19-de8101d21b Mon Jul 18 18:04:23 2022 +0000 X:unified 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/drchase/Library/Caches/go-build"
GOENV="/Users/drchase/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT="unified"
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/drchase/work/gocode/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/drchase/work/gocode"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/drchase/work/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/drchase/work/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="devel go1.19-de8101d21b Mon Jul 18 18:04:23 2022 +0000 X:unified"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v6/9yfw749x623dr55rpm6ntnsc0095tn/T/go-build1756414274=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

GOEXPERIMENT=unified ./make.bash
cd ../test
go run run.go -- fixedbugs/issue27836.go

What did you expect to see?

Success

What did you see instead?

<unknown line number>: internal compiler error: have package "test/Äfoo" (0xc00044fd10), want package "test/Äfoo" (0xc00044fb80)
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 19, 2022
@dr2chase dr2chase self-assigned this Jul 19, 2022
@dr2chase
Copy link
Contributor Author

I'm going to start bisecting. I tried this on a hunch that something had changed in tip (since the dev.unified branch was working, till the latest merge).

@bcmills
Copy link
Contributor

bcmills commented Jul 19, 2022 •
edited

The compiledir action in run.go has a goDirPackages helper function to associate the file into packages, and that function uses ioutil.ReadFile to parse the actual package line from the source code. So the output of ReadDir shouldn't actually matter, because we're using the source code — not the filename.

I think the bug is here:
https://cs.opensource.google/go/go/+/master:test/run.go;l=272;drc=1243ec9c177007879958443262fe4d25099c5ede
We're using names[0] instead of pkgname to compute the import path to pass to the compiler. (names[0] is the filename from ReadDir, not the actual package name parsed from the file.)

That suggests https://go.dev/cl/395258 as the culprit (attn @mdempsky). Does March 24 sound like a plausible timeframe?

@dr2chase
Copy link
Contributor Author

It was working on May 2, at the boringcrypto merge 0668e3c .
That still might be part of the problem, but I want to see what bisecting finds, now that I have good and bad points.

@dr2chase
Copy link
Contributor Author

So the other half of the problem, no surprise, was cmd/compile: set LocalPkg.Path to -p flag.

@mdempsky
Copy link
Member

We're using names[0] instead of pkgname to compute the import path to pass to the compiler.

But package path and package name are separate names. The test is structured so you can vary them independently, and we allow them to vary separately in normal usage too (eg, packages within GOPATH or modules).

The problem I think is issue27836 relies on the file system preserving Unicode file names exact byte encoding, but macOS canonicalizes file names in some cases. I would expect the same problem could arise if a Go module had a similarly named directory: it would be compiled as one path on macOS due to canonicalization, and a different one on other OSes.

@gopherbot
Copy link

Change https://go.dev/cl/418294 mentions this issue: test: change Unicode file/package name to use characters not translated by macOS.

@bcmills
Copy link
Contributor

bcmills commented Jul 19, 2022 •
edited

I would expect the same problem could arise if a Go module had a similarly named directory: it would be compiled as one path on macOS due to canonicalization, and a different one on other OSes.

That shouldn't be the case (modulo bugs, of course), but see also #38342 and #38571. 🙃

@mdempsky mdempsky mentioned this issue Jul 19, 2022
Open
@gopherbot
Copy link

Change https://go.dev/cl/418334 mentions this issue: [dev.unified] test: change Unicode file/package name to use characters not translated by macOS.

@prattmic prattmic added this to the Backlog milestone Jul 25, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@dr2chase @jproberts
[dev.unified] test: change Unicode file/package name to use character…
9ee7e7b 
…s not translated by macOS.

In filenames, macOS translates Ä (U+00c4, c3 84) to Ä (U+0041 U+0308, 41 cc 88).
This causes problems for run.go's crude rules for testing the compiler.

Fixes golang#53954.

Change-Id: I850421cbf07e022ca5ff8122e0fb4e80deb55adf
Reviewed-on: https://go-review.googlesource.com/c/go/+/418334
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Aug 29, 2022
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 29, 2022
@golang golang locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dr2chase dr2chase

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
6 participants
@mdempsky @prattmic @dmitshur @dr2chase @bcmills @gopherbot