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

cmd/compile: function-to-interface conversion leads to miscompilation and segfault #48645

Closed
tdakkota opened this issue Sep 27, 2021 · 6 comments
Closed

Comments

@tdakkota
Copy link

@tdakkota tdakkota commented Sep 27, 2021

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

$ go version
go version devel go1.18-6e5dd0b59b Mon Sep 27 11:52:07 2021 +0000 windows/amd64

Does this issue reproduce with the latest release?

No

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

  • windows/amd64
  • linux/amd64 (godbolt)

What did you do?

https://play.golang.org/p/JxrNfegu__C

https://go.godbolt.org/z/1E6Kx3aoT

What did you expect to see?

Successful compilation.

What did you see instead?

Stack trace (with -l flag)
unexpected fault address 0x4666f0
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x2 addr=0x4666f0 pc=0x4666f0]

goroutine 1 [running]:
runtime.throw({0x461472, 0x7fe1f2155108})
	/opt/compiler-explorer/go-tip/src/runtime/panic.go:965 +0x71 fp=0xc00002c680 sp=0xc00002c650 pc=0x42b3f1
runtime.sigpanic()
	/opt/compiler-explorer/go-tip/src/runtime/signal_unix.go:743 +0x305 fp=0xc00002c6d0 sp=0xc00002c680 pc=0x43eb65
main.Stream[.shape.int_0].Iterate(0x4544c5, {{0x45bb00, 0xc00005a000}}, 0x0)
	/app/example.go:22 +0x31 fp=0xc00002c6f0 sp=0xc00002c6d0 pc=0x454411
main.Reduce[.shape.int_0,.shape.[]int_1](0x476d40, {{0x45bb00, 0xc00005a000}}, {0x0, 0x0, 0x0}, 0x4665b8)
	/app/example.go:52 +0xfe fp=0xc00002c738 sp=0xc00002c6f0 pc=0x45469e
main.main()
	/app/example.go:65 +0x4b fp=0xc00002c780 sp=0xc00002c738 pc=0x4541eb
runtime.main()
	/opt/compiler-explorer/go-tip/src/runtime/proc.go:255 +0x227 fp=0xc00002c7e0 sp=0xc00002c780 pc=0x42dbe7
runtime.goexit()
	/opt/compiler-explorer/go-tip/src/runtime/asm_amd64.s:1446 +0x1 fp=0xc00002c7e8 sp=0xc00002c7e0 pc=0x451281
@danscales
Copy link

@danscales danscales commented Sep 28, 2021

@randall77 Keith, do you want to look at this? I think this may be a problem with doing CONVIDATA on a function pointer which happens at line 47, when the program converts 'it' from an IteratorFunc[R] to an Iterator[R]. Do you know of any probably there might be with CONVIDATA in this case?

Loading

@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 29, 2021

I have a theory. It looks like there are 2 instantiations of Pipe, Pipe[.shape.int_0,.shape.int_1] and Pipe[.shape.int_0,.shape.int_0] each has a different dictionary layout. But the dictionaries when instantiated with int and int generate identically named dictionaries, .dict.Pipe[int,int].

I think the code ends up using the dictionary designed from the 0/1 version, but passes it to the 0/0 version.

Why does the 0/0 version exist at all? I'm not sure about that.

>>> InstInfo for Pipe[.shape.int_0,.shape.int_1]
  Typeparam .shape.int_0
  Typeparam .shape.int_1
  Subdictionary at generic function/method call: FromIterator - (<node FUNCINST>)(IteratorFunc[.shape.int_1](it))
  Itab for interface conv: IteratorFunc[.shape.int_1](it)
  Derived type Stream[.shape.int_0]
  Derived type func(.shape.int_0) (.shape.int_1, bool)
  Derived type Stream[.shape.int_1]
  Derived type func(func(.shape.int_1) bool)
  Derived type Iterator[.shape.int_0]
  Derived type func(.shape.int_1) bool
  Derived type func(.shape.int_0) bool
  Derived type func(Iterator[.shape.int_1]) Stream[.shape.int_1]
  Derived type Iterator[.shape.int_1]
  Derived type IteratorFunc[.shape.int_1]
>>> Done Instinfo
>>> InstInfo for Pipe[.shape.int_0,.shape.int_0]
  Typeparam .shape.int_0
  Typeparam .shape.int_0
  Subdictionary at generic function/method call: FromIterator - (<node FUNCINST>)(IteratorFunc[.shape.int_0](it))
  Itab for interface conv: IteratorFunc[.shape.int_0](it)
  Derived type Stream[.shape.int_0]
  Derived type func(.shape.int_0) (.shape.int_0, bool)
  Derived type func(func(.shape.int_0) bool)
  Derived type Iterator[.shape.int_0]
  Derived type func(.shape.int_0) bool
  Derived type func(Iterator[.shape.int_0]) Stream[.shape.int_0]
  Derived type IteratorFunc[.shape.int_0]
>>> Done Instinfo

Note that 0/1 has 14 entries, whereas 0/0 has 11 entries.

This is the only dictionary for Pipe in the assembly output, having 14 entries (so it's for the 0/1 instantiation):

""..dict.Pipe[int,int] SRODATA dupok size=112
	0x0000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	0x0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	0x0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	0x0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	0x0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	0x0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	0x0060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	rel 0+0 t=25 type."".Iterator[int]+96
	rel 0+0 t=24 type.int+0
	rel 0+0 t=24 type."".IteratorFunc[int]+0
	rel 0+0 t=24 type.int+0
	rel 0+0 t=25 type."".Iterator[int]+96
	rel 0+0 t=24 type."".Stream[int]+0
	rel 0+0 t=24 type.func("".Iterator[int]) "".Stream[int]+0
	rel 0+0 t=24 type.func(int) (int, bool)+0
	rel 0+0 t=24 type.func(int) bool+0
	rel 0+0 t=24 type."".Stream[int]+0
	rel 0+0 t=24 type.func(int) bool+0
	rel 0+0 t=24 type.func(func(int) bool)+0
	rel 0+8 t=1 type.int+0
	rel 8+8 t=1 type.int+0
	rel 16+8 t=1 type."".Stream[int]+0
	rel 24+8 t=1 type.func(int) (int, bool)+0
	rel 32+8 t=1 type."".Stream[int]+0
	rel 40+8 t=1 type.func(func(int) bool)+0
	rel 48+8 t=1 type."".Iterator[int]+0
	rel 56+8 t=1 type.func(int) bool+0
	rel 64+8 t=1 type.func(int) bool+0
	rel 72+8 t=1 type.func("".Iterator[int]) "".Stream[int]+0
	rel 80+8 t=1 type."".Iterator[int]+0
	rel 88+8 t=1 type."".IteratorFunc[int]+0
	rel 96+8 t=1 ""..dict.FromIterator[int]+0
	rel 104+8 t=1 go.itab."".IteratorFunc[int],"".Iterator[int]+0

But the only call in the binary is to the 0/0 version:

	0x0080 00128 (/Users/khr/gowork/issue48645.go:30)	CALL	"".Pipe[.shape.int_0,.shape.int_0](SB)

Which seems to be using the dictionary appropriate for the 0/1 implementation (reading it out of .dict.Stream[int].DropWhile).

Loading

@korzhao
Copy link
Contributor

@korzhao korzhao commented Sep 29, 2021

package main

import (
	"fmt"
	"reflect"
)

type Stream[T any] struct {
}

func (s Stream[T]) DropWhile() Stream[T] {
	return Pipe[T, T](s)
}

func Pipe[T, R any](s Stream[T]) Stream[R] {
	it := func(fn func(R) bool) {
	}
	fmt.Println(reflect.TypeOf(it).String())
	return Stream[R]{}
}

func main() {
	s := Stream[int]{}
	s = s.DropWhile()
}

Output:

main.Stream[int]

Loading

@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 29, 2021

@korzhao Yes, I believe that misbehavior is the same issue. It's trying to load the derived type func(func(.shape.int_0) bool) from the dictionary but instead gets Stream[.shape.int_1] because it is using the wrong dictionary.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 29, 2021

Change https://golang.org/cl/353030 mentions this issue: cmd/compile: make sure shapes have proper indexes for sub-instantiation

Loading

@danscales
Copy link

@danscales danscales commented Sep 29, 2021

OK, thanks @randall77 and @korzhao for point out the behavior with the multiple InstInfo & dictionaries. I figured out that problem and fixed it with this change: https://go-review.googlesource.com/c/go/+/353030

However, it looks like there is another bug, possibly related to converting a closure with captured variables to an interface using a dictionary. (so this could next bug could possible be related to OCONVIDATA). In the CL above, I comment out the closure body so that the test would pass.
If you re-enable the closure body, the you get a seg fault later in the execution in the closure with Reduce/Stream.Iterate/IteratorFunc.Iteration above it.

Loading

@gopherbot gopherbot closed this in c2de759 Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants