Win: Use exceptions with empty data for SEH panic exception copies instead of a new panic

[Fulgen301]

For unwinding with SEH, we currently construct a C++ exception with the panic data. Being a regular C++ exception, it interacts with the C++ exception handling machinery and can be retrieved via std::current_exception, which needs to copy the exception. We can't support that, so we panic, which throws another exception, which the C++ runtime tries to copy and store into the exception_ptr, which panics again, which causes the C++ runtime to store a bad_exception instance.

However, this doesn't work because the panics thrown by the copy function will be dropped without being rethrown, and causes unnecessary log spam in stderr. Fix this by directly throwing an exception without data, which doesn't cause log spam and can be dropped without being rethrown.

Fixes #143623.

This also happens to be the solution @dpaoliello suggested, though I'm not sure how to handle the commit credit attribution.

[rustbot]

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[dpaoliello]

Minor typo, but LGTM

[ChrisDenton]

Could you squish your commits?

[ChrisDenton]

Thanks!

[ChrisDenton]

Let's try that again...

@bors r+ rollup

[bors]

📌 Commit 96cdbb9 has been approved by ChrisDenton

It is now in the queue for this repository.

[purplesyringa]

This also happens to be the solution @dpaoliello suggested

For context, that's not quite the case... Unless I'm missing something, he recommended exception_cleanup to disarm the discard check on the original panic. Your solution doesn't, and I think that's more correct.

[dpaoliello]

For context, that's not quite the case... Unless I'm missing something, he recommended exception_cleanup to disarm the discard check on the original panic. Your solution doesn't, and I think that's more correct.

Actually, that's a good point: are we still going to see a panic because the original panic wasn't rethrown (i.e., in exception_cleanup it will see that the original panic's data is not None)?

[Fulgen301]

Correct. The panic exceptions with data == None only serve as sentinel exceptions so that the exception_ptrmachinery stores a bad_exception instance instead. The original exception still has to be rethrown. We can't do a fake copy and silence the original drop check, because Rust panics must not be handled by C++.

If we wanted to support rethrowing via the exception_ptr, we'd have to internally refcount the data, but still keep a drop check so that either the original exception on the stack or a copy in an exception_ptr was rethrown. This isn't a good fix for three reasons:

• It adds the cost of an Arc or equivalent for to every panic.
• It wouldn't be as clear cut of a bugfix as this is; C++ code has to deal with the possiblity of std::current_exception() returning a std::bad_exception ptr because the standard explicitely declared that to be the fallback for exception copy failures, whereas the bug here was a misguided assumption in code that already existed (threw an exception in the copy function that had to be rethrown because it was a panic).
• It'd mean the C++ side could store a copy of the panic exception and rethrow it later. We can detect that as std::rethrow_exception would copy the exception again, but we can't reuse the exception or throw a panic exception in that case, because unwinding might return to Rust code which would get a panic out of nowhere, so we'd have to add another throw info for a primitive or a different exception type. And it's a lot of code to support C++ storing a, from Rust's point of view, implementation detail with no stability guarantees, all to avoid a scenario that C++ code already has to deal with.