Skip to content

Commit

Permalink
cmd/compile: use FmtLeft to generate symbol name for unexported inter…
Browse files Browse the repository at this point in the history
…face methods

The bug in 29612 is that there are two similar-looking anonymous interface
types in two different packages, ./p1/ssa and ./p2/ssa:

v.(interface{ foo() }).foo()

These types should be treated differently because the unexported method
makes the types different (according to the spec).

But when generating the type descriptors for those two types, they
both have the name "interface { ssa.foo() }". They thus get the same
symbol, and the linker happily unifies them. It picks an arbitrary one
for the runtime to use, but that breaks conversions from concrete types
that have a foo method from the package which had its interface type
overwritten.

We need to encode the metadata symbol for unexported methods as package
path qualified (The same as we did in CL 27791 for struct fields).

So switching from FmtUnsigned to Fmtleft by default fixes the issue.
In case of generating namedata, FmtUnsigned is used.

The benchmark result ends up in no significant change of compiled binary
compare to the immediate parent.

Fixes #29612

Change-Id: I775aff91ae4a1bb16eb18a48d55e3b606f3f3352
Reviewed-on: https://go-review.googlesource.com/c/go/+/170157
Reviewed-by: Matthew Dempsky <[email protected]>
  • Loading branch information
cuonglm authored and mdempsky committed Apr 1, 2019
1 parent a7fc710 commit 5fc55b3
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/cmd/compile/internal/gc/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,11 @@ func typefmt(t *types.Type, flag FmtFlag, mode fmtMode, depth int) string {
case types.IsExported(f.Sym.Name):
buf = append(buf, sconv(f.Sym, FmtShort, mode)...)
default:
buf = append(buf, sconv(f.Sym, FmtUnsigned, mode)...)
flag1 := FmtLeft
if flag&FmtUnsigned != 0 {
flag1 = FmtUnsigned
}
buf = append(buf, sconv(f.Sym, flag1, mode)...)
}
buf = append(buf, tconv(f.Type, FmtShort, mode, depth)...)
}
Expand Down
24 changes: 24 additions & 0 deletions test/fixedbugs/issue29612.dir/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run

// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Do not panic on conversion to anonymous interface, which
// is similar-looking interface types in different packages.

package main

import (
ssa1 "./p1/ssa"
ssa2 "./p2/ssa"
)

func main() {
v1 := &ssa1.T{}
_ = v1

v2 := &ssa2.T{}
ssa2.Works(v2)
ssa2.Panics(v2) // This call must not panic
}
18 changes: 18 additions & 0 deletions test/fixedbugs/issue29612.dir/p1/ssa/ssa.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package ssa

type T struct{}

func (T) foo() {}

type fooer interface {
foo()
}

func Unused(v interface{}) {
v.(fooer).foo()
v.(interface{ foo() }).foo()
}
28 changes: 28 additions & 0 deletions test/fixedbugs/issue29612.dir/p2/ssa/ssa.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package ssa

type T struct{}

func (T) foo() {}

type fooer interface {
foo()
}

func Works(v interface{}) {
switch v.(type) {
case interface{}:
v.(fooer).foo()
}
}

func Panics(v interface{}) {
switch v.(type) {
case interface{}:
v.(fooer).foo()
v.(interface{ foo() }).foo()
}
}

0 comments on commit 5fc55b3

Please sign in to comment.