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

Support generating backtraces in emulated code #1033

Closed
Aaron1011 opened this issue Nov 4, 2019 · 13 comments
Closed

Support generating backtraces in emulated code #1033

Aaron1011 opened this issue Nov 4, 2019 · 13 comments
Labels
A-shims Area: This affects the external function shims C-project Category: a larger project is being tracked here, usually with checkmarks for individual steps

Comments

@Aaron1011
Copy link
Member

Once rust-lang/rust#60026 is merged, Miri will have support for panicking via unwinding.

However, we will still lack support for printing backtraces from the panic hook (currently, Miri builds libstd without the backtrace feature, so nothing is printed).

Unlike the implementation of panicking, getting this to work will require modifying a third party-crate. Specifically, we will need to modify backtrace-rs, which is used by libstd to generate backtraces.

This will require Miri to expose some sort of API to the outside world for the first time. At a minimum, backtrace-rs needs a way to get a list of Frames from Miri (the definition of Frame is up to us).

This bring up an interesting question: do we ever want to stabilize this API? For printing panic backtraces, it's sufficient for this API to be unstable - Miri builds libstd using nightly Rust, so we can just enable some internal feature. However, it seems reasonable for third-party crates using backtrace-rs to 'just work' under Miri. This means that whatever mechanism backtrace-rs uses to communicate with Miri will (eventually) need to be stable as well.

One way to go about this would be to have Miri define some special foreign function (e.g. __rust_miri_magic_get_backtrace), rather than an intrinsic. Conceptually, this would make Miri just another target platform, which happens to guarantee the existence of some foreign function.

@Aaron1011 Aaron1011 changed the title Support generating backtracs in emulated code Support generating backtraces in emulated code Nov 4, 2019
@RalfJung RalfJung added A-shims Area: This affects the external function shims C-project Category: a larger project is being tracked here, usually with checkmarks for individual steps labels Nov 4, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Nov 5, 2019

So far our strategy has been to implement the platform specific logic, but just looking at https://github.com/rust-lang/backtrace-rs/blob/master/src/windows.rs makes me shiver.

For things like this where there's no use in having miri run the platform specific logic to find UB, I think it would be fine to add cfg(miri) logic to backtrace-rs

@Aaron1011
Copy link
Member Author

Supporting the full backtrace-rs api is going to trickier than I had imagined.

The main problem is symbol_address - it returns a *mut c_void which can later be passed to backtrace::resolve.

Of course, Miri doesn't have a flat address space. I think the best approach would be to cast Span.lo to a raw pointer (for the span of the calling frame). Span.lo is a BytePos, which is always 32 bits - so we can do this on both 32-bit and 64-bit platforms. We can throw away span.hi, since all we need is the filename, line number, and function name - all of which we can obtain with just the lower half of the span.

This approach avoids allocating any additional memory, so repeatedly calling Backtrace::new() won't cause memory usage to grow.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 8, 2019

Neat trick, but how do we do the inverse? Creating a BytePos from user-given numbers seems dangerous.

@bjorn3
Copy link
Member

bjorn3 commented Nov 8, 2019

The main problem is symbol_address - it returns a *mut c_void which can later be passed to backtrace::resolve.

Maybe return fn_ptr as *mut c_void? Function allocations are only created once per Instance, right? If so, that means that extra memory usage is bounded with this approach, just like your BytePos proposal.

@Aaron1011
Copy link
Member Author

Neat trick, but how do we do the inverse? Creating a BytePos from user-given numbers seems dangerous.

How so? We can check if it's in the valid range for a Span - it may be a nonsensical span, but we can easily handle that case.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2019

I assumed that rustc would simply panic on out of bounds spans

I like the function pointer idea

@Aaron1011
Copy link
Member Author

Aaron1011 commented Nov 9, 2019

Using function pointers would make us lose precise line number information, which seems like a really unfortunate limitation.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2019

You can use the offset part of the pointer to encode an additional 32bit

@Aaron1011
Copy link
Member Author

@oli-obk: I'm not sure what you mean, or how that could work on 32-bit platforms.

Using the lower BytePos of the Span gives us exactly the information we need, doesn't require any additional allocations at all, and works on both 32-bit and 64-bit platforms.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2019

I guess. It just feels a bit hacky that we'd expose a compiler internal integer. Especially since they can change arbitrarily between runs

@Aaron1011
Copy link
Member Author

It just feels a bit hacky that we'd expose a compiler internal integer. Especially since they can change arbitrarily between runs

Doesn't that apply to Miri function pointers as well?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 10, 2019

You can't get at that information. It's not exposed to the program.

in https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=96c11f4863d9816868d9a09b0198a26b you will not see a rustc specific integer, but just the integer that you get from the ptr2int conversion which is a program-specific counter

@Aaron1011
Copy link
Member Author

This was implemented in several PRs (the last one was #1589)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-project Category: a larger project is being tracked here, usually with checkmarks for individual steps
Projects
None yet
Development

No branches or pull requests

4 participants