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

More exports #248

Closed
main-- opened this issue Oct 3, 2017 · 7 comments
Closed

More exports #248

main-- opened this issue Oct 3, 2017 · 7 comments

Comments

@main--
Copy link
Contributor

main-- commented Oct 3, 2017

Right now, gimli::parser::Pointer is not exported. Because of that, it's actually not possible to get/use personality functions and lsda pointers.

I also needed gimli::parser::parse_encoded_pointer to parse the .eh_frame_hdr for my unwinder, but perhaps that code should just live inside gimli?

@fitzgen
Copy link
Member

fitzgen commented Oct 3, 2017

I'm working (in what little time I have) on a stack walker (not focused on actually unwinding the stack until walking it works well) as well: https://github.com/fitzgen/pancakes

Perhaps we should work together?


IIRC, .eh_frame_hdr isn't super reliable, but perhaps compilers have improved or I misremembered. In either case, it certainly seems like .eh_frame_hdr parsing belongs in this crate.

@main--
Copy link
Contributor Author

main-- commented Oct 4, 2017

Perhaps we should work together?

Sounds good!

What I have at this point (https://github.com/main--/unwind-rs) can find .eh_frame_hdr at runtime, parse unwinding info from there and finally walk and unwind the stack (properly with personality functions etc). This works well for both debug and release builds for standard Rust panics (I'm exporting all relevant libunwind entry points and the linker prefers my versions over the genuine dynamically linked ones), but the implementation is still kind of a prototype and not portable at all (yet?). Right now, I'm mainly working on performance (binary search on the .eh_frame_hdr table works already but it's not committed yet) and actually building an clean and usable API.

@fitzgen
Copy link
Member

fitzgen commented Oct 4, 2017

@main-- I factored out the dl_iterate_phdr bits into the https://github.com/fitzgen/findshlibs crate.

My original goal was to allow iterating over sections within each mapped segment, but that isn't possible with ELF/dl_iterate_phdr, just mach-o and dyld APIs.

So I think instead it makes sense to build a tiny crate on top of findshlibs that is less general than "give me all the sections in this mapped segment" and is instead just "give me all the eh_frame (and eh_frame_hdr) sections mapped into memory" which is possible to implement for ELF with PT_GNU_EH_FRAME.

When performing actual unwinding, or inspecting the stack from a debugger, we need all the registers. When quickly capturing a stack trace for a profiler, we generally only need a couple of the registers.

When unwinding the stack for a panic, we're in the same process as the stack that is being unwound. When implementing a debugger or profiler, it is likely that we are in a different process from the stack being walked, and are presumably using something like https://crates.io/crates/read-process-memory

The stack walking / unwinding should be generic / customizable enough to handle all these different scenarios.

Also, I strongly typed different kinds of pointers that we encounter, before or after applying the loaders offset, etc: https://github.com/fitzgen/findshlibs/blob/master/src/lib.rs#L51-L72 I find this very helpful for wrapping my head around what kind of pointer I'm dealing with at what times. It might make sense to pull these definitions out into their own crate and use them in gimli too.

Additionally, I have a TaggedWord type that tracks which registers we've successfully recovered at what point during unwinding. I notice that there were some unwrap_or(0)s inside unwind-rs, and I think TaggedWord (or even just Option<usize> or something) is a slightly better approach. https://github.com/fitzgen/pancakes/blob/master/src/tagged_word.rs#L9

So that's kind of where I wanted to go / what my plans were. All that said, pancakes doesn't actually work yet :-p But it does have traits and infrastructure that was tailored with these different scenarios in mind.

Oh, one final question: why write assembly rather than use getcontext? Ultimately easier and more portable?

So where do you want to go from here?

@main--
Copy link
Contributor Author

main-- commented Oct 4, 2017

"give me all the eh_frame (and eh_frame_hdr) sections mapped into memory"

AFAIK .eh_frame_hdr is the only way to find .eh_frame since dl only gives you ELF segments, not sections.

I don't actually have a clear understanding of how you're supposed to do this part properly, especially since gimli's BaseAddresses wants text+data+cfi base addrs. What I'm doing right now is: For EhFrameHdr, set CFI to .eh_frame_hdr. This somehow yields a correct eh_frame_ptr so who am I to complain. But before we declare victory, note that there is no length. Of course I can just make it infinitely large and call it a day but there has to be a better way to do this.

Now for all the EhFrame stuff, the CFI base is suddenly .eh_frame. Makes sense, but I still spent like 4 hours wondering why my unwinder is setting registers to incorrect values.

I can even use the same BaseAddresses to decode the .eh_frame_hdr table, except those pointers are tagged as data-relative where "data" really means .eh_frame_hdr.

Long story short: I hate this and I'm really surprised that it works.

When quickly capturing a stack trace for a profiler, we generally only need a couple of the registers.

What exactly do you mean? Since the encoding allows arbitrary register/frame layout the only optimization I can think of is evaluating registers only lazily, but I kindof doubt that that would make a significant difference.

Also, I strongly typed different kinds of pointers that we encounter, before or after applying the loaders offset, etc

Stronger types are always good, the only reason unwind-rs is lacking in that respect right now is that it's still a prototype and not a polished solution.

I notice that there were some unwrap_or(0)s inside unwind-rs, and I think TaggedWord (or even just Option<usize> or something) is a slightly better approach.

We have the same idea here as that's exactly what I'm doing! I assume you're talking about this function which is the lander glue that jumps to landing pads. I'm just setting all registers because I think that's the cleanest solution, zeroing out the undefined ones.

why write assembly rather than use getcontext? Ultimately easier and more portable?

Maybe? I didn't know about this API, but I also didn't search for anything like that as doing that part in plain asm was just easier (I don't have to worry about what the library function does). One obstacle that comes to my mind right now is how to ensure that everything ... well ... works? What I mean is that any function which returns context data is very hard - if not impossible - to use correctly since that means I'll be walking my own stack frame that I'm changing right now. This is especially tricky when you think about compiler optimizations (most notably inlining).

I couldn't really wrap my head around this so I implemented the one reliable mechanism I came up with: Save the calling state and call the unwinder so the frame where it starts unwinding is frozen.


It seems like we have two puzzle pieces here: I have a working stacktracer and unwinder while you have some nice abstractions. We could merge if you want to. Obviously the final dream is for unwind-rs to become a fast and portable unwinder and rule the world 😆 . But jokes aside, problems like libbacktrace being slow are a reasonable argument against relying more on stack traces and debug info which I happen to be a big proponent of: rust-lang/rfcs#2154

So what I may one day end up doing (given enough free time of course) is implementing a complete unwinding+backtracing solution so Rust no longer has to rely on unmaintained upstream libraries for that. An added bonus could be the project I'm working on right now (why I actually created unwind-rs) where I print a stack trace along with all live variables and their values.

@fitzgen
Copy link
Member

fitzgen commented Oct 4, 2017

AFAIK .eh_frame_hdr is the only way to find .eh_frame since dl only gives you ELF segments, not sections.

Yes, that is what I was getting at :) I ultimately realized that the API I originally imagined was only possible to implement on OSX / Mach-O, and not on Linux / ELF.

Long story short: I hate this and I'm really surprised that it works.

Yes. And it is all pretty terribly documented in many far out, disparate locations :(

I wonder if inverting how we handle relative pointers makes sense... Return a symbolic form of the pointer, eg pc + 0x1234, rather than pass base addresses in.

Not sure that would actually help at all.

What exactly do you mean? Since the encoding allows arbitrary register/frame layout the only optimization I can think of is evaluating registers only lazily, but I kindof doubt that that would make a significant difference.

In practice,

  • almost all register rules are the simple ones like CFA + offset, and don't need to evaluate DWARF expressions,

  • finding the CFA is almost always register + offset, and

  • the registers referenced in the above rules are one of rbp, rip, and rsp.

A fast path that only handles the above case is a boon for a stack sampling profiler. libunwind has such a path. Firefox's builtin profiler pretty much only supports this path, and gets away with it pretty well.

Implementation wise, I think it is just a different register set struct, that only has the common subset of registers and a cut down version of the unwind routine.

I couldn't really wrap my head around this so I implemented the one reliable mechanism I came up with: Save the calling state and call the unwinder so the frame where it starts unwinding is frozen.

Yeah, I have a similar-ish approach that involves calling a closure + some lifetimes to ensure that you can't return and stomp over everything. https://github.com/fitzgen/pancakes/blob/master/src/x86_64/registers.rs#L108

I have a working stacktracer and unwinder while you have some nice abstractions. We could merge if you want to.

Sounds good to me :)

Obviously the final dream is for unwind-rs to become a fast and portable unwinder and rule the world 😆

Indeed :)


So I guess we are left with two immediate questions:

  1. Which name do you like better, pancakes or unwind-rs? :-P

  2. Where should the repo live? Move it into gimli-rs? New organization? I think it would fit with the gimli-rs org, if @philipc @jonhoo @tromey don't object to the idea.

@fitzgen
Copy link
Member

fitzgen commented Oct 10, 2017

Some discussion of other people potentially interested in helping out: https://internals.rust-lang.org/t/is-there-any-pure-rust-solution-for-libgcc-s-so-in-libunwind/5998/10

@main--
Copy link
Contributor Author

main-- commented Oct 10, 2017

Moving it to the gimli-rs org is fine with me. I'm not set on any name but I do like extern crate unwind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants