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

rustdoc incorrectly evaluates and fails due to panic in zerocopy #122758

Closed
Mark-Simulacrum opened this issue Mar 20, 2024 · 6 comments
Closed

rustdoc incorrectly evaluates and fails due to panic in zerocopy #122758

Mark-Simulacrum opened this issue Mar 20, 2024 · 6 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

This code:

extern crate zerocopy;

pub use std::cell::RefMut as LockGuard;

with Cargo.toml:

[package]
name = "test-rustdoc-bug"
version = "0.1.0"
edition = "2021"

[dependencies]
zerocopy = "0.7.32"

produces the following error:

$ RUSTDOCFLAGS="-Zunstable-options --document-hidden-items" cargo +nightly doc
 Documenting test-rustdoc-bug v0.1.0 (/Users/mark/test-rustdoc-bug)
error[E0080]: evaluation of `<std::cell::RefMut<'_, [u8]> as zerocopy::ByteSlice>::INTO_REF_INTO_MUT_ARE_SOUND` failed
    --> /Users/mark/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.7.32/src/lib.rs:5238:9
     |
5238 |         panic!("Ref::into_ref and Ref::into_mut are unsound when used with core::cell::RefMut; see https://github.com/google/zerocopy/issues/716")
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Ref::into_ref and Ref::into_mut are unsound when used with core::cell::RefMut; see https://github.com/google/zerocopy/issues/716', /Users/mark/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.7.32/src/lib.rs:5238:9
     |
     = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0080`.
error: could not document `test-rustdoc-bug`

This seems to happen on all(?) of current stable (1.77), beta (1.78), and nightly (1.79, 3c85e56 2024-03-18) but I'm suspicious that there's some kind of new breakage here in 1.78, because this broke rust-lang/rust's CI on similar code (in rustc_data_structures): #122754 (comment) while performing a bump of the bootstrap compiler from 1.77 to 1.78. This minimal example appears to reproduce across all versions. Tagging this as a regression for now despite lacking a minimal reproducer demonstrating that.

I think this is likely a bug of some kind in rustdoc regardless, because presumably it's not intentional that we should fail to document (benign-looking) code like this.

@Mark-Simulacrum Mark-Simulacrum added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Mar 20, 2024
@Mark-Simulacrum Mark-Simulacrum added this to the 1.78.0 milestone Mar 20, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 20, 2024
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Mar 20, 2024
Filed rust-lang#122758 to track a proper fix, but this seems to solve the
problem in the meantime and is probably OK in terms of impact on
(internal) doc quality.
@Mark-Simulacrum
Copy link
Member Author

Mark-Simulacrum@eb7c254 works locally to resolve this bug, probably by just avoiding the relevant code path in rustdoc.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Mar 20, 2024
Filed rust-lang#122758 to track a proper fix, but this seems to solve the
problem in the meantime and is probably OK in terms of impact on
(internal) doc quality.
@GuillaumeGomez
Copy link
Member

This is strange. Tagging the compiler team as well.

@GuillaumeGomez GuillaumeGomez added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 20, 2024
@rust-lang rust-lang deleted a comment from algora-pbc Mar 20, 2024
@jieyouxu jieyouxu added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Apr 4, 2024
@apiraino
Copy link
Contributor

I've tried bisecting this error, but results are perhaps a bit iffy. Does any of these commit ring a bell?

Regression in nightly-2023-07-19
converted 2023-07-19 to 903e279

  commit[0] 2023-07-17: Auto merge of #89132 - Cyborus04:rc_allocator_support, r=Amanieu
  commit[1] 2023-07-18: Auto merge of #113061 - Amanieu:x86_64-ohos, r=compiler-errors
  commit[2] 2023-07-18: Auto merge of #113574 - GuillaumeGomez:rustdoc-json-strip-hidden-impl, r=aDotInTheVoid,notriddle
  commit[3] 2023-07-18: Auto merge of #113801 - compiler-errors:iter-instantiated, r=oli-obk
  commit[4] 2023-07-18: Auto merge of #113659 - ericmarkmartin:smir-refs-and-ptrs, r=spastorino
  commit[5] 2023-07-18: Auto merge of #113677 - bryangarza:unevaluated-const-ice_issue-110892, r=davidtwco
  commit[6] 2023-07-18: Auto merge of #113706 - Alexendoo:compiletest-backslash-re, r=oli-obk
  commit[7] 2023-07-18: Auto merge of #112374 - chenx97:better-mips64r6, r=jackh726
  commit[8] 2023-07-18: Auto merge of #113837 - matthiaskrgr:rollup-v4xud4s, r=matthiaskrgr
  commit[9] 2023-07-18: Auto merge of #113636 - compiler-errors:opaque-recursive-check-bad, r=oli-obk
  commit[10] 2023-07-18: Auto merge of #113841 - weihanglo:update-cargo, r=weihanglo

@Mark-Simulacrum
Copy link
Member Author

This is hitting the 1.79 promotion too. Probably the fix in Mark-Simulacrum@eb7c254 wasn't quite enough -- that did land on master, so it's not entirely missing.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Apr 28, 2024
This is a workaround for rust-lang#122758, but it's not clear why 1.79 requires a
more extensive amount of no_inline than the previous release. Seems like
there's something relatively subtle happening here.
@GuillaumeGomez
Copy link
Member

With everything going on in rustc_codegen_gcc, didn't have time to take a look yet...

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 28, 2024
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Apr 29, 2024
This is a workaround for rust-lang#122758, but it's not clear why 1.79 requires a
more extensive amount of no_inline than the previous release. Seems like
there's something relatively subtle happening here.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Apr 29, 2024
This is a workaround for rust-lang#122758, but it's not clear why 1.79 requires a
more extensive amount of no_inline than the previous release. Seems like
there's something relatively subtle happening here.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue May 2, 2024
This is a workaround for rust-lang#122758, but it's not clear why 1.79 requires a
more extensive amount of no_inline than the previous release. Seems like
there's something relatively subtle happening here.
bors added a commit to rust-lang-ci/rust that referenced this issue May 2, 2024
…bertlarsan68

Bump bootstrap compiler to latest beta

https://forge.rust-lang.org/release/process.html#master-bootstrap-update-t-2-day-tuesday

This also cherry-picks d716d72 from the beta branching, to continue to workaround rust-lang#122758.

r? bootstrap
@GuillaumeGomez
Copy link
Member

Took a look at it and it seems normal. Can be reproduced with this foo crate as dependency (no need to have a current crate, just using foo as a dependency is enough):

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

src/lib.rs:

pub const YEP: bool = if !cfg!(doc) {
    panic!("bad");
} else {
    false
};

The problem is that the dependencies are built with rustc before being built with rustdoc (to get crate information). So unless zerocopy is the final crate, this condition will always at some point be true. Problem seems to be solved in newest zerocopy versions so I think we can close this issue.

@apiraino apiraino removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants