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

Use the unwind crate to implement libunwind interfaces. #37

Closed
wants to merge 22 commits into from

Conversation

sunfishcode
Copy link
Owner

The unwind crate appears to work well for this purpose, even on panics with
RUST_BACKTRACE=full.

This uses the fde-phdr-dl backend, which uses dl_iterate_phdr, so implement
dl_iterate_phdr.

Fixes #27.

Factoring these out into separate files appears to be insufficient to
force them to all be linked in, replacing libc, so pull them all into
a single module.
The `unwind` crate appears to work well for this purpose, even on panics with
RUST_BACKTRACE=full.

This uses the `fde-phdr-dl` backend, which uses `dl_iterate_phdr`, so implement
`dl_iterate_phdr`.

Fixes #27.
Unwind doesn't support 32-bit x86 yet, so drop it for now.
It's used by `init_have_lse_atomics` in libgcc/compiler-rt.
pub(crate) const AT_HWCAP2: c_ulong = 26;
pub(crate) const AT_EXECFN: c_ulong = 31;
pub(crate) const AT_SYSINFO: c_ulong = 32;
pub(crate) const AT_SYSINFO_EHDR: c_ulong = 33;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be in linux-raw-sys?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, no, because c-scape is implementing the userspace libc ABI, and glibc and musl don't guarantee that their ABIs match Linux's ABI. They both have their own copies of these constants rather than just re-exporting the definitions from Linux's headers.

@nbdd0121
Copy link

nbdd0121 commented Oct 5, 2021

Would fde-phdr-aux work?

@sunfishcode
Copy link
Owner Author

Yeah, mustang could use either fde-phdr-aux or fde-phdr-dl. Do you prefer we use fde-phdr-aux?

@nbdd0121
Copy link

nbdd0121 commented Oct 5, 2021

I don't have a preference. I added fde-phdr-aux thinking it might be useful for you, but implementing dl_iterate_phdr would work too.

@nbdd0121
Copy link

nbdd0121 commented Oct 5, 2021

BTW I have renamed the repo unwinding and published to crates.io under that name. I haven't implement unwinding for x86 because I didn't really need it, but can do so if it'll be useful for you.

@sunfishcode
Copy link
Owner Author

Awesome, thanks! And thanks so much for developing unwinding, it works great in everything I've tried with it, on both x86-64 and aarch64, and fills a major gap for mustang overall!

32-bit x86 support would be convenient, as it would let us remove the stub unwind API implementation, but it's not a blocker for anything.

@nbdd0121
Copy link

nbdd0121 commented Oct 5, 2021

x86 support is completed. You may still want to keep your stubs though since there are no AArch32/PowerPC/MIPS support in unwinding.

@sunfishcode
Copy link
Owner Author

I've now updated to the version with x86 support, and enabled a bunch more tests, including several which include panics and catch_unwind, and everything passes, on x86, x86-64, and aarch64.

This PR is now just waiting on a gimli release, to support aarch64 without using [patch.crates-io].

@sunfishcode
Copy link
Owner Author

These patches are pulled in by #7. I've also temporarily disabled unwinding on aarch64 and riscv64 to avoid needing to patch in a git dependency on gimli for now.

@sunfishcode sunfishcode closed this Oct 7, 2021
@sunfishcode sunfishcode deleted the sunfishcode/extern-c-definitions branch October 7, 2021 05:36
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

Successfully merging this pull request may close these issues.

Investigate the unwind crate
3 participants