-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
fix regression in panic_misaligned_pointer_dereference #118683
fix regression in panic_misaligned_pointer_dereference #118683
Conversation
…th panic_immediate_abort
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Yes, the entire point of that feature is to permit these symbols to be fully optimized out, along with all the formatting code that they drag in. Can you explain your use-case more? I don't understand how the feature used to work for you before; you're building an unoptimized standard library so it should be full of panic formatting code. |
I see. In that case it might make sense to investigate further within the compiler.
I'm building a Rather, my problem is that neither my program nor |
I can trivially reproduce all the behavior you're experiencing on Linux with the default linker. That's good, I suppose. Since I wrote the alignment checks and I've fiddled a lot with r? saethlin |
…achable, r=bjorn3 Tell MirUsedCollector that the pointer alignment checks calls its panic symbol Fixes rust-lang#118683 (not an issue, but that PR is a basically a bug report) When we had `panic_immediate_abort` start adding `#[inline]` to this panic function, builds started breaking because we failed to write up the MIR assert terminator to the correct panic shim. Things happened to work before by pure luck because without this feature enabled, the function we're inserting calls to is `#[inline(never)]` so we always generated code for it. r? bjorn3
Rollup merge of rust-lang#118693 - saethlin:alignment-check-symbol-reachable, r=bjorn3 Tell MirUsedCollector that the pointer alignment checks calls its panic symbol Fixes rust-lang#118683 (not an issue, but that PR is a basically a bug report) When we had `panic_immediate_abort` start adding `#[inline]` to this panic function, builds started breaking because we failed to write up the MIR assert terminator to the correct panic shim. Things happened to work before by pure luck because without this feature enabled, the function we're inserting calls to is `#[inline(never)]` so we always generated code for it. r? bjorn3
@thomasdangl The PR that I linked this to is now part of the latest nightly. Can you confirm that the latest nightly works for you? |
we were also effected by this in https://github.com/esp-rs/esp-idf-hal but our CI is working again with latest nightly. |
…nnethercote Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
…ethercote Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
…ethercote Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
…ethercote Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang/rust#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang/rust#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
Fix cases where std accidentally relied on inline(never) This PR increases the power of `-Zcross-crate-inline-threshold=always` so that it applies through `#[inline(never)]`. Note that though this is called "cross-crate-inlining" in this case especially it is _just_ lazy per-CGU codegen. The MIR inliner and LLVM still respect the attribute as much as they ever have. Trying to bootstrap with the new `-Zcross-crate-inline-threshold=always` change revealed two bugs: We have special intrinsics `assert_inhabited`, `assert_zero_valid`, and `assert_mem_uniniitalized_valid` which codegen backends will lower to nothing or a call to `panic_nounwind`. Since we may not have any call to `panic_nounwind` in MIR but emit one anyway, we need to specially tell `MirUsedCollector` about this situation. `#[lang = "start"]` is special-cased already so that `MirUsedCollector` will collect it, but then when we make it cross-crate-inlinable it is only assigned to a CGU based on whether `MirUsedCollector` saw a call to it, which of course we didn't. --- I started looking into this because rust-lang/rust#118683 revealed a case where we were accidentally relying on a function being `#[inline(never)]`, and cranking up cross-crate-inlinability seems like a way to find other situations like that. r? `@nnethercote` because I don't like what I'm doing to the CGU partitioning code here but I can't come up with something much better
#118362 introduced a regression that breaks pointer dereferencing in
core
.Until 8de5bd0 this worked totally fine.
However, since the commit ended up in master, the linker is quite unhappy about the symbol:
Is there a specific reason
panic_misaligned_pointer_dereference
is supposed to be inlined?Otherwise I suggest we just avoid inlining it for
panic_immediate_abort
.Perhaps this is also something to be added as an example for #118393.
Best,
Thomas