-
Notifications
You must be signed in to change notification settings - Fork 10
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
Demo currently doesn't work. #22
Comments
This happens because the caller address is outside the text region we find. The ld find_cfi method calculates the text region by looking at the first PT_LOAD. Unfortunately, in this binary's case, the first PT_LOAD is not the executable, but a read-only bit. So the text region we find is actually not the .text, but some other segment. I believe we should use the flags to find the executable section.
|
Fixed the above problem with #23. Unfortunately, it's not enough. I end up executing an ud2, causing an illegal instruction exception. Freaky. Here's the backtrace from gdb:
And the asm:
The binary is at https://dl.roblab.la/demo , and verbose logs in the details below. I'm going to sleep on this for now, hoping for a stroke of genius.
|
looks like the line it's dying on is https://github.com/gimli-rs/unwind-rs/blob/master/unwind/src/libunwind_shim.rs#L148 if that helps at all. |
Alright so after tracing execution many times in GDB, it seems like a problem un _Unwind_Resume. The unwind_lander somehow ends up restoring the stack right there: https://github.com/gimli-rs/unwind-rs/blob/master/unwind/src/libunwind_shim.rs#L114. Which obviously causes all sorts of issues. It's supposed to be skipping over this stack, so I guess we're doing something wrong in the code that's skipping over the frames in the resume. |
FWIW I bisected this using rustup: Good: 1.32.0-nightly (4a45578bc 2018-12-07) Sadly, there are no nightly builds from Dec 08 to Dec 12 I had a very quick skim over the 131 commits between those two but nothing obvious stuck out. |
The unwinding all looks fine to me, but, from looking at the MIR/LLVM-IR, rust is generating an abort for the unwind cleanup in |
I don't understand how cleanup of This is related to (This is based on my memory from a couple of hours ago, I haven't double checked the details.) |
Oh of course! This is rust-lang/rust#55982 ! Adding the allow-unwind attribute to all those extern functions is certainly the correct fix. From what I remember the current implementation just hopes that the unwinding code never reaches high enough on the stack to actually touch the unwinder object. Of course this is bad and should be replaced with a proper solution eventually. |
We only need to add the allow-unwind attribute to Another option would be to get the context immediately in |
Agreed.
The problem I see with skipping frames is inlining. I would much rather have a single clean cut as we do right now, where everything below that is getting unwound and everything above is not. |
Right. I was misunderstanding things a bit. But still, I think we could change where that single clean cut is, so that it is below the cleanup that aborts. And we could do that by getting the context immediately in |
Probably should leave this open, because it's going to break on stable rust soon. |
I might be missing something here, but why not just modify |
Yep I've been looking into doing that.
This is true for
Not sure I follow. The |
I don't see how this is possible, considering that the closure is passed to a function on the unwinder object. Basically I'm unsure if the way I implemented But if any of these assumptions are incorrect perhaps we should go for a totally different solution. If possible, I would like to keep At this point I feel like at least for Rust we can just |
Yes it is, and this is what causes the problem in this issue. Without |
When running the demo example on my machine (64-bit linux) using a new-ish nightly (rustc 1.33.0-nightly (b2b7a063a 2019-01-01)), unwind-rs crashes.
Did something break?
The text was updated successfully, but these errors were encountered: