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

Proc macro errors can lead to rustc panics at stage1 or on non-Linux #59998

Closed
RalfJung opened this issue Apr 15, 2019 · 5 comments · Fixed by #99944
Closed

Proc macro errors can lead to rustc panics at stage1 or on non-Linux #59998

RalfJung opened this issue Apr 15, 2019 · 5 comments · Fixed by #99944
Assignees
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ O-macos Operating system: macOS O-windows Operating system: Windows P-medium Medium priority T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Apr 15, 2019

Update (by @eddyb): this also occurs with x.py test --stage 1 src/test/ui on Linux (and is responsible for several // ignore-stage1 in src/test/ui), so I'm updating the title to reflect it.
See also #59998 (comment) for more details.


Discovered at #59769 (comment): running the ui\proc-macro\invalid-punct-ident-*.rs tests on some platforms (observed on Windows and macOS but not on Linux) leads to things like

thread 'rustc' panicked at 'unsupported character `'`'`', src\libsyntax_ext\proc_macro_server.rs:315:13
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

being printed on stderr.

Cc @eddyb

@estebank estebank added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ O-windows Operating system: Windows labels Apr 15, 2019
@Centril Centril added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 15, 2019
@RalfJung RalfJung changed the title Proc macro errors can lead to rustc panics on Windows Proc macro errors can lead to rustc panics on non-Linux Apr 16, 2019
@RalfJung RalfJung added the O-macos Operating system: macOS label Apr 16, 2019
@RalfJung
Copy link
Member Author

RalfJung commented Apr 16, 2019

This seems to also happen on macOS. Just not on Linux. @eddyb speculated that this is about whether libstd is shared between the proc macro client and server. I do not know what that means.

@eddyb
Copy link
Member

eddyb commented Apr 16, 2019

Statically linked Rust libraries (such as libcore or libstd) can be are duplicated between non-Rust "final" artifacts (e.g. binaries, or cdylibs, and the latter is what proc macros are).

That is, you can end up with several copies of libstd's functions and globals in one process, if you dynamically load a Rust cdylib / proc macro crate.
The functions aren't really an issue, but globals control panicking, and this means that a proc macro is not guaranteed to use the same panicking "global state machine" as the rustc that loaded it.

We account for this in terms of catching panics and transporting them over RPC, but we've forgotten to do the "stderr printing bypass" for panics happening on the server-side (i.e. caused by rustc handling a request from a proc macro client), we're only bypassing the printing for panics that happened client-side.

Also, to be clear, sharing libstd's is not the solution, but rather Linux is hiding an issue because it happens to do so (and this is not the case at e.g. stage1, because the server and client libstd's are incompatible).
#59769 (comment) has more info on what we might do to fix this.

@pnkfelix
Copy link
Member

triage: P-medium. Leaving nomination tag in hopes we can discuss at meeting.

@pnkfelix pnkfelix added the P-medium Medium priority label Apr 18, 2019
@pnkfelix
Copy link
Member

pnkfelix commented May 16, 2019

discussed at T-compiler meeting. self-assigning. unnominating.

@pnkfelix pnkfelix self-assigned this May 16, 2019
@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Aug 6, 2019
@eddyb eddyb changed the title Proc macro errors can lead to rustc panics on non-Linux Proc macro errors can lead to rustc panics at stage1 or on non-Linux Jun 29, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 29, 2022
Unbreak stage1 tests via ignore-stage1 in `proc-macro/invalid-punct-ident-1.rs`.

rust-lang#98188 broke `./x.py test --stage 1` (which I thought we ran in PR CI, cc `@rust-lang/infra)` i.e. the default `./x.py test` in dev checkouts, as the panic in `src/test/ui/proc-macro/invalid-punct-ident-1.rs` moved from the server (`rustc`) to the client (proc macro), and that means it's now affected by rust-lang#59998.

I made the test look like `src/test/ui-fulldeps/issue-76270-panic-in-libproc-macro.rs` tho I'm a bit confused why that one is in `src/test/ui-fulldeps`, it should still work in `src/test/ui`, no? (cc `@Aaron1011)`
@eddyb
Copy link
Member

eddyb commented Jul 29, 2022

Not sure yet if relevant, but: turns out my expectation of proc macros being "most cdylib" is wrong because (more details here: #99909 (comment)) of this bail-out:

// We manually create a list of exported symbols to ensure we don't expose any more.
// The object files have far more public symbols than we actually want to export,
// so we hide them all here.
if !self.sess.target.limit_rdylib_exports {
return;
}
if crate_type == CrateType::ProcMacro {
return;
}

AFAICT that's what's causing proc macro dylibs to have so many dynamic exports (instead of exactly one, the proc_macro_decls or w/e it's called) - and if all it takes for the panic runtimes to conflict is dynamic linking to unify one of those symbols, that might be what's been causing all of these issues for years and we just didn't look in the right place...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ O-macos Operating system: macOS O-windows Operating system: Windows P-medium Medium priority T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants