Prevent aborting guard from aborting the process in a forced unwind #104070

nbdd0121 · 2022-11-06T21:37:29Z

rustbot · 2022-11-06T21:37:36Z

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

bors · 2022-11-15T12:40:17Z

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

bjorn3 · 2023-04-07T14:43:19Z

compiler/rustc_codegen_ssa/src/traits/builder.rs

@@ -272,6 +272,7 @@ pub trait BuilderMethods<'a, 'tcx>:

    // These are used by everyone except msvc
    fn cleanup_landing_pad(&mut self, pers_fn: Self::Value) -> (Self::Value, Self::Value);
+    fn filter_landing_pad(&mut self, pers_fn: Self::Value) -> (Self::Value, Self::Value);


What is the difference between cleanup and filter? How should a backend implement it?

It's purely metadata in LSDA. https://itanium-cxx-abi.github.io/cxx-abi/exceptions.pdf

A negative value in action record means filter, a positive value means catch, and zero (or absence of action record) means cleanup.

This corresponds to section 7.5 "Exception Specification" of that document?

Yeah. I call it filter because that's how LLVM calls it.

The code here doesn't actually specify any exceptions (corresponds to C++ throw()), so it's essentially catch-all. But it provides a way for the personality function to distinguish terminate vs cleanup.

rustbot · 2023-04-07T15:31:42Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

nbdd0121 · 2023-04-07T15:32:15Z

r? @Amanieu

Amanieu · 2023-04-07T16:26:22Z

compiler/rustc_codegen_gcc/src/builder.rs

@@ -1226,6 +1226,10 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
        (value1, value2)
    }

+    fn filter_landing_pad(&mut self, pers_fn: RValue<'gcc>) -> (RValue<'gcc>, RValue<'gcc>) {
+        self.cleanup_landing_pad(pers_fn)


This seems incorrect?

The proper way to encode this in GCC would be <<<eh_filter (NULL)>>> (or even better, <<<eh_must_not_throw (terminate)>>>, which is something we hoped for for LLVM!). But I don't think there is support in libgccjit yet, @antoyo to confirm?

Yeah, I'll take care of this. I'm not done with supporting unwinding yet.

But, please add a // TODO(antoyo): generate the correct landing pad.

For my personal information, what are <<<eh_filter (NULL)>>> and <<<eh_must_not_throw (terminate)>>>? I've never seen this syntax. Are they C attributes?

I don't remember exactly what filter is doing, but the proper implementation of this method might be very similar as cleanup_landing_pad, but without adding to self.cleanup_blocks.

These are part of generic/gimple: https://godbolt.org/z/5adjbahnd

compiler/rustc_codegen_llvm/src/builder.rs

Amanieu · 2023-04-07T19:30:46Z

r=me after double-checking the GCC changes with @antoyo.

Amanieu · 2023-04-08T17:02:39Z

@bors r+

bors · 2023-04-08T17:02:41Z

📌 Commit 368cf88fbedf18ef2789acabbe80747c9529a555 has been approved by Amanieu

It is now in the queue for this repository.

bors · 2023-04-08T19:27:47Z

⌛ Testing commit 368cf88fbedf18ef2789acabbe80747c9529a555 with merge 5a90fe78cc076aca81d61b2c0f06c7e666aef04c...

bors · 2023-04-08T19:47:16Z

💔 Test failed - checks-actions

Amanieu · 2023-05-07T11:42:09Z

tests/run-make/forced-unwind-terminate-pof/foo.rs

@@ -0,0 +1,17 @@
+// Tests that forced unwind through POF Rust frames wouldn't trigger our terminating guards.


It would be nice to have a Windows version of this test that uses longjmp. On Windows longjmp is implemented using SEH exceptions and effectively acts like a forced unwind.

We already have a test longjmp-across-rust added by #48572 to fix #48251

Amanieu · 2023-05-07T11:46:43Z

@bors r+

bors · 2023-05-07T11:46:44Z

📌 Commit 5516791017a61acc9e5b1964accc8775bc340abe has been approved by Amanieu

It is now in the queue for this repository.

bors · 2023-05-08T04:25:52Z

⌛ Testing commit 5516791017a61acc9e5b1964accc8775bc340abe with merge 08c382f09932d3d0b04871bb834f68d59ffc8bb3...

bors · 2023-05-08T04:50:21Z

💔 Test failed - checks-actions

nbdd0121 · 2023-05-08T10:27:54Z

Somehow a commit from my other (already merged) PR was mixed in. Fixed

Amanieu · 2023-05-08T12:46:22Z

@bors r+

bors · 2023-05-08T12:46:24Z

📌 Commit 16abe6c has been approved by Amanieu

It is now in the queue for this repository.

…mpiler-errors Rollup of 6 pull requests Successful merges: - rust-lang#104070 (Prevent aborting guard from aborting the process in a forced unwind) - rust-lang#109410 (Introduce `AliasKind::Inherent` for inherent associated types) - rust-lang#111004 (Migrate `mir_transform` to translatable diagnostics) - rust-lang#111118 (Suggest struct when we get colon in fileds in enum) - rust-lang#111170 (Diagnostic args are still args if they're documented) - rust-lang#111354 (Fix miscompilation when calling default methods on `Future`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned wesleywiser Nov 6, 2022

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 Nov 6, 2022

jackh726 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 10, 2023

nbdd0121 mentioned this pull request Apr 4, 2023

LLVM 17 regression: tests/ui/backtrace.rs can't double-panic anymore #109672

Closed

nbdd0121 force-pushed the unwind branch from 4fa30c6 to 1451487 Compare April 7, 2023 14:18

bjorn3 reviewed Apr 7, 2023

View reviewed changes

nbdd0121 marked this pull request as ready for review April 7, 2023 15:31

rustbot assigned Amanieu and unassigned wesleywiser Apr 7, 2023

nbdd0121 force-pushed the unwind branch 2 times, most recently from 994b4a4 to 8e349db Compare April 7, 2023 16:16

Amanieu reviewed Apr 7, 2023

View reviewed changes

antoyo mentioned this pull request Apr 8, 2023

Support filter for exception handling rust-lang/rustc_codegen_gcc#264

Open

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-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Apr 8, 2023

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

nbdd0121 added 4 commits May 7, 2023 12:38

Add test for unwinding past terminating POF

ecd04fd

Fix num reserved clauses for landing pad

9792636

Add todo for filter landing pad

91afde5

Fix codegen test

16abe6c

nbdd0121 force-pushed the unwind branch from db80c08 to 5516791 Compare May 7, 2023 11:39

Amanieu reviewed May 7, 2023

View reviewed changes

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 May 7, 2023

This comment has been minimized.

Sign in to view

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

nbdd0121 force-pushed the unwind branch from 5516791 to 16abe6c Compare May 8, 2023 10:26

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 May 8, 2023

compiler-errors mentioned this pull request May 8, 2023

Rollup of 6 pull requests #111358

Merged

bors merged commit fcb275f into rust-lang:master May 8, 2023

rustbot added this to the 1.71.0 milestone May 8, 2023

nbdd0121 mentioned this pull request May 9, 2023

vec-shrink-panik: update expectations to work on LLVM 17 #111385

Merged

nbdd0121 mentioned this pull request Apr 4, 2024

panic in a no-unwind function leads to not dropping local variables #123231

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent aborting guard from aborting the process in a forced unwind #104070

Prevent aborting guard from aborting the process in a forced unwind #104070

nbdd0121 commented Nov 6, 2022 •

edited

Loading

rustbot commented Nov 6, 2022

bors commented Nov 15, 2022

bjorn3 Apr 7, 2023

nbdd0121 Apr 7, 2023

bjorn3 Apr 7, 2023

nbdd0121 Apr 7, 2023

rustbot commented Apr 7, 2023

nbdd0121 commented Apr 7, 2023

Amanieu Apr 7, 2023

nbdd0121 Apr 7, 2023

antoyo Apr 8, 2023

nbdd0121 Apr 8, 2023

Amanieu commented Apr 7, 2023

Amanieu commented Apr 8, 2023

bors commented Apr 8, 2023

bors commented Apr 8, 2023

bors commented Apr 8, 2023

Amanieu May 7, 2023

nbdd0121 May 7, 2023

Amanieu commented May 7, 2023

bors commented May 7, 2023

bors commented May 8, 2023

This comment has been minimized.

bors commented May 8, 2023

nbdd0121 commented May 8, 2023

Amanieu commented May 8, 2023

bors commented May 8, 2023

		@@ -0,0 +1,17 @@
		// Tests that forced unwind through POF Rust frames wouldn't trigger our terminating guards.

Prevent aborting guard from aborting the process in a forced unwind #104070

Prevent aborting guard from aborting the process in a forced unwind #104070

Conversation

nbdd0121 commented Nov 6, 2022 • edited Loading

rustbot commented Nov 6, 2022

bors commented Nov 15, 2022

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

rustbot commented Apr 7, 2023

nbdd0121 commented Apr 7, 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

Amanieu commented Apr 7, 2023

Amanieu commented Apr 8, 2023

bors commented Apr 8, 2023

bors commented Apr 8, 2023

bors commented Apr 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amanieu commented May 7, 2023

bors commented May 7, 2023

bors commented May 8, 2023

This comment has been minimized.

bors commented May 8, 2023

nbdd0121 commented May 8, 2023

Amanieu commented May 8, 2023

bors commented May 8, 2023

nbdd0121 commented Nov 6, 2022 •

edited

Loading