Use `unwinding` crate for unwinding on Xous platform #117072

xobs · 2023-10-23T08:04:00Z

This patch adds support for using unwinding on platforms where libunwinding isn't viable. An example of such a platform is riscv32imac-unknown-xous-elf.

Background

The Rust project maintains a fork of llvm at llvm-project where it applies patches on top of the llvm project. This mostly seems to be to get unwinding support for the SGX project, and there may be other patches that I'm unaware of.

There is a lot of machinery in the build system to support compiling libunwind on other platforms, and I needed to add additional patches to llvm in order to add support for Xous.

Rather than continuing down this path, it seemed much easier to use a Rust-based library. The unwinding crate by @nbdd0121 fits this description perfectly.

Future work

This could potentially replace the custom patches for libunwind on other platforms such as SGX, and could enable unwinding support on many more exotic platforms.

Anti-goals

This is not designed to replace libunwind on tier-one platforms or those where unwinding support already exists. There is already a well-established approach for unwinding there. Instead, this aims to enable unwinding on new platforms where C++ code may be difficult to compile.

rustbot · 2023-10-23T08:04:09Z

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-10-23T08:04:12Z

These commits modify compiler targets.
(See the Target Tier Policy.)

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustbot · 2023-10-23T08:36:31Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

These commits modify compiler targets.
(See the Target Tier Policy.)

Third-party dependency whitelist may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

cuviper · 2023-10-27T20:21:16Z

I'm nominating this for libs team discussion, both because std dependencies are not taken lightly, and because this may also set precedent for other targets.

bors · 2023-11-05T12:33:16Z

☔ The latest upstream changes (presumably #117504) made this pull request unmergeable. Please resolve the merge conflicts.

xobs · 2023-11-05T14:57:55Z

rebased to follow the removal of cc as a dependency within unwinding.

bors · 2023-11-08T21:34:30Z

☔ The latest upstream changes (presumably #117716) made this pull request unmergeable. Please resolve the merge conflicts.

Cargo.lock

xobs · 2023-11-09T01:06:48Z

Forced push changes that:

follow the renaming of compiler/rustc_target/src/spec -> compiler/rustc_target/src/spec/targets
bump unwinding to 0.2.1
remove the second copy of gimli from Cargo.lock

Amanieu · 2023-11-15T16:13:59Z

library/unwind/src/unwinding.rs

@@ -0,0 +1,102 @@
+#![allow(nonstandard_style)]


Most of these are available from unwinding::abi, can this be used instead?

The problem is that the callee uses raw pointers, so most of the functions here are casting pointers to references. The calling code could be modified to use references rather than pointers, but I didn't want to touch such fundamental structures.

That's fine then. But at least change the pub fn to pub unsafe fn since they accept raw pointers.

Alright, I've changed the signatures and verified that it still works.

The `unwinding` crate supports processing unwinding data, and is written entirely in Rust. This allows it to be ported to new platforms more easily than using the llvm-based `libunwind`. While `libunwind` is very well supported on major targets, it is difficult to use on other targets. SGX is an example of this where Rust carries custom patches in order to enable backtrace support. This adds an alternative for supported architectures. Rather than providing a custom target, `unwinding` allows for a solution that is completely written in Rust. This adds `xous` as the first consumer, and forthcoming patches will modify libstd to take advantage of this. Signed-off-by: Sean Cross <sean@xobs.io>

Now that `unwind` supports Xous, enable unwinding panics on Xous. Signed-off-by: Sean Cross <sean@xobs.io>

Xous as an operating system is compiled with gcc-type personalities when it comes to unwinding. This enables unwinding inside panics on Xous, which enables Rust tests. Signed-off-by: Sean Cross <sean@xobs.io>

Now that everything is in place to support unwinding on Xous, enable this for that target. Signed-off-by: Sean Cross <sean@xobs.io>

Add `unwinding` as a dependency. This is required on platforms where unwinding isn't provided by llvm. Signed-off-by: Sean Cross <sean@xobs.io>

library/std/src/sys/xous/os.rs

The main() function takes an argument that contains the eh_frame address. Implement `unwinding` support by looking for unwinding data at this address. Signed-off-by: Sean Cross <sean@xobs.io>

Add `unwinding` as a permitted dependency of rustc, as it is now used as part of panic unwinding within platforms such as Xous. Signed-off-by: Sean Cross <sean@xobs.io>

cuviper · 2023-12-06T01:48:43Z

@bors r+

bors · 2023-12-06T01:48:46Z

📌 Commit 0773afc has been approved by cuviper

It is now in the queue for this repository.

bors · 2023-12-06T02:23:05Z

⌛ Testing commit 0773afc with merge 84a554c...

bors · 2023-12-06T04:19:45Z

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing 84a554c to master...

rust-timer · 2023-12-06T05:40:33Z

Finished benchmarking commit (84a554c): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	3
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	1.2%	[1.2%, 1.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	[-2.4%, -2.1%]	4
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.395s -> 673.722s (-0.25%)
Artifact size: 314.15 MiB -> 314.15 MiB (0.00%)

rustbot assigned cuviper Oct 23, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 23, 2023

This comment has been minimized.

Sign in to view

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 23, 2023

This comment has been minimized.

Sign in to view

xobs force-pushed the unwinding-crate-support branch from 87f857a to f2d3fed Compare October 23, 2023 08:46

cuviper added the I-libs-nominated Nominated for discussion during a libs team meeting. label Oct 27, 2023

xobs force-pushed the unwinding-crate-support branch from f2d3fed to d91d1dc Compare November 5, 2023 14:08

nbdd0121 reviewed Nov 8, 2023

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

xobs force-pushed the unwinding-crate-support branch from d91d1dc to ca43176 Compare November 9, 2023 00:58

rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Nov 9, 2023

xobs force-pushed the unwinding-crate-support branch from ca43176 to c419805 Compare November 9, 2023 01:06

Amanieu reviewed Nov 15, 2023

View reviewed changes

Amanieu removed the I-libs-nominated Nominated for discussion during a libs team meeting. label Nov 15, 2023

xobs force-pushed the unwinding-crate-support branch from c419805 to 40bbd13 Compare November 16, 2023 07:16

This comment has been minimized.

Sign in to view

xobs added 3 commits November 16, 2023 15:23

panic_unwind: support unwinding on xous

2820317

Now that `unwind` supports Xous, enable unwinding panics on Xous. Signed-off-by: Sean Cross <sean@xobs.io>

std: personality: support gcc personality on Xous

1828cf8

Xous as an operating system is compiled with gcc-type personalities when it comes to unwinding. This enables unwinding inside panics on Xous, which enables Rust tests. Signed-off-by: Sean Cross <sean@xobs.io>

xobs added 2 commits November 16, 2023 15:23

compiler: enable unwinding on riscv32imac_unknown_xous_elf

bf0e0af

Now that everything is in place to support unwinding on Xous, enable this for that target. Signed-off-by: Sean Cross <sean@xobs.io>

Cargo.lock: add unwinding to lock file

2a533df

Add `unwinding` as a dependency. This is required on platforms where unwinding isn't provided by llvm. Signed-off-by: Sean Cross <sean@xobs.io>

xobs force-pushed the unwinding-crate-support branch from 40bbd13 to 7c7bc5f Compare November 16, 2023 07:23

cuviper reviewed Dec 5, 2023

View reviewed changes

library/std/src/sys/xous/os.rs Outdated Show resolved Hide resolved

xobs added 2 commits December 6, 2023 09:07

std: xous: take eh_frame address from main args

a6b8de6

The main() function takes an argument that contains the eh_frame address. Implement `unwinding` support by looking for unwinding data at this address. Signed-off-by: Sean Cross <sean@xobs.io>

tidy: add unwinding as an allowed dependency

0773afc

Add `unwinding` as a permitted dependency of rustc, as it is now used as part of panic unwinding within platforms such as Xous. Signed-off-by: Sean Cross <sean@xobs.io>

xobs force-pushed the unwinding-crate-support branch from 7c7bc5f to 0773afc Compare December 6, 2023 01:07

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 6, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 6, 2023

bors merged commit 84a554c into rust-lang:master Dec 6, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Dec 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `unwinding` crate for unwinding on Xous platform #117072

Use `unwinding` crate for unwinding on Xous platform #117072

xobs commented Oct 23, 2023 •

edited

Loading

rustbot commented Oct 23, 2023

rustbot commented Oct 23, 2023

This comment has been minimized.

rustbot commented Oct 23, 2023

This comment has been minimized.

cuviper commented Oct 27, 2023

bors commented Nov 5, 2023

xobs commented Nov 5, 2023

bors commented Nov 8, 2023

xobs commented Nov 9, 2023

Amanieu Nov 15, 2023

xobs Nov 15, 2023

Amanieu Nov 15, 2023

xobs Nov 16, 2023

This comment has been minimized.

cuviper commented Dec 6, 2023

bors commented Dec 6, 2023

bors commented Dec 6, 2023

bors commented Dec 6, 2023

rust-timer commented Dec 6, 2023

Use unwinding crate for unwinding on Xous platform #117072

Use unwinding crate for unwinding on Xous platform #117072

Conversation

xobs commented Oct 23, 2023 • edited Loading

Background

Future work

Anti-goals

rustbot commented Oct 23, 2023

rustbot commented Oct 23, 2023

This comment has been minimized.

rustbot commented Oct 23, 2023

This comment has been minimized.

cuviper commented Oct 27, 2023

bors commented Nov 5, 2023

xobs commented Nov 5, 2023

bors commented Nov 8, 2023

xobs commented Nov 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

cuviper commented Dec 6, 2023

bors commented Dec 6, 2023

bors commented Dec 6, 2023

bors commented Dec 6, 2023

rust-timer commented Dec 6, 2023

Overall result: ✅ improvements - no action needed

Use `unwinding` crate for unwinding on Xous platform #117072

Use `unwinding` crate for unwinding on Xous platform #117072

xobs commented Oct 23, 2023 •

edited

Loading