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

track_caller on panic_no_unwind leads to LLVM crash #94337

Closed
Mark-Simulacrum opened this issue Feb 24, 2022 · 5 comments
Closed

track_caller on panic_no_unwind leads to LLVM crash #94337

Mark-Simulacrum opened this issue Feb 24, 2022 · 5 comments
Labels
A-codegen Area: Code generation F-track_caller `#![feature(track_caller)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

#94290 is expecting to land having introduced #[cfg_attr(bootstrap, track_caller)] on panic_no_unwind in library/core/src/panicking.rs.

We saw these errors in stage 1 std compilation in CI (with track_caller and without it):

Incorrect number of arguments passed to called function!
  tail call void @_ZN4core9panicking15panic_no_unwind17heb5bd7bc5de639ecE(%"core::panic::location::Location"* bitcast (<{ i8*, [16 x i8] }>* @10 to %"core::panic::location::Location"*)) #15
in function _ZN12panic_unwind8real_imp5panic17exception_cleanup17h7b2c07bcac8db7c5E
LLVM ERROR: Broken function found, compilation aborted!
�[0m�[0m�[1m�[31merror�[0m�[1m:�[0m could not compile `panic_unwind`
�[0m�[0m�[1m�[33mwarning�[0m�[1m:�[0m build failed, waiting for other jobs to finish...
Incorrect number of arguments passed to called function!
  call void @_ZN4core9panicking15panic_no_unwind17heb5bd7bc5de639ecE(%"panic::location::Location"* bitcast (<{ i8*, [16 x i8] }>* @364 to %"panic::location::Location"*)) #26
in function _ZN4core9panicking15panic_no_unwind17heb5bd7bc5de639ecE
LLVM ERROR: Broken function found, compilation aborted!
Incorrect number of arguments passed to called function!
  call void @_ZN4core9panicking15panic_no_unwind17h8631ddd8246c176fE() #16
in function _ZN4core9panicking15panic_no_unwind17h8631ddd8246c176fE
LLVM ERROR: Broken function found, compilation aborted!

Locally, I'm not able to reproduce these (perhaps need to enable LLVM asserts or something), but I do see the following in stage 1 std compilation when building with track_caller.

error: failed to parse bitcode for LTO module: Invalid record (Producer: 'LLVM14.0.0-rust-1.61.0-nightly' Reader: 'LLVM 14.0.0-rust-1.61.0-nightly')

error: could not compile `core` due to previous error

cc @Amanieu @nbdd0121 -- seems plausible that something in #92911 is causing this

@Mark-Simulacrum Mark-Simulacrum added A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-track_caller `#![feature(track_caller)]` labels Feb 24, 2022
@Mark-Simulacrum Mark-Simulacrum changed the title track_caller on panci_no_unwind leads to track_caller on panic_no_unwind leads to Feb 24, 2022
@Mark-Simulacrum Mark-Simulacrum changed the title track_caller on panic_no_unwind leads to track_caller on panic_no_unwind leads to LLVM crash Feb 24, 2022
@Amanieu
Copy link
Member

Amanieu commented Feb 24, 2022

The relevant change is here. This function used to take a caller location but this was removed to reduce code size bloat.

@Mark-Simulacrum
Copy link
Member Author

I guess it seems ok for an internal function and a lang item, but I'm surprised that we don't have some proper error being emitted if the function is somehow being manually called -- that seems generally hazardous. (Should it have some special ABI declared at the Rust syntax level?)

I also note that the function still calls Location::caller: which seems useless or at least confusing without track_caller, right?

    let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller(), false);`

@Amanieu
Copy link
Member

Amanieu commented Feb 24, 2022

The panic_no_unwind function is only expected to be called by the compiler via the lang item. It is never used directly.

I also note that the function still calls Location::caller: which seems useless or at least confusing without track_caller, right?

That's the only way to construct a Location which is required for a PanicInfo.

@nbdd0121
Copy link
Contributor

Oops. I think I started working on #92911 before panic_no_unwind lands on beta and I forget to add cfg bootstrap after it's promoted into beta.

@Mark-Simulacrum
Copy link
Member Author

Even if it's not used directly, we should probably be adding a comment to the function about that, right? It seems like such a function should be documented as being compiler-only.

Anyway, seems OK -- I can adjust my FIXME comment to include this information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation F-track_caller `#![feature(track_caller)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants