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

Stop mentioning internal lang items in no_std binary errors #116343

Conversation

Noratrieb
Copy link
Member

When writing a no_std binary, you'll be greeted with nonsensical errors mentioning lang items like eh_personality and start. That's pretty bad because it makes you think that you need to define them somewhere! But oh no, now you're getting the internal_features lint telling you that you shouldn't use them! But you need a no_std binary! What now?

No problem! Writing a no_std binary is super easy. Just use panic=abort and supply your own platform specific entrypoint symbol (like main) and you're good to go. Would be nice if the compiler told you that, right?

This makes it so that it does do that.

I don't love the new messages yet, but they're decent I think. They can probably be improved, please suggest improvements.

@rustbot
Copy link
Collaborator

rustbot commented Oct 2, 2023

r? @cjgillot

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

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. labels Oct 2, 2023
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the maybe-dont-mention-all-the-weird-lang-items-just-saying branch from a5a795a to bbbe28d Compare October 2, 2023 13:10
|
= note: this can occur when a binary crate with `#![no_std]` is compiled for a target where `eh_personality` is defined in the standard library
= help: you may be able to compile for a target that doesn't need `eh_personality`, specify a target with `--target` or in `.cargo/config`
= help: specify panic="abort" instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this always correct? #[panic_handler] does work on no-std, so I thought you don't have to specify panic="abort". Is the previous help about the target not helpful?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unwinding panics require the eh_personality lang item. on targets that don't support unwinding at all, panic=abort is the default so you don't have to care about eh_personality at all. but on targets that do support unwinding (that people still want to write no_std binaries for sometimes) unwinding is still the default even with no_std.

@rukai
Copy link
Contributor

rukai commented Oct 3, 2023

The original suggestion:

you may be able to compile for a target that doesn't need `eh_personality`, specify a target with `--target` or in `.cargo/config`

Was arrived at because it's very easy to run cargo run and forget to include a --target and hit this error because the default target does not have unwind=abort.
So i think it's worth considering how the new error would be interpreted by a user making this easy mistake.

@Noratrieb
Copy link
Member Author

you may want to compile for a target that uses panic=abort by default, specify a target with `--target` or in `.cargo/config`

Adding this sounds reasonable.

@bjorn3
Copy link
Member

bjorn3 commented Oct 6, 2023

rust_eh_personality is currently always necessary if libcore is compiled with panic=unwind, even when using panic=abort for the end product. It should be possible to generate a dummy rust_eh_personality which aborts though if it is not defined but we are linking a panic=abort crate.

@Noratrieb
Copy link
Member Author

rust_eh_personality is currently always necessary if libcore is compiled with panic=unwind, even when using panic=abort for the end product

I don't think that's true? Compiling with -Cpanic=abort works.

@bjorn3
Copy link
Member

bjorn3 commented Oct 6, 2023

Only if you don't reference any functions in libcore that have rust_eh_personality as personality function and you don't pass -Clink-dead-code. If either condition is true you get an error like:

$ echo '#![no_std] #![no_main] #[panic_handler] fn panic_handler(_: &core::panic::PanicInfo<'"'"'_>) -> ! { loop {} } #[link(name = "c")] extern {} #[no_mangle] extern "C" fn main() { let mut foo = [0; 32]; foo.sort_unstable() }' | rustc - -Cpanic=abort -Clink-dead-code
error: linking with `cc` failed: exit status: 1
  |
  = note: LC_ALL="C" PATH="/home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin:/home/bjorn/.local/bin:/home/bjorn/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/games" VSLANG="1033" "cc" "-m64" "/tmp/rustcsLYlQT/symbols.o" "rust_out.rust_out.40d35c37ea8bccf-cgu.0.rcgu.o" "-Wl,--as-needed" "-L" "/home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bdynamic" "-lc" "-Wl,-Bstatic" "/home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-04991604e2730fd6.rlib" "/home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-f02db372677d4667.rlib" "/home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-448eba810c83029d.rlib" "-Wl,-Bdynamic" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "rust_out" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs"
  = note: /usr/bin/ld: /home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-f02db372677d4667.rlib(core-f02db372677d4667.core.63ccc595abf60ca6-cgu.0.rcgu.o):(.data.DW.ref.rust_eh_personality[DW.ref.rust_eh_personality]+0x0): undefined reference to `rust_eh_personality'
          collect2: error: ld returned 1 exit status
          
  = note: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
  = note: use the `-l` flag to specify native libraries to link
  = note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)

error: aborting due to previous error

@Noratrieb
Copy link
Member Author

I see. So users of no_std binaries should use -Zbuild-std to fully get rid of unwinding? So help: specify panic="abort" instead should say help: specify panic="abort" and use -Zbuild-std to avoid unwinding in the precompiled core instead

@bjorn3
Copy link
Member

bjorn3 commented Oct 8, 2023

Currently yes. I did call it a bug in rustc though. One option to fix this would be for rustc to synthesize an aborting personality function when linking a crate with the personality function. See also #106864

@bors
Copy link
Contributor

bors commented Oct 10, 2023

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

@cjgillot
Copy link
Contributor

I'm not sure where you discussion converged.
r? @bjorn3

@rustbot rustbot assigned bjorn3 and unassigned cjgillot Oct 21, 2023
@Dylan-DPC Dylan-DPC 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2023
@Noratrieb Noratrieb force-pushed the maybe-dont-mention-all-the-weird-lang-items-just-saying branch from bbbe28d to 7bcb83c Compare December 16, 2023 21:25
@Noratrieb Noratrieb added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 16, 2023
@bors
Copy link
Contributor

bors commented Dec 26, 2023

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

@Noratrieb Noratrieb force-pushed the maybe-dont-mention-all-the-weird-lang-items-just-saying branch from 7bcb83c to 560b2a3 Compare January 7, 2024 19:47
@rust-log-analyzer

This comment has been minimized.

@Noratrieb Noratrieb force-pushed the maybe-dont-mention-all-the-weird-lang-items-just-saying branch from 560b2a3 to 822c1e0 Compare January 7, 2024 19:57
@bors
Copy link
Contributor

bors commented Jan 8, 2024

📌 Commit 4533a77 has been approved by bjorn3

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 8, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 8, 2024
…he-weird-lang-items-just-saying, r=bjorn3

Stop mentioning internal lang items in no_std binary errors

When writing a no_std binary, you'll be greeted with nonsensical errors mentioning lang items like eh_personality and start. That's pretty bad because it makes you think that you need to define them somewhere! But oh no, now you're getting the `internal_features` lint telling you that you shouldn't use them! But you need a no_std binary! What now?

No problem! Writing a no_std binary is super easy. Just use panic=abort and supply your own platform specific entrypoint symbol (like `main`) and you're good to go. Would be nice if the compiler told you that, right?

This makes it so that it does do that.

I don't _love_ the new messages yet, but they're decent I think. They can probably be improved, please suggest improvements.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 8, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#116343 (Stop mentioning internal lang items in no_std binary errors)
 - rust-lang#118903 (Improved support of collapse_debuginfo attribute for macros.)
 - rust-lang#119033 (coverage: `llvm-cov` expects column numbers to be bytes, not code points)
 - rust-lang#119598 (Fix a typo in core::ops::Deref's doc)
 - rust-lang#119660 (remove an unnecessary stderr-per-bitwidth)
 - rust-lang#119663 (tests: Normalize `\r\n` to `\n` in some run-make tests)
 - rust-lang#119681 (coverage: Anonymize line numbers in branch views)
 - rust-lang#119704 (Fix two variable binding issues in lint let_underscore)
 - rust-lang#119725 (Add helper for when we want to know if an item has a host param)
 - rust-lang#119738 (Add `riscv32imafc-esp-espidf` tier 3 target for the ESP32-P4.)
 - rust-lang#119740 (Remove crossbeam-channel)

Failed merges:

 - rust-lang#119723 (Remove `-Zdont-buffer-diagnostics`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#119747 (comment)

@bors 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 8, 2024
@Noratrieb Noratrieb force-pushed the maybe-dont-mention-all-the-weird-lang-items-just-saying branch from 4533a77 to db930c6 Compare January 9, 2024 20:00
@Noratrieb
Copy link
Member Author

@bors r=bjorn3

@bors
Copy link
Contributor

bors commented Jan 9, 2024

📌 Commit db930c6 has been approved by bjorn3

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 9, 2024
@rust-log-analyzer

This comment has been minimized.

@matthiaskrgr
Copy link
Member

2024-01-09T20:04:57.2449779Z tidy error: following path contains more than 868 entries, you should move the test to some relevant subdirectory (current: 869): /checkout/tests/ui

@Noratrieb
Copy link
Member Author

I don't get this error locally 🤷 , ls -l also only shows 868 (and it does show the no_std tests). Let's see what happens in bors

@WaffleLapkin
Copy link
Member

@Nilstrieb might it be something akin to a merge conflict? (at least part of the CI is run after a test merge is done, so tidy might be running on code you don't have locally?)

@Noratrieb
Copy link
Member Author

I thought PR CI didn't merge master... but good point I should try that

@bjorn3
Copy link
Member

bjorn3 commented Jan 10, 2024

PR CI does merge master. Unlike bors runs this may however be outdated by the time the PR is actually merged into master.

When writing a no_std binary, you'll be greeted with nonsensical errors
mentioning lang items like eh_personality and start. That's pretty bad
because it makes you think that you need to define them somewhere! But
oh no, now you're getting the `internal_features` lint telling you that
you shouldn't use them! But you need a no_std binary! What now?

No problem! Writing a no_std binary is super easy. Just use panic=abort
and supply your own platform specific entrypoint symbol (like `main`)
and you're good to go. Would be nice if the compiler told you that,
right?

This makes it so that it does do that.
@Noratrieb Noratrieb force-pushed the maybe-dont-mention-all-the-weird-lang-items-just-saying branch from db930c6 to da26317 Compare January 10, 2024 20:19
@bjorn3
Copy link
Member

bjorn3 commented Jan 10, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jan 10, 2024

📌 Commit da26317 has been approved by bjorn3

It is now in the queue for this repository.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 11, 2024
Rollup of 3 pull requests

Successful merges:

 - rust-lang#116343 (Stop mentioning internal lang items in no_std binary errors)
 - rust-lang#119814 (bootstrap: exclude link_jobs from `check_ci_llvm!` checks)
 - rust-lang#119829 (Add debug info for macOS CI actions)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6794dc9 into rust-lang:master Jan 11, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 11, 2024
Rollup merge of rust-lang#116343 - Nilstrieb:maybe-dont-mention-all-the-weird-lang-items-just-saying, r=bjorn3

Stop mentioning internal lang items in no_std binary errors

When writing a no_std binary, you'll be greeted with nonsensical errors mentioning lang items like eh_personality and start. That's pretty bad because it makes you think that you need to define them somewhere! But oh no, now you're getting the `internal_features` lint telling you that you shouldn't use them! But you need a no_std binary! What now?

No problem! Writing a no_std binary is super easy. Just use panic=abort and supply your own platform specific entrypoint symbol (like `main`) and you're good to go. Would be nice if the compiler told you that, right?

This makes it so that it does do that.

I don't _love_ the new messages yet, but they're decent I think. They can probably be improved, please suggest improvements.
@Noratrieb Noratrieb deleted the maybe-dont-mention-all-the-weird-lang-items-just-saying branch January 11, 2024 16:19
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 S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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 this pull request may close these issues.

10 participants