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

Is unwinding from lang_start really UB? #107381

Closed
ChrisDenton opened this issue Jan 27, 2023 · 9 comments
Closed

Is unwinding from lang_start really UB? #107381

ChrisDenton opened this issue Jan 27, 2023 · 9 comments
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows

Comments

@ChrisDenton
Copy link
Member

A comment in the std claims that unwinding past lang_start is UB:

rust/library/std/src/rt.rs

Lines 136 to 145 in 6874f4e

// Guard against the code called by this function from unwinding outside of the Rust-controlled
// code, which is UB. This is a requirement imposed by a combination of how the
// `#[lang="start"]` attribute is implemented as well as by the implementation of the panicking
// mechanism itself.
//
// There are a couple of instances where unwinding can begin. First is inside of the
// `rt::init`, `rt::cleanup` and similar functions controlled by bstd. In those instances a
// panic is a std implementation bug. A quite likely one too, as there isn't any way to
// prevent std from accidentally introducing a panic to these functions. Another is from
// user code from `main` or, more nefariously, as described in e.g. issue #86030.

See: #86034, #86030, #86027.

This is surprising because C++ panics escaping main is well defined to terminate (and will trigger an attached debugger, which is useful).

cc @Amanieu, who questioned this assertion.

@ChrisDenton ChrisDenton added the A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows label Jan 27, 2023
@digama0
Copy link
Contributor

digama0 commented Jan 27, 2023

cc: @rust-lang/wg-unsafe-code-guidelines I think?

@Amanieu
Copy link
Member

Amanieu commented Jan 27, 2023

Additionally, I believe there is some benefit to making unhandled panics call abort instead of returning 101 as they currently do. This raises a SIGABRT which will trigger an attached debugger and also create a core dump at the point where the exception was thrown, which is helpful for post-mortem debugging of panics.

@CAD97
Copy link
Contributor

CAD97 commented Jan 27, 2023

At least before the C-unwind work, it was theoretically UB, because there was no guarantee that Rust's unwinding mechanism was compatible with the platform unwinding interface.

Even if it's not UB to unwind out of the start function, we still want to set our own catch, as an uncaught C++ unwind will terminate without running destructors, which we want to do. Additionally, the unwind failing to start will cause an rtabort!... probably; it depends on whether __rust_start_unwind is defined to diverge or return an error code, and I don't recall atm.

There's essentially three options:

  • The start function is extern "C", and gets the #[rustc_nounwind] shim. Unwinding out of start is defined to unwind the stack then abort.
  • The start function is extern "C-unwind". The behavior of an unwind escaping is target defined.
    • On targets where the Rust unwind mechanism differs from the native one, a shim is used to translate. We always unwind the stack to the shim, and then the platform semantics take over when we raise the platform unwind.
    • On targets where the Rust unwind mechanism matches the native mechanism, behavior at the unwind start point is defined by the platform. It's probably an unwind failure leading to rtabort, but it could also unwind somewhat gracefully out of the start function, or it could just be UB if the platform does not define what happens1 in this case.
    • On targets without a native unwind mechanism, it's just UB.
  • The start function is extern "Rust". The previous, but now platforms where the Rust unwind doesn't match the native unwind are also just UB.

... or for #[lang = "start"] at least, just catch the unwind and don't worry about it; this is a private implementation detail not intended for stabilization. Though we should decide what the behavior for #[start], since that is more aimed towards potential stabilization.

My initial bias is that it should be extern "C" and get those same semantics (unwind to and then rtabort).

will trigger an attached debugger

Any debugger somewhat aware of Rust should probably have a default breakpoint on start_panic as well. I'd personally rather do the unwind and run drop cleanup up to the entry point if we're going to handle unwinds from start, which means the abort core dump would always point there.

Footnotes

  1. This could reasonably happen for a platform that has a C entry point that's not allowed to unwind, and the C++ runtime implements that to set up an unwind landing pad and then call the C++ main. The major platforms tend to just treat compiled C and C++ objects as the same thing, though, and allow unwinding through C stack frames despite this not being defined by the C language2.

  2. ... kinda sorta, caveats around setjmp/longjmp notwithstanding.

@ChrisDenton
Copy link
Member Author

For context, this came up when looking at #34502 (a Windows issue from 2016).

@RalfJung
Copy link
Member

RalfJung commented Jan 27, 2023

This is surprising because C++ panics escaping main is well defined to terminate (and will trigger an attached debugger, which is useful).

Panics escaping main are well-defined in Rust, too. So Rust works like C++ here.

Rust's lang_start corresponds to the hidden start item in a C++ runtime, not to C++ main.

@Amanieu
Copy link
Member

Amanieu commented Jan 27, 2023

The goal here is to be able to detect, before throwing an exception, that there are no exception handlers on the stack. This enables the following:

  • On Windows, this triggers the debugger due to an uncaught exception. Since this happens before the exception is thrown, the debugger has access to the full stack leading up to the original panic.
  • On UNIX we can call abort which raises a SIGABRT. This has 2 effects:
    • It allows a debugger to handle the abort, just like on Windows.
    • It allows us to generate a core dump which contains the full stack leading up to the panic.

Even if it's not UB to unwind out of the start function, we still want to set our own catch, as an uncaught C++ unwind will terminate without running destructors, which we want to do.

This is a good point, and may have visible consequence such as skipping the cleanup of temporary files & directories. We should take this trade-off into account when making a decision.

Additionally, the unwind failing to start will cause an rtabort!... probably; it depends on whether __rust_start_unwind is defined to diverge or return an error code, and I don't recall atm.

This is something that we can easily change since it's an implementation detail of the panic machinery.

@CAD97
Copy link
Contributor

CAD97 commented Jan 28, 2023

If this is a case we want to have different behavior for, then it should probably be exposed to the panic handler as well. Ideally, giving the handler the power to choose between unwinding or not1. Independent of unwinding ABI, this could be accomplished by a thread-local count2 of how many catch_unwind are registered3.

Changing from a clean exit with failing exit code to a signal exit on unixes AIUI is a fairly small change in behavior, all other things being equal. On Windows, it's a much larger change, being the difference between the program exiting or a pop-up saying the program crashed.

The main question is thus: is a panic unwinding from Rust fn main considered a "clean failing exit" or a "crash" by the language? Secondary question: if "crash", is it a crash when the unwind exits main (after running unwind cleanup) or when the unwind is raised (before running unwind cleanup)?

Once those questions are answered, the implementation details (i.e. what it means to unwind from #[start]4 or #[lang = "start"]) can be chosen in service of that.

My initial bias is that it should have the same behavior as fn main() -> Result returning Err. Whether that's a "clean failing exit" or "crash" I don't really care, so long as the choice is the same. This even goes for a JIT Debugger pop-up on Windows; debuggers are outside the scope of the language, and having a debugger prompt before unwinding is perfectly acceptable in my book so long as if the debugger prompt is dismissed (e.g. by having JIT debugging disabled) the unwind cleanup still happens.

Footnotes

  1. On unixes, this is easily accomplishable, since the desired behavior is just an abort() call. On Windows, by my quick research, __fastfail() should trigger the JIT debugger.

  2. Yes, I'm aware that we've worked to make the no unwind case of catch_unwind as free as possible, and this would regress that.

  3. If we want the same behavior for #[rustc_nounwind] and/or nounwind destructors, entering/exiting one would need to temporarily reset the catch_unwind count to 0, again making what's supposed to be essentially free, not.

  4. Since it's supposed to be exactly the low-level entry point of the executable, I'd say #[start] needs to be defined as extern "C" or extern "C-unwind", and if it's extern "C-unwind", the behavior of raising an unwind which would escape is "target undefined" (target defined, where undefined is a possible choice).

@bjorn3
Copy link
Member

bjorn3 commented May 17, 2023

When using a crate which enables raw mode and disables it again in a drop function, turning panics on the main thread from unwinding to an immediate abort will mess up the terminal in such a way that you have to blindly type the command to reset the terminal state back to cooked mode.

@RalfJung
Copy link
Member

So the answer to the question is yes, this is really UB. I'll close the issue as I don't think we have a lot of choice here; we can't let unwinding escape beyond the start item as we cannot rely on the surrounding context supporting unwinding. If someone wants to change this, please open a new issue with rationale and explaining how it could be achieved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows
Projects
None yet
Development

No branches or pull requests

6 participants