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.

Sign up for GitHub

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

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

cmd/compile: throw instead of panic in runtime #25201

josharian opened this issue May 1, 2018 · 6 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.12

Comments

@josharian
Copy link
Contributor

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 the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 1, 2018
@josharian josharian added this to the Go1.12 milestone May 1, 2018
@randall77
Copy link
Contributor

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
Copy link
Member

odeke-em 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
Copy link
Contributor Author

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
Copy link

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

@josharian
Copy link
Contributor Author

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

@gopherbot
Copy link

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
@ianlancetaylor
runtime: throw if the runtime panics with out of bounds index
f43aa1d 
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>
@golang golang locked and limited conversation to collaborators Jun 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
4 participants
@josharian @odeke-em @randall77 @gopherbot