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

runtime: better panic messages in case of zero embedded interface #21152

Closed
opennota opened this issue Jul 25, 2017 · 11 comments

Comments

Projects
None yet
6 participants
@opennota
Copy link

commented Jul 25, 2017

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

1.9beta2

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

linux/amd64

What did you do?

Run the following program:

package main

type I interface {
	f()
}

type S struct {
	I
}

func main() {
	var s S
	s.f()
}

What did you expect to see?

A panic with a message explaining that the interface field is zero.

What did you see instead?

A somewhat cryptic message:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x450642]

goroutine 1 [running]:
main.main()
        /home/opennota/gocode/src/github.com/opennota/issue/main.go:13 +0x22
@odeke-em

This comment has been minimized.

Copy link
Member

commented Jul 25, 2017

Not a regression ie present on Go1.8 https://play.golang.org/p/78Cyilnq2C
screen shot 2017-07-24 at 9 46 19 pm

I'll mark this for Go1.10.

/cc runtime squad @aclements @RLH

@odeke-em odeke-em added this to the Go1.10 milestone Jul 25, 2017

@cznic

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2017

What did you expect to see?

A panic with a message explaining that the interface field is zero.

That would confuse me a lot. The zero value of any variable (struct field is a variable) is never invalid per se. Some operations don't work on some zero values, but that's a different story. (Integer division by zero falls into the same category, for example)

What did you see instead?

A somewhat cryptic message:

But it really is a nil dereference. The message correctly points out at what line it happened, which is enough information to derive the possible causes, in this case it's just one. On the contrary, to reconstruct at runtime that the particular signal was generated by an instruction which dereferences a value in an interface header that was fetched from a struct field (it's irrelevant if embedded or not actually and it's even irrelevant that the variable is a struct field), is non trivial and IMO not worth of the effort.

@opennota

This comment has been minimized.

Copy link
Author

commented Jul 25, 2017

it really is a nil dereference.

But it confuses new users, e.g. https://dev.to/loderunner/go-i-love-you-but-youre-bringing-me-down
(see Initialization of embedded interfaces).

is non trivial and IMO not worth of the effort

Maybe, and maybe not. I just thought I would open an issue in case there IS a simple solution, because the same had happened to me in the past.

@aclements

This comment has been minimized.

Copy link
Member

commented Jul 25, 2017

Do you have a proposal for what the message should say? As @cznic pointed out, this really is a nil pointer dereference.

Maybe, and maybe not. I just thought I would open an issue in case there IS a simple solution, because the same had happened to me in the past.

Unfortunately, there's no simple solution. Most pointer dereferences in Go don't even have an explicit check for nil pointers. We rely on the hardware to trap, and all we get from the hardware is a program counter. We'd need to associate extra metadata with that program counter to produce any more detail in the error message, which is possible, but means more complexity and bigger binaries.

@opennota

This comment has been minimized.

Copy link
Author

commented Jul 26, 2017

Add to the message an http(s) link to a page at golang.org, which enumerates the possible causes?

this really is a nil pointer dereference

There are no pointers in the program, in the Go sense.

@aclements

This comment has been minimized.

Copy link
Member

commented Jul 26, 2017

Add to the message an http(s) link to a page at golang.org, which enumerates the possible causes?

That just punts the problem to the web page. What would you say on the web page? The possible causes are that something dereferenced a nil pointer, so is the question really "where can nil pointers come from?" (I really am asking what isn't clear about the panic message because to me it is clear and does seem to indicate what the problem is, so I can't do anything without understanding how you're interpreting it. I might still not be able to do anything because of implementation limitations.)

There are no pointers in the program, in the Go sense.

Ah, that's interesting. Is the lack of clarity in the message, then, about nil pointers (as in *T for some T) versus nil interface values? (And, perhaps, nil maps, channels, and slices?)

Would it be an improvement simply for the message to say "nil value" instead of "nil pointer"? Did "nil pointer" lead you in the wrong direction?

@opennota

This comment has been minimized.

Copy link
Author

commented Jul 26, 2017

@aclements I think "nil value" is better.

@cznic

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2017

Nil value has a different and/or wider meaning in Go than nil pointer. It's not possible to dereference other nil values, for example.

The current error message is IMO more accurate in talking about a nil pointer than it would be with nil value.

@aclements

This comment has been minimized.

Copy link
Member

commented Jul 26, 2017

Nil value has a different and/or wider meaning in Go than nil pointer. It's not possible to dereference other nil values, for example.

That's a good point that if it said "nil value" it probably couldn't also use "dereference". I'm not sure what the right phrasing would be, though.

The current error message is IMO more accurate in talking about a nil pointer than it would be with nil value.

"Nil pointer" is more precise, but as @opennota noted, less accurate. It's framed in terms of implementation details, rather than in terms of the language specification. (Though perhaps not as bad as "NullPointerException" in Java, which doesn't have a language-level pointer concept at all :)

It would be nice if it could say "nil pointer", "nil interface", "nil map", etc as appropriate, but I don't think that's worth the non-trivial implementation cost. If there were a common phrasing that could capture all of these, however, I could be swayed.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2017

Well said, Austin.

However, even if there was a good common phrasing, I don't think we should switch to it. If there's any panic text that people are (inappropriately) relying on, it's this one; changing it would probably break so much code as to make it not worth doing, whether or not doing so would be "legal" according to go1 compat.

@aclements

This comment has been minimized.

Copy link
Member

commented Jul 31, 2017

If there's any panic text that people are (inappropriately) relying on, it's this one; changing it would probably break so much code as to make it not worth doing

That's a good, albeit unfortunate point.

Given this, I don't think we could change the message even if we could come up with a better wording. :(

@aclements aclements closed this Jul 31, 2017

@golang golang locked and limited conversation to collaborators Jul 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.