RFC: Stabilize implementing panics as aborts

[alexcrichton]

• Stabilize the -Z no-landing-pads flag under the name -C panic=val
• Implement a number of unstable features akin to custom allocators to swap out
  implementations of panic just before a final product is generated.
• Add a [profile.dev] option to Cargo to disable unwinding.

Rendered

[nagisa]

NIT: we already have a system for boolean configuration switches in attributes. That’s cfg. I strongly propose changing the needs_panic_runtime thing to a cfg.

[ticki]

Agreed. No reason for not using the general, #[cfg].

[alexcrichton]

@nagisa can you elaborate on what you're thinking? Given the way these attributes are defined below I'm not understanding how this could be implemented with #[cfg].

[nagisa]

@alexcrichton This, not these. Turns out that this attribute is inverse of what I thought it is at first (i.e. it declares that crate wants a runtime, rather than describe the fact that runtime is needed). Never mind me here.

[nagisa]

Okay, I have a bunch of thoughts which I won’t be posting inline, because of GitHub’s tendency to lose the comments in the long run.

Overall, I think this RFC is incorrectly conflating what current -Z no-landing-pads does with the panicking mechanism used. Namely, all -Z no-landing-pads does is turn off translation of cleanups. Presence or lack of these cleanups is completely unrelated concept to the mechanism used to signal unrecoverable error, IMO. A bunch of my further comments is strongly related to this problem:

There will currently be two supported strategies:

• unwind - this is what the compiler implements by default today via the invoke LLVM instruction.
• abort - this will implement that -Z no-landing-pads does today, which is to disable the invoke instruction and use call instead everywhere.

Right, so I have two points here: with this RFC you cannot disable codegen of cleanups and continue to use the unwinding panic – which is what -Z no-landing-pads does currently. Another point is that it is possible to “register” cleanups on abort calls, namely, by catching the SIGABRT signal on UNIXes and running all the necessary cleanups then.

---

Any crate DAG can only contain at most one instance of #![panic_runtime].

Similarly, previously any crate in the dependency tree could opt-out from cleanups (as an optimisation). One crate using -Z no-landing-pads does not break other crates which have them. This RFC, as written currently does not allow for this use case. You have to go all-in into one single option in all crates – even those that do not need it.

That being said, I do see the necessity to only have a single panicking mechanism in a final artefact, so the comment basically boils down to lack of distinction between panicking mechanism and cleanup mechanism or lack thereof.

---

Finally, as the one who landed the NoLandingPads pass for MIR, I would like to see some more discussion in the RFC on how the compiler is supposed to decide whether to generate traditional cleanups or not. I suppose it could be as simple as checking whether -C panic=unwind, but given my comments above, I find that lacklustre.

[brson]

I have some minor reservations.

This only accomplishes half of the goal of creating no-unwind builds because it still uses a libstd with landing pads. Eventually we're going to get to a place where if you want this you are going to be using either a provided no-unwind std or building it yourself.

It's not clear to me how that future world will work with the new panic option in Cargo. In particular, I don't know how or if Cargo will be aware of which variant of std is in use.

If we're going to continue the pattern of #![needs_foo] traits to control the runtime composition, I want to change the #[allocator] attribute to #![provides_allocator] and #![panic_runtime] to #![provides_panic_runtime], to make the pairing of 'needs'/'provides' clear. It's easier to talk about as 'needs'/'provides' attributes. I realize these are unstable, but they will become defacto stable eventually. I might also want to generalize this pattern further to e.g. #[needs(allocator)] / #[provides(allocator)].

[aturon]

I like the overall thrust of this RFC, and am really excited to see abort semantics one step closer to reality!

My biggest concern is with the final drawback (which I talked with @alexcrichton about privately):

Applications may silently revert to the wrong panic runtime given the heuristics here. For example if an application relies on unwinding panics, if a dependency is pulled in with an explicit extern crate panic_abort, then the entire application will switch to aborting panics silently. This can be corrected, however, with an explicit extern crate panic_unwind on behalf of the application.

In particular, my feeling is that typically applications should be the one to choose the strategy, and all of their dependencies must be compatible with that strategy. (In the RFC, if any crate in the graph has abort semantics, the entire artifact does.) It's not hard to imagine pulling in some indirect dependency that silently changes from unwinding to abort, and I don't think that crate authors should be required to explicitly extern crate the desired panic strategy. If we're going to provide unwinding by default, I believe we should not silently change that default, at least for an application.

[aturon]

cc @wycats @nikomatsakis

[bstrie]

cc @jamwt, because Dropbox has long been a proponent of abort-on-panic.

[jamwt]

Yeah, I agree with having applications choose the strategy. As long as we're allowed to ensure that all panics abort (as users of various libraries but owners of the application), we're good.

[alexcrichton]

Oh wow, that was fast, thanks for the comments!

@nagisa

It's true that this RFC is diverging from what -Z no-landing-pads (an unstable option) strictly does today. It sounds like your main concern is preserving the ability to disable the generation of landing pads in a crate, but still continue to use unwinding as a panic strategy? I personally see that as a serious footgun that should be avoided at all costs. I at least couldn't come up with a use case for a crate where resources are intentionally leaked on panics. As a result, this RFC was written in a way to disallow this case entirely. Do you have a use case in mind though?

I'd be fine leaving the -Z no-landing-pads option lying around for just testing purposes, but in terms of a stable interface I'd want to have some more motivation before we stabilize the ability to intentionally compile a crate so it may leak resources on panics.

Another point is that it is possible to “register” cleanups on abort calls, namely, by catching the SIGABRT signal on UNIXes and running all the necessary cleanups then.

I'm not really sure how this is relevant, can you clarify? The whole point of the unwinding strategy on Unix today is that it's zero cost in the case where no exception is thrown, and if we had to register a handler every time we had a function call that... wouldn't be zero cost?

I will say, however, that if this is just an alternate strategy of handling panics, this RFC should account for that. We would implement another -C panic=sigabrt in the compiler, and that would codegen whatever was necessary to install handlers and such (and then we'd ship a panic_sigabrt crate with runtime support).

I would like to see some more discussion in the RFC on how the compiler is supposed to decide whether to generate traditional cleanups or not.

I suppose I didn't really discuss this as I'm not really sure what the discussion would be in the first place. My current interpretation of the state of MIR and such would be:

• MIR continues to have the concept of an "invoke vs call" similar to LLVM, where call instructions can return in an abnormal fashion to another basic block which has a bunch of cleanups.
• For -C panic=abort we'd run the NoLandingPads pass you've implemented.
• For any -C panic=foo where foo is not "abort", we wouldn't run NoLandingPads and the MIR would looks the same regardless of panic strategy.
• When lowering to LLVM, call instructions that have an exceptional return edge (to a cleanup block) would be translated appropriately depending on the value of -C panic. For example -C panic=unwind means that we'd use the LLVM invoke instruction.

Does that make sense? Did you have other concerns? I'd definitely want to make sure that any MIR plans align well with this RFC!

---

@brson

This only accomplishes half of the goal of creating no-unwind builds because it still uses a libstd with landing pads. Eventually we're going to get to a place where if you want this you are going to be using either a provided no-unwind std or building it yourself.

I think that when rustup.rs is ready and we're distributing/managing multiple compiled modes of the standard library we'd still basically want almost everything in this RFC. The first alternative mentions this explicitly by indicating that one major change could be disallowing mixed panic modes in a DAG entirely (instead of allowing a panic runtime to link with crates compiled any which way). Are you thinking, however, that other parts of this RFC would change in the face of such a distribution mechanism?

Also, the only drawback I know of with linking an unwinding libstd with an aborting application is that the object code in the standard library itself may have missed optimization opportunities based on the fact that functions will never unwind. Do you know of other drawbacks, however? Much of this RFC is foundationally built upon the idea that linking aborting applications to the libstd binaries we ship is good enough in basically all situations.

It's not clear to me how that future world will work with the new panic option in Cargo. In particular, I don't know how or if Cargo will be aware of which variant of std is in use.

One critical portion of this RFC is the validation that the compiler performs to ensure that any sort of weird situations like this can't cause problems at runtime. For example the possible scenarios I see here are:

• If you have an unwinding std and panic = 'unwind' in Cargo, everything will link A-OK
• If you have an unwinding std and panic = 'abort' in Cargo, everything will link A-OK
• If you have an aborting std and panic = 'abort' in Cargo, everything will link A-OK
• If you have an aborting std and panic = 'unwind' in Cargo, then a few things are possible:
  • In reality, we'd probably have extern crate panic_abort included in this build of libstd. That way you'd get an error immediately if you tried to compile anything with -C panic=unwind and link to this libstd.
  • Otherwise, if we didn't include this, the compiler will auto-choose aborting panics. This sounds pretty bad, and is essentially @aturon's concern, and is convincing me that we may want a more principled error in this case.

Basically, the compiler should always be able to provide a principled error message in these situations. Along those lines I'm not really sure what the negative interactions would be between Cargo and choosing the right libstd, but we should be able to relatively easy encourage the user to select an appropriate toolchain.

Did you have other concerns in mind, however?

If we're going to continue the pattern of #![needs_foo] traits to control the runtime composition, I want to change the #[allocator] attribute to #![provides_allocator] and #![panic_runtime] to #![provides_panic_runtime], to make the pairing of 'needs'/'provides' clear. It's easier to talk about as 'needs'/'provides' attributes. I realize these are unstable, but they will become defacto stable eventually. I might also want to generalize this pattern further to e.g. #[needs(allocator)] / #[provides(allocator)].

Yeah I'd be fine with basically anything here. I didn't put too much thought into it because they're unstable. I somewhat disagree that these will ever become defacto stable (in the panicking case). It's essentially impossible to write your own panic runtime without compiler support, so I don't think we'll see many of those any time soon.

Writing a custom allocator, however, that may indeed hasten the defacto stability!

---

@aturon

If we're going to provide unwinding by default, I believe we should not silently change that default, at least for an application.

And to clarify, this RFC does allow for the case of any crate to explicitly choose a panic strategy via extern crate panic_{abort,unwind}. Along those lines, the worry here is that projects may assume that the default unwind strategy is getting used without ever asserting that it's getting used. This RFC then will subvert expectations and silently switch to panic=abort if any crate in the DAG links to panic_abort and the panic_unwind crate is never mentioned.

This, coupled with @brson's concerns, I believe, have convinced me that this needs to be a first class compiler error. Specifically if a final artifact is being produced the compiler will assert that the panic runtime selected is compiled in the same panic mode as the final artifact itself.

---

@jamwt

As long as we're allowed to ensure that all panics abort (as users of various libraries but owners of the application), we're good.

You can indeed! It should be as simple as:

extern crate panic_abort;

And then you're done!

[bstrie]

Did you mean panic = abort here? (Also, did you mean "naive" instead of "native"?)

If not, then can you elaborate why specifying the same unwinding strategy as the default would force a recompile?

[alexcrichton]

Gah oops, this is a hilarious combination of multiple types. Correct on both counts!

[bstrie]

To clarify, does #![needs_panic_runtime] solely exist to make it trivial to detect cyclic dependencies?

I also agree that this pattern of paired needs_foo attributes is begging for a more general solution.

[vadimcn]

Regarding the need to ship two versions of std: Hypothetically, if LLVM extended to other platforms the Windows model of outlining cleanups into separate funclets, those could be placed into separate sections, and stripped out by the linker when not needed.

Edit: then again, the same could be achieved with -Clto...

[alexcrichton]

To clarify, does #![needs_panic_runtime] solely exist to make it trivial to detect cyclic dependencies?

That's part of it, but it also enables the logic in the compiler to auto-inject a runtime if one isn't found. This is essentially solving the case where so long as you never use the standard library there's no need for a panic runtime to ever get injected.

[brson]

Are you thinking, however, that other parts of this RFC would change in the face of such a distribution mechanism?

Yes. As I mentioned, I don't see how the cargo option aligns with a world with custom stds. There will be some - yet undesigned - mechanism in the future to specify what combination of codegen/runtime features that users want the entire world to be compiled with. This new panic option in cargo is just one facet of that. I am concerned that it will be obsoleted by a more general mechanism in the future.

I am further concerned that baking the unwinding configuration into the crate's configuration that we are opening the door to crates being able to specify incompatible codegen/runtime options.

Things that need to be specified globally in the future:

• CPU architecture
• cogegen options like SSE, PIC, etc.
• unwinding runtime strategy
• unwinding codegen strategy
• allocator
• debuginfo

There are probably more I'm forgetting right now. These are all related and we should have a plan to solve this issue of custom codegen/runtime components generally, not piecewise.

Do you know of other drawbacks, however?

Bloat.

[Amanieu]

How does this interact with panic handlers? Will panic handlers be called before an abort?

[ticki]

@Amanieu No, there is no unwinding, destructors, etc. involved (hence 'abort').

[Amanieu]

I would like to propose an alternative design for this feature, just to throw the idea out there.

The main issue with -Z no-landing-pads is that it can easily result in undefined behavior for two reasons:

• Unwinding from a nounwind function is undefined behavior.
• A lot of unsafe code relies on destructors running to ensure safety.

However we could make -C panic=abort generate landing pads that simply call an abort function. This is the same mechanism that C++ uses to implement noexcept (calls std::terminate if an exception leaks out of a noexcept function).

Benefits:

• No need to create multiple crates implementing panic and the complex machinery that goes with it.
• Compile times should be pretty fast since landing pads are just a single call.
• In many cases the landing pad can be optimized away if a function only calls nounwind functions.
• Even if the landing pad is not optimized away, code generation is the same as if there was no landing pad since it never returns.

Drawbacks:

• libstd is still compiled with unwinding enabled, so panics inside libstd might execute a few destructors before the process is aborted. This shouldn't be a problem in practice though.
• There is still some code size overhead due to the calls to abort in the landing pads, but it should be much smaller than the current landing pads.

[nagisa]

@alexcrichton

It sounds like your main concern is preserving the ability to disable the generation of landing pads in a crate, but still continue to use unwinding as a panic strategy?

My main concern is conflation of panicking and cleanup strategies in this RFC. Every other major issue I have with the RFC stems from that.

I at least couldn't come up with a use case for a crate where resources are intentionally leaked on panics. As a result, this RFC was written in a way to disallow this case entirely. Do you have a use case in mind though?

Any crate at the branches of call tree could use the option of explicitly leaking resources as an optimisation. As stated in the RFC itself, the cleanups are not entirely free – they consume binary space in the final executable, they increase compilation times significantly. They also compete for CPU cache, decrease instruction locality and so on. I do not see e.g. numerics crate caring about a leaked vector or two if that helps to avoid generating a bunch of code near success pathes in computation-intensive functions.

I'm not really sure how this is relevant, can you clarify?

My only point here that conflating panicking and cleanup strategy doesn’t make much sense. I didn’t intend to claim it is a sensible cleanup strategy though.

For any -C panic=foo where foo is not "abort", we wouldn't run NoLandingPads and the MIR would looks the same regardless of panic strategy.

Right, so for some fictional process::exit(101) panicking strategy somebody implements for their own use-cases we would still generate unnecessary cleanups.

---

@Amanieu

Unwinding from a nounwind function is undefined behavior.

But we do not generate nounwind functions and it is crate author’s responsibility to catch panicks at FFI boundaries?

However we could make -C panic=abort generate landing pads

I do not see how it makes sense to generate landing pads at all if we panic by abort()ing in the first place. Are you suggesting we keep our only panicking strategy we have currently and focus on implementing multiple cleanup strategies instead?

[sdroege]

At the application level can make sense, but what about libraries that want to opt-out because e.g. they can't recover from assertions. But what if you combine such library with applications that do want recoverable panics because they don't want to take the whole process down?

[alexcrichton]

Unfortunately that's essentially a situation which has no solution. The crate which wants to force panic=abort would link to panic_abort while the application forcing panic=unwind would link to panic_unwind. The compiler would detect this situation and generate an error.

I don't currently know of a way to make that kind of situation succeed to compile.

[phil-opp]

Just a small nit: The PR summary comment uses a -C unwind=val flag instead of the proposed -C panic=strategy.

[matthieu-m]

I have some concerns about the ability of an application to make an informed choice about the panic strategy.

Specifically, I am thinking that as a crate author I could perfectly use speculative execution and catch panics in case of issue. Today, from a user's point of view, this is completely transparent. Tomorrow, if the user decides to specify panic=abort, then my crate will halt her application (sometimes).

Worse, of course, is when a crate that previously was never generating/catching a panic suddenly starts doing so; as an application, it means that I need to audit each new version of the crates I pull to ensure their compatibility with panic=abort.

In essence:

• I am seeing an implementation detail leak here => I now need to know whether a crate requires unwinding to work correctly or not
• I am not seeing any control mechanism

To me, as a crate author, whether I am willing to commit to support panic=abort should be an explicit choice, in the same way that marking a function const is.

[steveklabnik]

Rust already never guarantees destructors run though, so you can't make that kind of assumption. And when setting panic to abort, isn't "halt her application" what she'd want in the first place?

On Feb 27, 2016, 10:40 -0500, matthieu-mnotifications@github.com, wrote:

I have some concerns about the ability of an application to make an informed choice about the panic strategy.

Specifically, I am thinking that as a crate author I could perfectly use speculative execution and catch panics in case of issue. Today, from a user's point of view, this is completely transparent. Tomorrow, if the user decides to specifypanic=abort, then my crate will halt her application (sometimes).

Worse, of course, is when a crate that previously was never generating/catching a panic suddenly starts doing so; as an application, it means that I need to audit each new version of the crates I pull to ensure their compatibility withpanic=abort.

In essence:

I am seeing an implementation detail leak here =>I now need to know whether a crate requires unwinding to work correctly or not
I am not seeing any control mechanism

To me, as a crate author, whether I am willing to commit to supportpanic=abortshould be an explicit choice, in the same way that marking a functionconstis.

—
Reply to this email directly orview it on GitHub(#1513 (comment)).

[main--]

Specifically, I am thinking that as a crate author I could perfectly use speculative execution and catch panics in case of issue.

@matthieu-m Of course you can do so if - and only if - you choose to ignore the warning in the std::panic::recover docs that tells you not to do this.

Perhaps "panic=abort mode" would finally provide a strong motivation against misusing recover as it shows directly what issues doing so can cause.

---

Now to my general comments on the RFC:

• For -C panic=abort we'd run the NoLandingPads pass you've implemented.
• For any -C panic=foo where foo is not "abort", we wouldn't run NoLandingPads and the MIR would looks the same regardless of panic strategy.

It's unclear to me which direction this design really chooses: While some parts of the RFC ("There will currently be two supported strategies") suggest that the set of available is clearly defined and very definitely not arbitrarily user-extensible, some of the discussion (e.g. the part I'm quoting here) appeared to be about user-defined panic strategies. This second version is also implied by the general design around (seemingly) arbitrary crates.

The major difference between this area and memory allocators is that the latter generally don't require any specific language support and hence can be implemented purely as libraries (crates). This approach doesn't really seem viable for panic strategies as these care about code generation (things like invoke vs call) and are therefore always tied to hardcoded paths in the compiler unless the design is powerful enough to allow each strategy crate to hook into these compiler paths on its own, e.g. by being loaded as a compiler plugin.

So to sum things up, I don't see how the current design benefits (at all) from utilizing a generic crate-based solution (versus e.g. just a compiler flag and some special piece of crate metadata that can be checked for compatibility at link-time), so I obviously don't think the downsides (conflicts etc) are worth it.

[arielb1]

rustc would like to not use unwinding, but print a nice ICE message before aborting. Is that going to be supported?

[wycats]

Specifically, I am thinking that as a crate author I could perfectly use speculative execution and catch panics in case of issue. Today, from a user's point of view, this is completely transparent. Tomorrow, if the user decides to specify panic=abort, then my crate will halt her application (sometimes).

One of the biggest concerns I (and others) had with stabilizing recover is that it would cause crate authors to believe that they should use it in this way.

I emphatically do not believe that crate authors should use recover in order to save themselves from dealing with intermediate Results. We have worked hard to make Result-based propagation ergonomic and to encourage the use of Results for handleable errors.

The idea behind panic! in Rust is to allow the propagation of fatal errors that can only be reasonably handled by destroying the entire isolation unit that they are a part of (threads, FFI boundaries or processes). By destroying an entire isolation unit, we encourage a programming model where integrity bugs (out-of-bounds access, integer overflow) do not propagate into the rest of the program.

On the other hand, handleable errors are expressed using Result, which can always be handled locally. By definition Result can't "do anything" to the caller, while exceptions can.

Is is a feature not a bug that this RFC discourages crate authors from using panics to represent handle-able errors. If Result is still not sufficiently ergonomic after ?, we should work to improve the ergonomics of handleable errors, not give up and let panic! become a handle-able error (as it is in Go).

[bstrie]

I'm going to sound like a broken record here, but the confusion over whether recover would work with aborting is why I preferred to call it recover_from_unwinding so as to be maximally explicit at the capability that it provided (as well as a longer name further discouraging casual misuse).

[steveklabnik]

I like that name 👍

On Feb 27, 2016, 21:30 -0500, Ben Striegelnotifications@github.com, wrote:

I'm going to sound like a broken record here, but the confusion over whetherrecoverwould work with aborting is why I preferred to call itrecover_from_unwindingso as to be maximally explicit at the capability that it provided (as well as a longer name further discouraging casual misuse).

—
Reply to this email directly orview it on GitHub(#1513 (comment)).

[rillian]

@alexcrichton

Specifically it sounds like for panicking Gecko would desire a standard library which:

• Is compiled with panic=abort, removing landing pads
• Removes all panic messages, reducing size

Yes. It's perhaps worth distinguishing, though, between size issues and behaviour issues. It's easier to take special optimization steps on build infrastructure, so a higher bar is possible there. But as you say, this is something that can benefit many projects, so making it easy is advantageous.

[alexcrichton]

@rillian yeah I agree that separating them out is important. Behaviorally I believe Gecko needs no more than what this RFC provides. This provides a guaranteed ability to translate all Rust panics to aborts rather than stack unwinding (even through a custom method via your own panic runtime).

The optimization steps, specifically for size, are tangential infrastructure related problems on our end which we intend to tackle eventually anyway :)

[aturon]

The lang team did not have full quorum this week and will consider this RFC during next week's meeting; remaining in FCP until then.

[nrc]

It seems that there is some kind of false genericity here - you can only really choose between two options, you can't plug in your own strategy. That would require some mechanism for augmenting the compiler (pluggable MIR pass?).

I'm a bit unclear what would happen with -Cpanic=foo. Would the compiler pick one of its codegen strategies and look for a panic_foo crate? Or give an error that foo != {abort | unwind}? I would prefer the latter until we stabilise a mechanism for augmenting the compiler nicely.

However, as a step towards a fully generic mechanism, this RFC looks good. in particular, adopting a somewhat future proof compiler flag. The attributes sound like the right direction too, though it sounds like there will be some future discussion there.

[alexcrichton]

@nrc yeah this isn't intended to be any form of "truly general mechanism" or something like that, the compiler will reject any -C panic=foo arguments when it doesn't recognize foo. One could create multiple -C panic=abort runtimes, however, which differ slightly in exactly how the process is aborted.

[Ericson2314]

Will just libcore have #![needs_panic_runtime]?

[alexcrichton]

No, the standard library will have that attribute as that's where panics are implemented.

[Ericson2314]

The additional lang items for core panics don't strike me as something we want forever. But if this RFC's proposals don't get stabilized until a more general needs/provides mechanism is investigated, I guess this is fine for now.

edit heh and that's basically what I wrote a month ago #1513 (comment) So +1 for RFC if we tree treat it as experiment/stopgap. -1 for longterm as is: besides services pattern without proper support, rustc is now further entangled with std lib architecture which isn't something we generally want.

[Diggsey]

WRT. the needs_X convention, perhaps a more general solution would be to use the existing trait system:

The idea is that a single downstream crate will be able to "bless" a particular trait implementation. Upstream crates will be able to implicitly use the yet-to-be-determined "blessed" implementation by simply calling the trait methods: Trait::do_x() (or a more explicit syntax, eg. <Blessed as Trait>::do_x() where Blessed is a lang item)

This would normally be an error, because the compiler wouldn't be able to determine which implementation of Trait to use based on the parameter or return types, but the "blessed" trait implementation would act as a fallback in these cases.

The exact mechanism for blessing a trait could vary, but an easy option is to mirror the existing attributes:
#![needs_blessed(Trait)]
#![bless(Trait, Type)] (requires that Type: Trait)

The advantages are extensibility and proper namespacing of blessed items. Additional fun could be had by allowing Trait to be generic...

[nikomatsakis]

It's official! The @rust-lang/lang team has decided to accept this RFC.

[nikomatsakis]

Tracking issue: rust-lang/rust#32837

If you'd like to keep following the development of this feature, please subscribe to that issue, thanks! :)