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

panic mode where it aborts upon panic iff there are no catch_unwind calls to catch it #49032

Open
michaeleiselsc opened this issue Mar 15, 2018 · 14 comments
Labels
A-codegen Area: Code generation C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@michaeleiselsc
Copy link

michaeleiselsc commented Mar 15, 2018

The two options for panic handling, abort and unwind, seem insufficient to cover my use case. When I release my app to users, I use a crash reporter (catches crash signals with a signal handler, records the stack trace, and uploads it). If I use abort for panics, then I'll see the stack trace of the panic in my crash reporter, but I won't be able to catch panics. If I use unwind, then I'll be able to catch panics, but uncaught panics will unwind all the way to the FFI boundary (this is an app built in another language and calls into Rust). This means that I won't be able to see the full stack trace. I see a few existing solutions:

  • Use abort mode and never catch panics (this won't work for me personally, because there are places where I need to catch panics, e.g. for OOM)
  • Have a thread-local variable with the number of catch_unwind calls that I'm currently nested in. Use my own version of catch_unwind to update it. If this number is 0, then that means the panic is going to abort after some amount of unwinding, so just abort it right there. Otherwise, leave it alone because it will be caught gracefully. This will cause aborts if the catch_unwind calls are outside of my code base, but that doesn't seem too common

Any ideas what a good way to deal with this is?

@michaeleiselsc michaeleiselsc changed the title panic mode where it abort upon panic iff there are no catch_unwind calls to catch it panic mode where it aborts upon panic iff there are no catch_unwind calls to catch it Mar 15, 2018
@steveklabnik
Copy link
Member

This feels like a large enough feature to require an RFC, to me. @rust-lang/lang?

@nikomatsakis
Copy link
Contributor

Agreed, but I can see the utility of it.

@michaeleiselsc
Copy link
Author

@comex curious for your thoughts as another ios dev

@michaeleiselsc
Copy link
Author

this is basically copying objc exception behavior

@sfackler
Copy link
Member

I think this would just require skipping the catch_unwind before main and in thread startup, right?

@comex
Copy link
Contributor

comex commented Mar 16, 2018

It seems like ideally this would just be the default behavior on all platforms. In addition to working better with non-debugger crash reporting tools, this could improve the debugger experience by obviating the need to manually break on rust_panic. In the linked thread, there was a suggestion of having GDB and LLDB automatically add such a breakpoint, but this could be a better option, as it would avoid breaking in the debugger for panics that will ultimately be handled 'cleanly'. This isn't necessarily what the user wants, since panics are supposed to indicate programmer error even if something catches them, but it makes sense as a default, since merely attaching a debugger shouldn't force the user to deal with errors they may not care about. Or even if the default were to break on all panics, it would still be nice to have the ability to single out panics that result in aborts.

The problem, of course, is that we still want to run destructors before aborting, as it provides the user more control. It's just that at the moment the program kicks the bucket, we'd like to show external tools the original stack.

I wonder, is it possible to change libunwind to call destructors on top of the full original stack frame from the panic? i.e. currently, if fn caller() has an object on its stack with a destructor, and calls fn callee() which panics, if you break on the destructor, you see a backtrace like

    frame #2: 0x0000000100001109 a`core::ptr::drop_in_place::ha2e84b9e18d73acc + 9
    frame #3: 0x0000000100000c1e a`a::caller::h9be23d264b575721 + 46
    frame #4: 0x0000000100000c49 a`a::main::hed51cb165992ecb8 + 9

where the return address for caller is part of a special unwind trampoline that ultimately calls Unwind_Resume. But instead it could be something like

1. core::ptr::drop_in_place
2. Unwind_Whatever
3. rust_panic
4. a::callee
5. a::caller

callee would already be unwound in terms of running destructors, but it wouldn't actually be popped off the native stack yet. If the panic was caught, it could longjmp down to the catcher; if it needed to abort, though, it could do so with the original stack still visible.

Actually, this would not be good default behavior when a debugger is attached, since the user may want to inspect objects from outer stack frames and in this case they'd be already destructed. So in that case, the best approach would be the original proposal, i.e. just abort immediately if we would otherwise abort after unwinding. On the other hand, this seems ideal for dealing with most crash reporting tools that just give you a stack trace.

It also sounds difficult to implement, but it would be cool :)

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Mar 16, 2018
@whitequark
Copy link
Member

whitequark commented Mar 16, 2018

@comex I think this is really not going to work with libunwind. Consider pseudo-IR like this:

void @foo() {
entry:
  %obj = alloca i32
  invoke void @bar() to label %normal unwind label %unwind
unwind:
  %r = landingpad ... cleanup
  call void @drop_obj(i32 %obj)
...

Now let's say %obj gets placed into a callee-saved register. This means that the frame of @bar needs to be examined to restore the value of %obj at cleanup point. There are exactly two ways to do this:

  • Either you pop the frame of @bar off the stack, completely erase any unwinder state on the stack and restore the registers where they belong, and then passing %obj to @drop_obj is the job of the register allocator working on @foo;
  • Or you ask libunwind to give you the value of a specific register at a specific point (it can do that, unw_get_reg and unw_get_fpreg), and then you have to figure out exactly where in registers or on stack %obj ended up. In other words, you need stack maps! A non-managed language with stack maps is very odd indeed.

Now, you certainly could implement stack maps (note that you will need a custom LSDA too; right now rustc uses C++-compatible LSDA), but they bloat the code considerably, and I think that was an argument in the GC discussion. It's also an enormous amount of work and will place a large burden on lesser-used architectures.

That said, I only wrote all of the above for posterity (also because I happen to have the somewhat arcane knowledge of libunwind internals and wanted to save everyone the effort), because on Windows you need to be compatible with SEH and I don't think this is compatible with SEH at all. Looks like I'm wrong and SEH actually works like @comex wants, though I'm not sure.

@comex
Copy link
Contributor

comex commented Mar 16, 2018

In theory libunwind could restore most registers but treat the stack pointer specially (and the landing pad would have to be aware of this). But I guess I'm derailing things a bit, given that this is all orders of magnitude more complicated than the original proposal...

@main--
Copy link
Contributor

main-- commented Mar 16, 2018

Note that C++ does not run destructors for uncaught exceptions but instead invokes std::terminate() which ends up calling abort() by default.

It certainly makes sense considering that by far the most common and natural use for RAII is the encapsulation of kernel resources like memory allocations or file descriptors which are more efficiently cleaned up by simply letting the process die.

This may yield surprising behavior in the edge case where your destructors have some externally observable behavior (IO) but I feel like that's a reasonable price to pay.

@bjorn3
Copy link
Member

bjorn3 commented Mar 17, 2018

This may yield surprising behavior in the edge case where your destructors have some externally observable behavior (IO) but I feel like that's a reasonable price to pay.

I really want destructors to run in terminal apps to restore terminal back from raw mode.

@XAMPPRocky XAMPPRocky added A-codegen Area: Code generation C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jun 26, 2018
@vorner
Copy link
Contributor

vorner commented Jul 24, 2018

Hello. Is this still blocked on someone writing an RFC? If so, I'd volunteer to try writing one.

@marmeladema
Copy link
Contributor

I am also interested in this feature, for the exact same reasons as @michaeleiselsc mentioned in the introduction post.
Has any progress been made? I am not familiar at all with the RFC process but if there are some guidelines on how to create one for a newcomer, i can try.

@steveklabnik
Copy link
Member

So, #52652 is sort of the ticket that seems to be tracking this now; i believe this should be closed in favor of that, given that that one is up-to-date

@vorner
Copy link
Contributor

vorner commented Dec 9, 2019

Is it? That one is about panics across FFI boundary. This one is about panicking „out of stack“ completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests