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: generic types implementing fmt.Stringer crash fmt.Printf #47740

Closed
4ad opened this issue Aug 17, 2021 · 6 comments
Closed

cmd/compile: generic types implementing fmt.Stringer crash fmt.Printf #47740

4ad opened this issue Aug 17, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@4ad
Copy link
Member

4ad commented Aug 17, 2021 •
edited

At tip (a304273) with -gcflags='-G=3'.

I have seen a strange behavior with generics types that implement interfaces such as fmt.Stringer. The errors happen at runtime, and greatly differ based on the specific example. Even modifying a few characters in the source code makes the specific panic different, or it makes it go away completely.

Here is one such example. I have managed to reproduce this with a smaller code example, but that was with an older version of the Go tree. As I said -- this is very twitchy and inconsistent.

package main

import "fmt"

type Exp[Ty any] interface {
	// fmt.Stringer /* only required for the workaround, see below.*/
	Eval() Ty
}

type Lit[Ty any] Ty

func (lit Lit[Ty]) Eval() Ty       { return Ty(lit) }
func (lit Lit[Ty]) String() string { return fmt.Sprintf("(lit %v)", Ty(lit)) }

type Eq[Ty any] struct {
	a Exp[Ty]
	b Exp[Ty]
}

func (e Eq[Ty]) String() string {
	// doesn't panic if String is called explicitely
	// (needs fmt.Stringer in Exp)
	return fmt.Sprintf("(eq %v %v)", e.a/*.String()*/, e.b/*.String()*/)
}

var (
	e0 = Eq[int]{Lit[int](128), Lit[int](64)}
	e1 = Eq[bool]{Lit[bool](true), Lit[bool](true)}
)

func main() {
	// this call prints:
	// %!v(PANIC=String method: runtime error: invalid memory address or nil pointer dereference)
	fmt.Printf("%v\n", e0)

	// this crashes the program with:
	// unexpected fault address 0x101010109
	// fatal error: fault
	// [signal SIGSEGV: segmentation violation code=0x1 addr=0x101010109 pc=0x106ed0d]
	fmt.Printf("%v\n", e1)
}

The code currently panics with

go run -gcflags='-G=3' f.go
%!v(PANIC=String method: runtime error: invalid memory address or nil pointer dereference)
unexpected fault address 0x101010109
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0x101010109 pc=0x106ed0d]

goroutine 1 [running]:
runtime.throw({0x10a2ff9, 0x1134f01})
	/Users/aram/go/src/runtime/panic.go:965 +0x71 fp=0xc000109610 sp=0xc0001095e0 pc=0x102ead1
runtime.sigpanic()
	/Users/aram/go/src/runtime/signal_unix.go:743 +0x305 fp=0xc000109660 sp=0xc000109610 pc=0x1042645
reflect.Value.Elem({0x1096800, 0x1124180, 0x726c8})
	/Users/aram/go/src/reflect/value.go:1150 +0xad fp=0xc0001096a0 sp=0xc000109660 pc=0x106ed0d
fmt.(*pp).printValue(0xc000078ea0, {0x1096800, 0x1124180, 0x0}, 0x76, 0x0)
	/Users/aram/go/src/fmt/print.go:810 +0x50a fp=0xc000109888 sp=0xc0001096a0 pc=0x108748a
fmt.(*pp).printArg(0xc000078ea0, {0x1096800, 0x1124180}, 0x76)
	/Users/aram/go/src/fmt/print.go:712 +0x74c fp=0xc000109928 sp=0xc000109888 pc=0x1086eec
fmt.(*pp).doPrintf(0xc000078ea0, {0x10a3977, 0xa}, {0xc000109aa0, 0xc0000001a0, 0xc000109a90})
	/Users/aram/go/src/fmt/print.go:1026 +0x288 fp=0xc000109a20 sp=0xc000109928 pc=0x10896e8
fmt.Sprintf({0x10a3977, 0xa}, {0xc000109aa0, 0x2, 0x2})
	/Users/aram/go/src/fmt/print.go:219 +0x59 fp=0xc000109a78 sp=0xc000109a20 pc=0x1083ed9
main.Eq[.shape.bool].String(...)
	/Users/aram/tmp/f.go:23
main.(*Eq[bool]).String(0xc000060040)
	<autogenerated>:1 +0x90 fp=0xc000109ad0 sp=0xc000109a78 pc=0x108ac50
fmt.(*pp).handleMethods(0xc000078d00, 0x1c0c0)
	/Users/aram/go/src/fmt/print.go:626 +0x31c fp=0xc000109d20 sp=0xc000109ad0 pc=0x108627c
fmt.(*pp).printArg(0xc000078d00, {0x1099880, 0xc000060040}, 0x76)
	/Users/aram/go/src/fmt/print.go:709 +0x693 fp=0xc000109dc0 sp=0xc000109d20 pc=0x1086e33
fmt.(*pp).doPrintf(0xc000078d00, {0x10a2da6, 0x3}, {0xc000109f50, 0x100916b, 0x1099880})
	/Users/aram/go/src/fmt/print.go:1026 +0x288 fp=0xc000109eb8 sp=0xc000109dc0 pc=0x10896e8
fmt.Fprintf({0x10c08c0, 0xc00000e018}, {0x10a2da6, 0x3}, {0xc000109f50, 0x1, 0x1})
	/Users/aram/go/src/fmt/print.go:204 +0x75 fp=0xc000109f18 sp=0xc000109eb8 pc=0x1083dd5
fmt.Printf(...)
	/Users/aram/go/src/fmt/print.go:213
main.main()
	/Users/aram/tmp/f.go:40 +0xc6 fp=0xc000109f80 sp=0xc000109f18 pc=0x108a726
runtime.main()
	/Users/aram/go/src/runtime/proc.go:255 +0x227 fp=0xc000109fe0 sp=0xc000109f80 pc=0x10311e7
runtime.goexit()
	/Users/aram/go/src/runtime/asm_amd64.s:1438 +0x1 fp=0xc000109fe8 sp=0xc000109fe0 pc=0x105a1e1

Notice that it dies only in the second call to fmt.Printf. The first call prints something about a panic, but doesn't actually crash the program, it's recovered.

A workaround for this issue is to manually call the String() method on the type. Of course this is feasible because it makes sense for fmt.Stringer, the workaround might not be feasible with other interface.

This is so twitchy that even inlining e1 inside the call to fmt.Printf makes the crash recoverable (but the output is still garbage).

I can't reproduce it any more, but with a previous version of the Go tree, the panic output said something about a possible linker bug.

I have not been able to reproduce this problem with go2go: https://go2goplay.golang.org/p/IOA2krkbxeZ
@4ad 4ad added this to the Go1.18 milestone Aug 17, 2021
@rogpeppe
Copy link
Contributor

Here's a simpler reproducer for at least one of the issues at play here:

package main

import "reflect"

type S[T any] struct {
	a interface{}
}

func (e S[T]) M() {
	v := reflect.ValueOf(e.a)
	_, _ = v.Interface().(int)
}

func main() {
	e := S[int]{0}
	e.M()
}

I get this error:

./tst.go:16:2: internal compiler error: assertion failed

goroutine 1 [running]:
runtime/debug.Stack()
	/home/rogpeppe/go/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x498000, 0xc0}, {0xcf3c95, 0x10}, {0x0, 0x0, 0x0})
	/home/rogpeppe/go/src/cmd/compile/internal/base/print.go:227 +0x154
cmd/compile/internal/base.Fatalf(...)
	/home/rogpeppe/go/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/base.Assert(...)
	/home/rogpeppe/go/src/cmd/compile/internal/base/print.go:239
cmd/compile/internal/noder.assert(...)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:27
cmd/compile/internal/noder.(*subster).node.func1({0xe62410, 0xc000796180})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:1125 +0x8bd
cmd/compile/internal/ir.editNodes({0xc00011c0d0, 0x1, 0xccc880}, 0xc00049b158)
	/home/rogpeppe/go/src/cmd/compile/internal/ir/node_gen.go:1521 +0x74
cmd/compile/internal/ir.(*AssignListStmt).editChildren(0xc0004869c0, 0xe5f9e0)
	/home/rogpeppe/go/src/cmd/compile/internal/ir/node_gen.go:108 +0x6e
cmd/compile/internal/ir.EditChildren(...)
	/home/rogpeppe/go/src/cmd/compile/internal/ir/visit.go:185
cmd/compile/internal/noder.(*subster).node.func1({0xe5f9e0, 0xc0007961e0})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:901 +0x423
cmd/compile/internal/noder.(*subster).node(0xc000154000, {0xe5f9e0, 0xc0007961e0})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:1184 +0xa5
cmd/compile/internal/noder.(*subster).list(0x13b8180, {0xc00066d2c0, 0x2, 0xc000088640})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:1267 +0x8e
cmd/compile/internal/noder.(*irgen).genericSubst(0xc000490000, 0xc0004bac30, 0xc0004944e0, {0xc000118010, 0x1, 0x1}, 0x1, 0xc000498000)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:714 +0xcc6
cmd/compile/internal/noder.(*irgen).getInstantiation(0xc000490000, 0xc0004944e0, {0xc00061a698, 0x1, 0x1}, 0x30)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:600 +0x33d
cmgo: exit 2
d/compile/internal/noder.(*irgen).instantiateMethods(0xc000490000)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:503 +0x1cb
cmd/compile/internal/noder.(*irgen).stencil(0xc000490000)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stencil.go:50 +0x9b
cmd/compile/internal/noder.(*irgen).generate(0xc000490000, {0xc000072b70, 0x2, 0x203000})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/irgen.go:265 +0x272
cmd/compile/internal/noder.check2({0xc000072b70, 0x2, 0x2})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/irgen.go:92 +0x16d
cmd/compile/internal/noder.LoadPackage({0xc00001e260, 0x2, 0x0})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/noder.go:90 +0x365
cmd/compile/internal/gc.Main(0xd1ae58)
	/home/rogpeppe/go/src/cmd/compile/internal/gc/main.go:190 +0xaf3
main.main()
	/home/rogpeppe/go/src/cmd/compile/main.go:55 +0xdd

@rogpeppe
Copy link
Contributor

actually, that's neither of the original issues - it's just crashing the compiler

@4ad
Copy link
Member Author

4ad commented Aug 17, 2021 •
edited

FWIW, the code I posted seems to work with GOEXPERIMENT=unified, although since this is so twitchy it might not mean anything.

Larger piece of code that works with GOEXPERIMENT=unified: https://go2goplay.golang.org/p/A3o8rIQ_mCn (although it doesn't work with go2go).

@mknyszek mknyszek changed the title generic types implementing fmt.Stringer crash fmt.Printf cmd/compile: generic types implementing fmt.Stringer crash fmt.Printf Aug 17, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 17, 2021
@mknyszek
Copy link
Contributor

CC @mdempsky since the above ICE is in the IR (though seems like it also leads to a miscompile?), and seems to be fixed with GOEXPERIMENT=unified.

@cuonglm
Copy link
Member

cuonglm commented Aug 17, 2021

Also cc @danscales @randall77

@gopherbot
Copy link

Change https://golang.org/cl/342989 mentions this issue: cmd/compile: only use dictionaries for conversions from type parameters

@golang golang locked and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
5 participants
@rogpeppe @mknyszek @4ad @cuonglm @gopherbot