-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: incorrectly permits merging of unexported embedded pointer field name #21120
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
Comments
Hey @ianlancetaylor I've made a repro under my bugs repo at https://github.com/odeke-em/bugs/tree/master/golang/21120. go1.8.3$ go version && go run main.go
go version go1.8.3 darwin/amd64
runtime.Version() = "go1.8.3"
a.F() == "_/Users/emmanuelodeke/Desktop/openSrc/bugs/golang/21120/a", expected "a"
exit status 1 tip at 83fb9c8$ go version && go run main.go
go version devel +83fb9c8 Tue Jul 18 21:31:15 2017 +0000 darwin/amd64
runtime.Version() = "devel +83fb9c8 Tue Jul 18 21:31:15 2017 +0000"
a.F() == "_/Users/emmanuelodeke/Desktop/openSrc/bugs/golang/21120/a", expected "a"
b.F1() == "_/Users/emmanuelodeke/Desktop/openSrc/bugs/golang/21120/b", expected ""
exit status 1 where the only difference/regression in output is where main.go is a modification of your main, to just print the Go version first package main
import (
"fmt"
"os"
"runtime"
"./a"
"./b"
)
func main() {
fmt.Printf("runtime.Version() = %q\n", runtime.Version())
ok := true
if s, want := a.F(), "a"; s != want {
fmt.Printf("a.F() == %q, expected %q\n", s, want)
ok = false
}
if s, want := b.F1(), ""; s != want {
fmt.Printf("b.F1() == %q, expected %q\n", s, want)
ok = false
}
if s, want := b.F2(), ""; s != want {
fmt.Printf("b.F2() == %q, expected %q\n", s, want)
ok = false
}
if !ok {
os.Exit(1)
}
} /cc @rsc @griesemer @mdempsky |
CL https://golang.org/cl/50710 mentions this issue. |
@odeke-em Thanks, the In any case the CL I just sent has a cleaner version of the test, that simply verifies that the two essentially identical functions return the same result, which I believe they should. |
It is possible to have an unexported name with a nil package, for an embedded field whose type is a pointer to an unexported type. We must encode that fact in the type..namedata symbol name, to avoid incorrectly merging an unexported name with an exported name. Fixes #21120 Change-Id: I2e3879d77fa15c05ad92e0bf8e55f74082db5111 Reviewed-on: https://go-review.googlesource.com/50710 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Crawshaw <crawshaw@golang.org> Reviewed-on: https://go-review.googlesource.com/50970 Reviewed-by: Chris Broadfoot <cbro@golang.org>
Change https://golang.org/cl/50970 mentions this issue: |
Test case has three input files.
a.go:
b.go:
main.go:
The two functions in b.go are identical except for the name they use for the type defined within the function. Therefore, they should return the same value. All Go releases since Go 1 have returned the empty string for the
PkgPath
of a struct field of embedded pointer type, even if the embedded pointer type is not exported. That seems wrong, but it is what we have done, and it is what the code in main.go expects.However, in this case,
F1
does not return an empty string. The reason is thatF1
uses a namex
that is internally represented as a string with the exported flag (the least significant bit of the first byte in thereflect.name
) clear. That is fine, but in a.go the existence of the fieldx
inS
produces a namex
that is internally represented as a string with the exported flag set. Although those tworeflect.name
values are different, the compiler generates the same symbol name for both (type..namedata.x.
). The linker merges the two names, causingF1
to unexpectedly see an unexported name, and to therefore return an unexpected non-emptyPkgPath
.This bug is new in 1.9. I suspect that it was introduced by https://golang.org/cl/35732.
The text was updated successfully, but these errors were encountered: