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: throw instead of panic in runtime #25201

Closed
josharian opened this issue May 1, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@josharian
Copy link
Contributor

commented May 1, 2018

For discussion; I don't know whether this is a good idea.

The runtime uses slices. Though the runtime contributors Never Make Mistakes, slice accesses can panic. Most such panics are very bad news. However, they might be caught by another package's recover. That's why we have throw.

We might want to have the compiler automatically insert throwindex instead of panicindex, throwslice instead of panicslice, etc. when compiling the runtime.

cc @aclements @randall77

@josharian josharian added this to the Go1.12 milestone May 1, 2018

@randall77

This comment has been minimized.

Copy link
Contributor

commented May 1, 2018

So the only way to get a panicindex in the runtime is with an explicit panic call?
Seems reasonable. I do worry about the slow buildup of more and more special cases in the compiler for runtime package, though.

@odeke-em

This comment has been minimized.

Copy link
Member

commented May 2, 2018

@josharian sorry if am preaching to the choir but won't this technically violate the spec https://golang.org/ref/spec#Run_time_panics which requires panics with runtime.Error for slice access issues?

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2018

won't this technically violate the spec

I don't think so. That's for observable panics. These "panics" are failures of the language implementation, and this change just makes sure that those take down the process rather than letting it continue in a broken state.

@gopherbot

This comment has been minimized.

Copy link

commented May 3, 2018

Change https://golang.org/cl/111257 mentions this issue: cmd/compile: use throwX instead of panicX in runtime

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2018

Not going to do this. See the discussion in the CL.

@josharian josharian closed this May 3, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Jun 29, 2018

Change https://golang.org/cl/121515 mentions this issue: runtime: throw if the runtime panics with out of bounds index

gopherbot pushed a commit that referenced this issue Jun 29, 2018

runtime: throw if the runtime panics with out of bounds index
If the runtime code panics due to a bad index or slice expression,
then throw instead of panicing. This will skip calls to recover and dump
the entire runtime stack trace. The runtime should never panic due to
an out of bounds index, and this will help with debugging if it does.

For #24991
Updates #25201

Change-Id: I85a9feded8f0de914ee1558425931853223c0514
Reviewed-on: https://go-review.googlesource.com/121515
Reviewed-by: Austin Clements <austin@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.