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: nil pointer dereference in the 1.18 compiler when dealing with generic types #50423

Closed
pcostanza opened this issue Jan 4, 2022 · 10 comments
Closed

cmd/compile: nil pointer dereference in the 1.18 compiler when dealing with generic types #50423

pcostanza opened this issue Jan 4, 2022 · 10 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

@pcostanza
Copy link

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

$ go version
go version go1.18beta1 darwin/amd64

and

$ go version
go version devel go1.18-95b240b Mon Jan 3 23:45:12 2022 +0000 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/pcostanz/Library/Caches/go-build"
GOENV="/Users/pcostanz/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/pcostanz/go/pkg/mod"
GONOPROXY=/*set but not revealed*/
GONOSUMDB=/*set but not revealed*/
GOOS="darwin"
GOPATH="/Users/pcostanz/go"
GOPRIVATE=/*set but not revealed*/
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/pcostanz/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/pcostanz/sdk/gotip/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="devel go1.18-95b240b Mon Jan 3 23:45:12 2022 +0000"
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/z2/j1tgwrsx2cdcpr780wzjk6tw0000gq/T/go-build154765131=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

It is very difficult to reproduce the issue. I am currently developing a set of three libraries depending on each other, using generic types in current go 1.18 beta / development versions. (library A depends on B, library B depends on C) The bug occurs when I try to build library A, and it seems to be triggered by an instantiation of a generic function from library B.

The error message I get is the following in go1.18beta1

go1.18beta1 build *.go
# command-line-arguments
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x4 pc=0x17ee660]

goroutine 1 [running]:
cmd/compile/internal/noder.(*genInst).genericSubst(0x1f89a60, 0xc001c22eb0, 0xc000f82270, {0xc000e02798, 0x1, 0x1}, 0x1, 0xc001ac35d8)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:758 +0x160
cmd/compile/internal/noder.(*genInst).getInstantiation(0x1f89a60, 0xc000f82270, {0xc000e02790, 0x1, 0x1a8ecb8?}, 0x38?)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:703 +0x39d
cmd/compile/internal/noder.(*genInst).scanForGenCalls.func1({0x1a8d6d8, 0xc001696f30?})
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:176 +0x2df
cmd/compile/internal/ir.Visit.func1({0x1a8d6d8, 0xc001696f30})
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:105 +0x30
cmd/compile/internal/ir.doNodes(...)
	/usr/local/go/src/cmd/compile/internal/ir/node_gen.go:1512
cmd/compile/internal/ir.(*AssignListStmt).doChildren(0xc001ab40c0, 0xc001ac35c0)
	/usr/local/go/src/cmd/compile/internal/ir/node_gen.go:100 +0xa9
cmd/compile/internal/ir.DoChildren(...)
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:94
cmd/compile/internal/ir.Visit.func1({0x1a8d160, 0xc001ab40c0})
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:106 +0x57
cmd/compile/internal/ir.doNodes(...)
	/usr/local/go/src/cmd/compile/internal/ir/node_gen.go:1512
cmd/compile/internal/ir.(*Func).doChildren(0x1a8e038?, 0xc001ac35c0?)
	/usr/local/go/src/cmd/compile/internal/ir/func.go:152 +0x6e
cmd/compile/internal/ir.DoChildren(...)
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:94
cmd/compile/internal/ir.Visit.func1({0x1a8e038, 0xc0022542c0})
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:106 +0x57
cmd/compile/internal/ir.Visit({0x1a8e038, 0xc0022542c0}, 0xc001c04f00)
	/usr/local/go/src/cmd/compile/internal/ir/visit.go:108 +0xb8
cmd/compile/internal/noder.(*genInst).scanForGenCalls(0x1f89a60, {0x1a8e038, 0xc0022542c0})
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:158 +0x1f0
cmd/compile/internal/noder.(*genInst).buildInstantiations(0x1f89a60, 0x1)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:73 +0x85
cmd/compile/internal/noder.BuildInstantiations(...)
	/usr/local/go/src/cmd/compile/internal/noder/stencil.go:47
cmd/compile/internal/noder.(*irgen).generate(0xc000174240, {0xc000433b80, 0xa, 0x203000?})
	/usr/local/go/src/cmd/compile/internal/noder/irgen.go:320 +0x3db
cmd/compile/internal/noder.check2({0xc000433b80, 0xa, 0xa})
	/usr/local/go/src/cmd/compile/internal/noder/irgen.go:92 +0x16d
cmd/compile/internal/noder.LoadPackage({0xc000128110, 0xa, 0x0?})
	/usr/local/go/src/cmd/compile/internal/noder/noder.go:90 +0x335
cmd/compile/internal/gc.Main(0x1946fe8)
	/usr/local/go/src/cmd/compile/internal/gc/main.go:191 +0xb13
main.main()
	/usr/local/go/src/cmd/compile/main.go:55 +0xdd

I get the following with the gotip version:

gotip build *.go
# command-line-arguments
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x4 pc=0x17ef320]

goroutine 1 [running]:
cmd/compile/internal/noder.(*genInst).genericSubst(0x1f8daa0, 0xc0023a66e0, 0xc0021f6820, {0xc0023a4478, 0x1, 0x1}, 0x1, 0xc00239f9b0)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/stencil.go:758 +0x160
cmd/compile/internal/noder.(*genInst).getInstantiation(0x1f8daa0, 0xc0021f6820, {0xc0023a4470, 0x1, 0x1a90018?}, 0x98?)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/stencil.go:703 +0x39d
cmd/compile/internal/noder.(*genInst).scanForGenCalls.func1({0x1a8ea38, 0xc00222a360?})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/stencil.go:176 +0x2df
cmd/compile/internal/ir.Visit.func1({0x1a8ea38, 0xc00222a360})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/visit.go:105 +0x30
cmd/compile/internal/ir.doNodes(...)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/node_gen.go:1512
cmd/compile/internal/ir.(*AssignListStmt).doChildren(0xc0022264e0, 0xc00239f998)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/node_gen.go:100 +0xa9
cmd/compile/internal/ir.DoChildren(...)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/visit.go:94
cmd/compile/internal/ir.Visit.func1({0x1a8e4c0, 0xc0022264e0})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/visit.go:106 +0x57
cmd/compile/internal/ir.doNodes(...)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/node_gen.go:1512
cmd/compile/internal/ir.(*Func).doChildren(0x1a8f398?, 0xc00239f998?)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/func.go:152 +0x6e
cmd/compile/internal/ir.DoChildren(...)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/visit.go:94
cmd/compile/internal/ir.Visit.func1({0x1a8f398, 0xc00221adc0})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/visit.go:106 +0x57
cmd/compile/internal/ir.Visit({0x1a8f398, 0xc00221adc0}, 0xc0023aa040)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/ir/visit.go:108 +0xb8
cmd/compile/internal/noder.(*genInst).scanForGenCalls(0x1f8daa0, {0x1a8f398, 0xc00221adc0})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/stencil.go:158 +0x1f0
cmd/compile/internal/noder.(*genInst).buildInstantiations(0x1f8daa0, 0x1)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/stencil.go:73 +0x85
cmd/compile/internal/noder.BuildInstantiations(...)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/stencil.go:47
cmd/compile/internal/noder.(*irgen).generate(0xc00014c240, {0xc000403bd0, 0xa, 0x203000?})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/irgen.go:320 +0x3db
cmd/compile/internal/noder.check2({0xc000403bd0, 0xa, 0xa})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/irgen.go:92 +0x16d
cmd/compile/internal/noder.LoadPackage({0xc000003950, 0xa, 0x0?})
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/noder/noder.go:90 +0x335
cmd/compile/internal/gc.Main(0x1948090)
	/Users/pcostanz/sdk/gotip/src/cmd/compile/internal/gc/main.go:191 +0xb13
main.main()
	/Users/pcostanz/sdk/gotip/src/cmd/compile/main.go:55 +0xdd

I have tried several times to create a small example that triggers the same bug, but I haven't yet managed to do so.

What did you expect to see?

I believe the source code to be correct. go1.18beta1 vet *.go and gotip vet *.go don't report any issues.

What did you see instead?

See above.
@ALTree
Copy link
Member

ALTree commented Jan 4, 2022

A reproducer (even if not minimized, assuming it's not 100k lines of code) could be useful.

cc @danscales @randall77 in the meantime.

@ALTree ALTree changed the title nil pointer dereference in the 1.18 compiler when dealing with generic types cmd/compile: nil pointer dereference in the 1.18 compiler when dealing with generic types Jan 4, 2022
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 4, 2022
@ALTree ALTree added this to the Go1.18 milestone Jan 4, 2022
@pcostanza
Copy link
Author

I am fully aware that something to reproduce this bug would be useful, and I'm working on it. ;)

Some more information: The nil dereference is triggered in line 758 of stencil.go. I have added a fmt.Println(nameNode) before line 756. The output that corresponds with the nil dereference looks like this:

LibraryB.nofunc.(*computedObject[bool,bool,int]).getElement

None of the other outputs until that point mention "nofunc" in their names. From elsewhere in the compiler sources, "nofunc" seems to suggest that it has something to do with "shape types." Could you tell me what a "shape type" is? Maybe this will give me a clue what I need to pay attention to to be able to create something to reproduce the bug.

Thanks,
Pascal

@pcostanza
Copy link
Author

More information: I have a method that looks as follows.

func (m *comp[D]) asMask() (result structure, ok bool) {
	m.initialized.Wait()
	switch m := any(m).(type) {
	case *comp[bool]:
		return m.resultStructure.construct(func(i int) bool {
			result, ok := m.getElement(i)
			return ok && result
		}), true
	default:
		return
	}
}

The bug seems to be triggered by m.getElement(i). When I change this as follows, the problem disappears:

func (m *comp[D]) asMask() (result structure, ok bool) {
	m.initialized.Wait()
	switch any(m).(type) {
	case *comp[bool]:
		return m.resultStructure.construct(func(i int) bool {
			result, ok := m.getElement(i)
			return ok && any(result).(bool)
		}), true
	default:
		return
	}
}

@danscales
Copy link
Contributor

Thanks for the issue report!

A simple example of your above code (a type switch on the receiver, where each case is instantiated types) seems to work fine. If there are no confidentiality issues and you are able to package things up reasonable, feel free to point to the entire large failure case, via a github tree, a gzipped archive, etc. Of course, a smaller repro is also welcome.

For info on shape types, see here. Basically, we group types into gcshape groups, and only create one instantiation of a specific generic function/type for all types in a group. (Our gcshape groups in Go 1.18 are finer-grain than the ones described in the proposal.) This is to reduce the number of instantiations we create and avoid possible code explosion. A shape type is the representative type in a gcshape group that we use when compiling an instantiation. I'm not sure if knowledge of shape types will necessarily help you create a smaller reproduction.

@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

It also looks like noder has not been placed under the usual panic handler (which hides panics when there are type errors in the program, and otherwise prints a nice 'internal error' report). It should be. The compiler should never just dump a Go panic like in the report.

@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

I see what happened. In the C version of the compiler the fault handler was subtly different from hidePanic. In the case where there were no errors printed, it called what amounts today to base.Fatalf("fault"), so that the compiler would print the 'Please file a bug report' text.

That's what's missing. And Fatalf already has the right logic (half duplicating hidePanic), so I think hidePanic can be simplfied to

func hidePanic() {
	if err := recover(); err != nil {
		base.Fatalf("panic: %v", err)
	}
}

And probably at that point should be renamed to handlePanic.

@gopherbot
Copy link

Change https://golang.org/cl/376057 mentions this issue: cmd/compile: print "internal compiler error" message for all compiler panics

gopherbot pushed a commit that referenced this issue Jan 11, 2022
@danscales
cmd/compile: print "internal compiler error" message for all compiler…
1abe9c1 
… panics

Change hidePanic (now renamed handlePanic) to print out the "internal
compiler error" message for all panics and runtime exceptions, similar
to what we already do for the SSA backend in ssa.Compile().

Previously, hidePanic would not catch panics/exceptions unless it wanted
to completely hide the panic because there had already been some
compiler errors.

Tested by manually inserting a seg fault in the compiler, and verifying
that the seg fault is cause and "internal compiler error" message (with
stack trace) is displayed proeprly.

Updates #50423

Change-Id: Ibe846012e147fcdcc63ac147aae4bdfc47bf5a58
Reviewed-on: https://go-review.googlesource.com/c/go/+/376057
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@pcostanza
Copy link
Author

I downloaded the newest version of gotip today, and now the issue doesn't occur anymore. Thanks a lot!

@danscales
Copy link
Contributor

OK, great, not sure what change fixed it. Still happy to have some version of your code as a test case if you're willing, but I realize that may be infeasible. Closing the issue.

@pcostanza
Copy link
Author

The code where this issue occurred is finally open sourced. You can find the libraries depending on each other at:

Alas, the code has also changed considerably since the bug report above, and what triggered the bug isn't even in the repositories anymore. But maybe you still find this interesting...

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
@danscales @jproberts
cmd/compile: print "internal compiler error" message for all compiler…
1df975c 
… panics

Change hidePanic (now renamed handlePanic) to print out the "internal
compiler error" message for all panics and runtime exceptions, similar
to what we already do for the SSA backend in ssa.Compile().

Previously, hidePanic would not catch panics/exceptions unless it wanted
to completely hide the panic because there had already been some
compiler errors.

Tested by manually inserting a seg fault in the compiler, and verifying
that the seg fault is cause and "internal compiler error" message (with
stack trace) is displayed proeprly.

Updates golang#50423

Change-Id: Ibe846012e147fcdcc63ac147aae4bdfc47bf5a58
Reviewed-on: https://go-review.googlesource.com/c/go/+/376057
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@golang golang locked and limited conversation to collaborators Apr 1, 2023
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
@rsc @danscales @ALTree @pcostanza @gopherbot