-
Notifications
You must be signed in to change notification settings - Fork 18k
runtime: throw, not panic, for cgo misuse #13679
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
Comments
I think they should panic. throw is reserved for errors which indicate a bug or limitation in the runtime (or a user corrupting the runtime data structures, but that's another story). panic is for a bug in the user's code. Failed cgo checks fit into the latter category. |
CL https://golang.org/cl/18047 mentions this issue. |
@randall77, that's not accurate: the GC throws when it finds bad pointers in user data. What cgo is finding is roughly the same. |
@rsc, true, but there isn't a choice in that situation. When the runtime finds a bad pointer, it's in the middle of GC and there isn't any reasonable place to panic. Imagine if we could detect a bad pointer immediately when it is generated, then we could (and I argue should) panic when the user code generates one, not throw. In the cgo checks we have a natural place where we can panic. throw is not defined in the language spec. We should prefer panic if feasible. |
I chose panic when writing the code by analogy to operations like making a slice that is too large: the user is passing invalid data to a runtime function. If the code could return an error, that would be better still, but it can't. I'm not strongly opposed to throw, but to me this feels like more like a run-time panic. The case of #13677 doesn't seem to argue one way or another. There are perfectly normal operations that that could cause a driver to panic. |
OK. I'll reopen this if a more compelling example comes along. |
I think whether a runtime error panic or throw should
depends on whether the fault is recoverable or not.
If there are invalid cgo uses, recovering the panic is
just going to hide the programming error.
To put it in another way, if it's a panic, what could
the program do after it's recovered?
|
The same thing you do when dividing by zero or accessing a slice out of bounds. |
Usually the runtime throws when it finds heap corruption or other pointer misuse. The new cgo checks panic instead. Perhaps they should throw. Panicking uncovered #13677, which we should fix separately anyway. Even so, the behavior would have been clearer had the cgo check used throw, and throw is more consistent with the rest of the runtime.
Thoughts?
/cc @ianlancetaylor
The text was updated successfully, but these errors were encountered: