Support different panic strategies.

[ehuss]

The current implementation does not support different panic strategies, and is hard-coded for unwind. This is a tricky and difficult issue, and there will need to be some investigation and experimentation to figure out all the details. Some things to think about:

• How will cargo know which implementation to use? Somehow inspecting the profile?
• Should cargo always build both unwind and abort?
• -C panic=abort must be passed for the panic_abort crate.
• Some targets default to abort, need to figure that out.
• When building abort crates, cargo currently rebuilds them with unwind when built with the libtest harness. Does this mean the entire standard library would need to be built twice? This needs more thought, as it is already subtle and finicky.

There is also some hard-coded knowledge in Cargo about building the panic-unwind/abort crates, due to the way the compiler works (just relying on the std feature flags wasn't enough). Alex mentioned a desire to remove this, but I'm not sure how.

[alexcrichton]

My opinion on this is that Cargo should largely be able to handle all of this transparently without users having to configure anything. My thinking is that users specify a panic setting in their profile, and then Cargo would automatically respect that and handle everything else, so you'd just say build-std and everything would auto-magically work.

Now obviously like everything else the devil is in the details, so let me be a bit more clear about how I think all this can work.

• Cargo is in control of every single crate in the crate graph, unlike today where libstd is precompiled. This gives Cargo the ability to compile every piece of code in the exact same panic profile. This, for example, lifts the restriction that panic_abort must be compiled with -C panic=abort. That's only done because we ship a precompiled copy, and the whole point of linking to it is to be a panic=abort implementation. If Cargo compiles everything with panic=unwind then if the compilation of the panic_abort crate happens it's technically just wasted time since it'll never be used.

• Eventually (hopefully soon) Cargo will be able to take a much more pure stance with respect to the test crate, assuming everything is abort or everything is panic, no questions asked. This is still a work in progress but if that's the last remaining issue here and it still isn't over the finish line I don't mind finishing the implementation myself :)

• Ideally we're compiling precisely one panic_* crate, and I think that's ideally configured via some sort of feature on the std crate itself. Right now there's a panic-unwind feature which controls whether panic_unwind is made available and linked in. If you compile libstd with panic=unwind you're required to compile in the panic-unwind feature/crate. Ideally with this feature we'd skip the panic=abort crate. Conversely if you're compiling with panic=abort then we can disable this feature and just build the panic_abort crate. Overall, I'm envisioning some sort of contract between libstd and rust-lang/rust's manifests, and we'd both invent the contract and uphold it when building with Cargo.

• The point about target-specific abort settings is pretty tricky. Almost all use cases of building libstd from source are panic=abort targets with no support for unwinding to boot. I think in these cases we should probably fix this at the source level in rust-lang/rust to make panic=unwind and panic=abort basically the same thing. By this I mean that on these targets the panic-unwind crate should do nothing but just depend on panic_abort and reexport it where panic_abort is the actual panic runtime. That way whether or not Cargo thinks it's compiling in panic=abort or panic=unwind mode you get the same code. (the thinking here is we just want "things to work by default")

How's that sound? Does that cover the concerns you were thinking of @ehuss?

[alexcrichton]

So testing this out a bit today, you can actually get this sort of working today by configuring panic = 'abort' in your profile and passing -Z build-std=panic_abort,std. The compiler will resolve the panic_abort crate at compile time, which means that this only actually works if Cargo passes --extern panic_abort=.... That's sort of unfortunate because it does mean that Cargo would have to hardcode the names panic_unwind and panic_abort which I'd ideally prefer to leave as internal details of libstd/rustc...

[ehuss]

Sorry if this is a dumb question. Are there targets that have a panic strategy of "unwind", but do not want to use libunwind? Such as libunwind does not compile on the target, or the user wants to provide their own panic handler?

Alex, you mentioned having a feature to control whether the "panic_unwind" crate is enabled, but I'm hoping to support this without the user specifying features=["panic-unwind"]. I was wondering if just looking at the profile setting would be sufficient to know if Cargo should build panic_abort or panic_unwind. I'm guessing if the strategy needs to be independent of the actual handler used, that might cause a problem.

If looking at the profile is not sufficient, are there any other options than using features?

[ehuss]

Another question: The cross_custom test cannot build either panic_unwind or panic_abort, because neither of those crates support "os": "none". How should that be handled?

[alexcrichton]

In general a panic=unwind strategy requires so much compiler support that's basically impossible to implement a custom unwinding panic strategy. In that sense only targets officially supported by rustc (likely tier 1) and shipped with binaries will support unwinding panics, and yeah I think everything there wants to use the libunwind crate and C library in general for unwinding. (MSVC is a bit odd but "eh")

I think that we can use the panic mode configured in the profile to configure which feature of libstd is compiled, whether it has panic-unwind or not. (or at least that's what I was thinking). I believe that this should be sufficient for us to get everything working, so long as we also do a bit of legwork for the source code in libpanic_unwind to compile on all platforms (but abort).

[ehuss]

The reason I ask is because I have a branch that is switching back to using --extern instead of --sysroot. However, Cargo needs to know if it should pass --extern panic_unwind=… and/or --extern panic_abort=… or neither. With the current nightly, you can control this with the list of crates passed to -Zbuild-std. However, in my branch I also have explicit std deps in Cargo.toml instead of passing the crates on the command-line. But I assume we don't want people to be required to list which panic crate they want in [dependencies], so Cargo needs to have some way of knowing which to pass in.

Looking at the profile is fine if either unwind or abort is what you want, but what if you want neither? Such as the case mentioned above in cross_custom, where not even panic_abort will build.

Should panic_abort have some kind of fallback that at least compiles on all platforms? I assume it is not possible, since the abort intrinsic isn't available everywhere? And I assume an infinite loop would also be bad…

[alexcrichton]

Oh ok so if we're passing --extern it's a lot more tricky. Agreed that we can only "succeed" here if no one writes down panic_abort = { sysroot = true } in their manifest.

There's a subtle difference, but we can't look at the profile for --extern, but we can look at it for features enabled with libstd. With libstd today and the sysroot approach panic=unwind means may use the unwinding strategy, but actually uses the default for the target. Using panic=abort hardwires it to abort. By enabling the panic_unwind feature of std it makes it a candidate for usage, but the compiler may end up using the panic_abort crate anyway (which IIRC is always built). Using --extern we're trying to take the decision into Cargo's hands, but Cargo doesn't know what the default panic strategy is.

The only real solution I can think of for this is to just always compile both panic_abort and panic_unwind. We'd then pass --extern for both of them and basically leave it up to the compiler to figure out which it wants to use. This strategy would require that panic_abort and panic_unwind have fallbacks to compile on all platforms. The fallback would be always aborting, and I think we could probably get by with intrinsics::abort to be "good enough", and then select platforms would have ways of improving on that. (there's also panic implementations and such nowadays to handle all this).

If you're in a case where neither is wanted, I think we can automatically detect that based on whether std is used or not. This is the #![needs_panic_runtime] attribute, which is only attached to std, so if you don't compile std we could probably auto-infer to avoid panic_abort and panic_unwind.

Overall that feels like a lot of custom logic in Cargo for the panic runtimes, which I'm not really overly comfortable with. It's not necessarily the end of the world though?

[ehuss]

This is the #![needs_panic_runtime] attribute

Oh, I didn't know about that. I think that does make it easier. And if it is possible to have a fallback for all platforms in panic_abort, that also might help.

I'll try to update my prototype and see how it goes. Thanks!

[ehuss]

So it seems to be working great except for one wrinkle. If you have a project with panic="abort", it builds the standard library with the abort strategy. This causes a problem for running tests, since they require all dependencies to be unwind. What do you think of these options:

1. Always build the standard library with the "unwind" strategy.
2. Build the standard library twice (once with unwind, once with abort), like we do with all other dependencies.
3. Require -Zpanic-abort-tests to run tests with abort strategy.

I like 3, since it is simple and straightforward, and already works. I think 2 is doable, but I think will be very messy (it will need to create multiple std graphs, know which one to use, etc.), and is also really slow. I don't really know the consequence of 1.

Thoughts?

[alexcrichton]

Definitely (3) in my opinion!

I suspect that'll stabilize long before -Zbuild-std anyway, and for now I think it'd be fine if -Zbuild-std basically implied -Zpanic-abort-tests

[adamgemmell]

@ehuss Can you recall if anything new came up since this discussion? It seems like all that's needed is to plumb panic=abort from the profile into building panic-abort automatically and using -Zpanic-abort-tests (still unstable)

[ehuss]

I don't think anything new has come up. I think the comments above still capture all the questions that need investigation and experimentation.

[adamgemmell]

I'd glossed over the trickiness resulting from using --extern. Thanks!