Avoid __cxa_thread_atexit_impl on Emscripten #106779

RReverser · 2023-01-12T18:47:43Z

Fixes Compiling to WASM with emscripten, parse exception: attempted pop from empty stack / beyond block start boundary #91628.
Fixes When compiling rust with threads, parse exception: attempted pop from empty stack / beyond block start boundary emscripten-core/emscripten#15722.

See discussion in both issues.

The TL;DR is that weak linkage causes LLVM to produce broken Wasm, presumably due to pointer mismatch. The code is casting a void pointer to a function pointer with specific signature, but Wasm is very strict about function pointer compatibility, so the resulting code is invalid.

Ideally LLVM should catch this earlier in the process rather than emit invalid Wasm, but it currently doesn't and this is an easy and valid fix, given that Emcripten doesn't have __cxa_thread_atexit_impl these days anyway.

Unfortunately, I can't add a regression test as even after looking into this issue for a long time, I couldn't reproduce it with any minimal Rust example, only with extracted LLVM IR or on a large project involving Rust + C++.

@alexcrichton

- Fixes rust-lang#91628. - Fixes emscripten-core/emscripten#15722. See discussion in both issues. The TL;DR is that weak linkage causes LLVM to produce broken Wasm, presumably due to pointer mismatch. The code is casting a void pointer to a function pointer with specific signature, but Wasm is very strict about function pointer compatibility, so the resulting code is invalid. Ideally LLVM should catch this earlier in the process rather than emit invalid Wasm, but it currently doesn't and this is an easy and valid fix, given that Emcripten doesn't have `__cxa_thread_atexit_impl` these days anyway. Unfortunately, I can't add a regression test as even after looking into this issue for a long time, I couldn't reproduce it with any minimal Rust example, only with extracted LLVM IR or on a large project involving Rust + C++. r? @alexcrichton

rustbot · 2023-01-12T18:47:50Z

Failed to set assignee to alexcrichton: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

rustbot · 2023-01-12T18:47:53Z

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

Noratrieb · 2023-01-13T13:42:48Z

r? libs

RReverser · 2023-01-13T13:51:55Z

For those curious, also raised an LLVM issue for the underlying problem llvm/llvm-project#60003, but this Rust-specific fix should be valid regardless.

Mark-Simulacrum · 2023-01-14T02:58:09Z

@bors r+

Seems okay.

bors · 2023-01-14T02:58:11Z

📌 Commit 6155b9a has been approved by Mark-Simulacrum

It is now in the queue for this repository.

Avoid __cxa_thread_atexit_impl on Emscripten - Fixes rust-lang#91628. - Fixes emscripten-core/emscripten#15722. See discussion in both issues. The TL;DR is that weak linkage causes LLVM to produce broken Wasm, presumably due to pointer mismatch. The code is casting a void pointer to a function pointer with specific signature, but Wasm is very strict about function pointer compatibility, so the resulting code is invalid. Ideally LLVM should catch this earlier in the process rather than emit invalid Wasm, but it currently doesn't and this is an easy and valid fix, given that Emcripten doesn't have `__cxa_thread_atexit_impl` these days anyway. Unfortunately, I can't add a regression test as even after looking into this issue for a long time, I couldn't reproduce it with any minimal Rust example, only with extracted LLVM IR or on a large project involving Rust + C++.

matthiaskrgr · 2023-01-14T12:03:34Z

@bors r- failed in a rollup
https://github.com/rust-lang-ci/rust/actions/runs/3918264123/jobs/6698547238#step:26:2171

RReverser · 2023-01-14T12:30:30Z

A little odd that I didn't see it unused in my testing, but okay, re-added the #[cfg_attr(target_family = "wasm", allow(unused))] attribute in the new place.

RReverser · 2023-01-23T11:48:11Z

r? @Mark-Simulacrum @matthiaskrgr

rustbot · 2023-01-23T11:48:12Z

Could not assign reviewer from: Mark-Simulacrum.
User(s) Mark-Simulacrum are either the PR author or are already assigned, and there are no other candidates.
Use r? to specify someone else to assign.

RReverser · 2023-01-23T18:04:19Z

@Mark-Simulacrum Sorry, not sure whether to trigger re-review somehow or just ask again.

Mark-Simulacrum · 2023-01-24T13:52:24Z

@bors r+

bors · 2023-01-24T13:52:26Z

📌 Commit a41c5f9 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors · 2023-01-24T13:52:27Z

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

Mark-Simulacrum · 2023-01-24T13:52:43Z

@rustbot ready in the future will update labels to waiting-on-review.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#97373 (impl DispatchFromDyn for Cell and UnsafeCell) - rust-lang#106625 (Remove backwards compat for LLVM 12 coverage format) - rust-lang#106779 (Avoid __cxa_thread_atexit_impl on Emscripten) - rust-lang#106811 (Append .dwp to the binary filename instead of replacing the existing extension.) - rust-lang#106836 (Remove optimistic spinning from `mpsc::SyncSender`) - rust-lang#106946 (implement Hash for proc_macro::LineColumn) - rust-lang#107074 (remove unnecessary check for opaque types) - rust-lang#107287 (Improve fn pointer notes) - rust-lang#107304 (Use `can_eq` to compare types for default assoc type error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

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

RReverser mentioned this pull request Jan 12, 2023

When compiling rust with threads, parse exception: attempted pop from empty stack / beyond block start boundary emscripten-core/emscripten#15722

Closed

rustbot assigned Mark-Simulacrum Jan 13, 2023

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 Jan 14, 2023

matthiaskrgr mentioned this pull request Jan 14, 2023

Rollup of 8 pull requests #106839

Closed

matthiaskrgr mentioned this pull request Jan 14, 2023

Rollup of 7 pull requests #106842

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 14, 2023

Re-add #[allow(unused)] attr

a41c5f9

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 24, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 24, 2023

matthiaskrgr mentioned this pull request Jan 26, 2023

Rollup of 9 pull requests #107318

Merged

bors merged commit cc92bdb into rust-lang:master Jan 26, 2023

rustbot added this to the 1.69.0 milestone Jan 26, 2023

RReverser deleted the patch-2 branch February 4, 2023 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid __cxa_thread_atexit_impl on Emscripten #106779

Avoid __cxa_thread_atexit_impl on Emscripten #106779

RReverser commented Jan 12, 2023 •

edited

Loading

rustbot commented Jan 12, 2023

rustbot commented Jan 12, 2023

Noratrieb commented Jan 13, 2023

RReverser commented Jan 13, 2023

Mark-Simulacrum commented Jan 14, 2023

bors commented Jan 14, 2023

matthiaskrgr commented Jan 14, 2023

RReverser commented Jan 14, 2023

RReverser commented Jan 23, 2023

rustbot commented Jan 23, 2023

RReverser commented Jan 23, 2023

Mark-Simulacrum commented Jan 24, 2023

bors commented Jan 24, 2023

bors commented Jan 24, 2023

Mark-Simulacrum commented Jan 24, 2023

Avoid __cxa_thread_atexit_impl on Emscripten #106779

Avoid __cxa_thread_atexit_impl on Emscripten #106779

Conversation

RReverser commented Jan 12, 2023 • edited Loading

rustbot commented Jan 12, 2023

rustbot commented Jan 12, 2023

Noratrieb commented Jan 13, 2023

RReverser commented Jan 13, 2023

Mark-Simulacrum commented Jan 14, 2023

bors commented Jan 14, 2023

matthiaskrgr commented Jan 14, 2023

RReverser commented Jan 14, 2023

RReverser commented Jan 23, 2023

rustbot commented Jan 23, 2023

RReverser commented Jan 23, 2023

Mark-Simulacrum commented Jan 24, 2023

bors commented Jan 24, 2023

bors commented Jan 24, 2023

Mark-Simulacrum commented Jan 24, 2023

RReverser commented Jan 12, 2023 •

edited

Loading