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 fails when building rust 1.85.0 for ARM targets #137366

Open
jpalus opened this issue Feb 21, 2025 · 18 comments
Open

rustdoc fails when building rust 1.85.0 for ARM targets #137366

jpalus opened this issue Feb 21, 2025 · 18 comments
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jpalus
Copy link

jpalus commented Feb 21, 2025

Running ./x.py dist results in error on ARM 32-bit targets, works fine for aarch64 though:

arm-unknown-linux-gnueabihf:

...
error: target feature `fp-armv8` cannot be toggled with `#[target_feature]`: Rust ties `fp-armv8` to `neon`
     --> library/core/src/../../stdarch/crates/core_arch/src/arm_shared/neon/generated.rs:20668:48
      |
20668 | #[cfg_attr(target_arch = "arm", target_feature(enable = "fp-armv8,v8"))]
      |                                                ^^^^^^^^^^^^^^^^^^^^^^

error: target feature `fp-armv8` cannot be toggled with `#[target_feature]`: Rust ties `fp-armv8` to `neon`
     --> library/core/src/../../stdarch/crates/core_arch/src/arm_shared/neon/generated.rs:20688:48
      |
20688 | #[cfg_attr(target_arch = "arm", target_feature(enable = "fp-armv8,v8"))]
      |                                                ^^^^^^^^^^^^^^^^^^^^^^

error: could not document `core`

Caused by:
  process didn't exit successfully: `/home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/bootstrap/debug/rustdoc --edition=2021 --crate-type lib --crate-name core library/core/src/lib.rs --target arm-unknown-linux-gnueabihf -o /home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/arm-unknown-linux-gnueabihf/stage2-std/arm-unknown-linux-gnueabihf/doc/arm-unknown-linux-gnueabihf/doc --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values("debug_refcell", "optimize_for_size", "panic_immediate_abort"))' --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --warn=unexpected_cfgs --check-cfg 'cfg(bootstrap)' --check-cfg 'cfg(no_fp_fmt_parse)' --check-cfg 'cfg(stdarch_intel_sde)' --check-cfg 'cfg(target_arch, values("xtensa"))' --check-cfg 'cfg(feature, values(any()))' -C metadata=cc6cca0d841652cf -L dependency=/home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/arm-unknown-linux-gnueabihf/stage2-std/arm-unknown-linux-gnueabihf/doc/arm-unknown-linux-gnueabihf/release/deps -L dependency=/home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/arm-unknown-linux-gnueabihf/stage2-std/arm-unknown-linux-gnueabihf/doc/release/deps -C debuginfo=0 -C strip=none -Csymbol-mangling-version=legacy '--check-cfg=cfg(feature,values(any()))' -Zunstable-options '--check-cfg=cfg(bootstrap)' '--check-cfg=cfg(test)' -Dwarnings '-Wrustdoc::invalid_codeblock_attributes' --crate-version '1.85.0	(4d91de4e4	    2025-02-17)	(builtfrom	   a	    source	tarball)' '-Zcrate-attr=doc(html_root_url="https://doc.rust-lang.org/1.85.0/")' '-Zcrate-attr=warn(rust_2018_idioms)' --extern-html-root-url 'std_detect=https://docs.rs/std_detect/latest/' --extern-html-root-takes-precedence --resource-suffix 1.85.0 --markdown-css rust.css --markdown-no-toc --index-page /home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/src/doc/index.md -Zunstable-options` (exit status: 1)
warning: build failed, waiting for other jobs to finish...

armv7-unknown-linux-gnueabihf:

...
error: target feature `neon` cannot be toggled with `#[target_feature]`: unsound on hard-float targets because it changes float ABI
    --> library/core/src/../../stdarch/crates/core_arch/src/arm/neon.rs:1398:18
     |
1398 | #[target_feature(enable = "neon,v7,aes")]
     |                  ^^^^^^^^^^^^^^^^^^^^^^

error: target feature `neon` cannot be toggled with `#[target_feature]`: unsound on hard-float targets because it changes float ABI
    --> library/core/src/../../stdarch/crates/core_arch/src/arm/neon.rs:1415:18
     |
1415 | #[target_feature(enable = "neon,v7,aes")]
     |                  ^^^^^^^^^^^^^^^^^^^^^^

error: could not document `core`

Caused by:
  process didn't exit successfully: `/home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/bootstrap/debug/rustdoc --edition=2021 --crate-type lib --crate-name core library/core/src/lib.rs --target armv7-unknown-linux-gnueabihf -o /home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/armv7-unknown-linux-gnueabihf/stage2-std/armv7-unknown-linux-gnueabihf/doc/armv7-unknown-linux-gnueabihf/doc --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values("debug_refcell", "optimize_for_size", "panic_immediate_abort"))' --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --warn=unexpected_cfgs --check-cfg 'cfg(bootstrap)' --check-cfg 'cfg(no_fp_fmt_parse)' --check-cfg 'cfg(stdarch_intel_sde)' --check-cfg 'cfg(target_arch, values("xtensa"))' --check-cfg 'cfg(feature, values(any()))' -C metadata=4ab4e812d278f910 -L dependency=/home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/armv7-unknown-linux-gnueabihf/stage2-std/armv7-unknown-linux-gnueabihf/doc/armv7-unknown-linux-gnueabihf/release/deps -L dependency=/home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/build/armv7-unknown-linux-gnueabihf/stage2-std/armv7-unknown-linux-gnueabihf/doc/release/deps -C debuginfo=0 -C strip=none -Csymbol-mangling-version=legacy '--check-cfg=cfg(feature,values(any()))' -Zunstable-options '--check-cfg=cfg(bootstrap)' '--check-cfg=cfg(test)' -Dwarnings '-Wrustdoc::invalid_codeblock_attributes' --crate-version '1.85.0	    (4d91de4e4	  2025-02-17)	    (built	from	a	   source	tarball)' '-Zcrate-attr=doc(html_root_url="https://doc.rust-lang.org/1.85.0/")' '-Zcrate-attr=warn(rust_2018_idioms)' --extern-html-root-url 'std_detect=https://docs.rs/std_detect/latest/' --extern-html-root-takes-precedence --resource-suffix 1.85.0 --markdown-css rust.css --markdown-no-toc --index-page /home/users/builder/rpm/BUILD/rust-1.85.0-build/rustc-1.85.0-src/src/doc/index.md -Zunstable-options` (exit status: 1)
warning: build failed, waiting for other jobs to finish...

Build is performed on aarch64 host although I think it shouldn't really matter.

@jpalus jpalus added the C-bug Category: This is a bug. label Feb 21, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 21, 2025
@ehuss
Copy link
Contributor

ehuss commented Feb 21, 2025

Thanks for the report! I'm able to reproduce this (on latest master, too).

What I'm thinking is that here it is collecting a map of all features. However, ARM and AARCH64 both enable features of the same name, but with different Stability settings. Thus there are key collisions in the map.

cc @RalfJung since it looks like you've been doing some work here. (Presumably this is from #133417)

@ehuss ehuss added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Feb 21, 2025
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 21, 2025
@RalfJung
Copy link
Member

RalfJung commented Feb 21, 2025

Hm... fp-armv8 is unstable (and allowed) as an ARM target feature, but "forbidden" as an aarch64 target feature. It used to just not be listed for aarch64, which would also make it a hard error when used with #[target_feature]. Now it is listed as "forbidden". And somehow that seems to leak over onto the ARM side?

@ehuss yeah I think you found the culprit, for rustdoc we're merging all features into one map and that just doesn't make sense, features of the same name on different targets will overwrite each other and we'll get some "random" final idea of what is and is not stable. It's kind of surprising that this worked for so long.

Cc @workingjubilee @rust-lang/rustdoc

@RalfJung
Copy link
Member

RalfJung commented Feb 21, 2025

The immediate short-term fix I can think of is to do that merging in a more clever way, where the "more stable" feature takes precedent.

But really I feel this needs to be redesigned, it's just not coherent to say "use all features from all targets". We probably want to only enable them all for cfg but the rust_target_features query should still return something sensible for the rest of the compiler.

@jieyouxu jieyouxu added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Feb 22, 2025
@truboxl
Copy link

truboxl commented Feb 25, 2025

armv7-linux-androideabi and thumbv7neon-linux-androideabi are also affected

Related: #134511, #136755

@RalfJung
Copy link
Member

FWIW there is a potential stability hazard here: rustdoc will (have to) accept code with #[target_feature(enable = "foo")] on all targets once foo is stable on one target. I hope our stability policy says that code which builds in rustdoc but not rustc is not considered stable.

@RalfJung
Copy link
Member

#137632 should fix the issue (but not the stability hazard).

@notriddle
Copy link
Contributor

I hope our stability policy says that code which builds in rustdoc but not rustc is not considered stable.

Is there any reason we can't emit docs for target_features that don't even exist? It doesn't seem to affect type checking, and we already have to allow a bunch of other code in rustdoc that rustc would've rejected.

@RalfJung
Copy link
Member

I don't understand the question / comment, sorry. Too many negations.^^ This is about #[target_feature], not cfg(target_feature).

I don't even know what exactly rustdoc currently does for cfg here, whether it does anything special.

@notriddle
Copy link
Contributor

Rustdoc itself treats target_feature as equivalent to cfg(target_feature).

// treat #[target_feature(enable = "feat")] attributes as if they were
// #[doc(cfg(target_feature = "feat"))] attributes as well
for attr in hir_attr_lists(attrs, sym::target_feature) {
if attr.has_name(sym::enable) {
if attr.value_str().is_some() {
// Clone `enable = "feat"`, change to `target_feature = "feat"`.
// Unwrap is safe because `value_str` succeeded above.
let mut meta = attr.meta_item().unwrap().clone();
meta.path = ast::Path::from_ident(Ident::with_dummy_span(sym::target_feature));
if let Ok(feat_cfg) = Cfg::parse(&ast::MetaItemInner::MetaItem(meta)) {
cfg &= feat_cfg;
}
}
}
}

However, while I can use nonexistent target features in cfg, I can't use nonexistent enable flags.

$ cat lib.rs 
#[target_feature(enable="blablabla")]
pub fn add(left: u64, right: u64) -> u64 {
    left + right
}
#[cfg(target_feature="blablabla")]
pub fn sub(left: u64, right: u64) -> u64 {
    left - right
}

$ rustdoc lib.rs 
error: the feature named `blablabla` is not valid for this target
 --> lib.rs:1:18
  |
1 | #[target_feature(enable="blablabla")]
  |                  ^^^^^^^^^^^^^^^^^^ `blablabla` is not valid for this target

error: aborting due to 1 previous error

I'm surprised that this doesn't successfully generate docs. Rustdoc doesn't generate code, so why does it care about codegen parameters other than displaying them?

With doc(cfg(target_feature="blablabla")), rustdoc can display Image.

I hope our stability policy says that code which builds in rustdoc but not rustc is not considered stable.

The answer to this question has to be "yes." There's a bunch of other stuff that rustdoc accepts that rustc rejects and that we can't really make any promises about.

@RalfJung
Copy link
Member

RalfJung commented Feb 26, 2025

Rustdoc itself treats target_feature as equivalent to cfg(target_feature).

I don't know what that means. What I was asking is, if I have #[cfg(target_feature)] on a function, will it be present or not? Your reply then mentions #[doc(cfg(target_feature))] which I never heard about before.

I'm surprised that this doesn't successfully generate docs. Rustdoc doesn't generate code, so why does it care about codegen parameters other than displaying them?

Rustdoc also fails when the program is not valid Rust syntax, or ill-typed, or uses extern "foobar" fn(), so... why would it accept a nonsense program using a non-existent target feature?

Target features are not codegen parameters, they are a concept of Rust proper. Target features map to codegen parameters, but well, everything Rust does eventually maps to some aspect of codegen so that doesn't mean anything.

@notriddle
Copy link
Contributor

Target features are not codegen parameters, they are a concept of Rust proper.

Got it.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 1, 2025
…r=workingjubilee

rustdoc: when merging target features, keep the highest stability

This addresses rust-lang#137366. (Not closing since we might consider a backport.)

rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that.

This PR makes the  `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 2, 2025
…r=workingjubilee

rustdoc: when merging target features, keep the highest stability

This addresses rust-lang#137366. (Not closing since we might consider a backport.)

rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that.

This PR makes the  `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 2, 2025
…r=workingjubilee

rustdoc: when merging target features, keep the highest stability

This addresses rust-lang#137366. (Not closing since we might consider a backport.)

rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that.

This PR makes the  `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 3, 2025
…r=workingjubilee

rustdoc: when merging target features, keep the highest stability

This addresses rust-lang#137366. (Not closing since we might consider a backport.)

rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that.

This PR makes the  `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 3, 2025
Rollup merge of rust-lang#137632 - RalfJung:rustdoc-target-features, r=workingjubilee

rustdoc: when merging target features, keep the highest stability

This addresses rust-lang#137366. (Not closing since we might consider a backport.)

rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that.

This PR makes the  `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.
@extended-euclidean
Copy link

There's still no 1.85 version for termux because of this. Is there a rough estimate of when this will be fixed?

@ehuss
Copy link
Contributor

ehuss commented Mar 4, 2025

AFAIK, it is fixed, and is nominated for backport to 1.86.

@RalfJung
Copy link
Member

RalfJung commented Mar 5, 2025

@jpalus @extended-euclidean @truboxl would be great if you could test whether the latest nightly fixes this.

This seems to affect fairly few people, so I am not sure if there will be a 1.85.1 release due to this, but yeah 1.86 should hopefully contain the fix.

@jpalus
Copy link
Author

jpalus commented Mar 5, 2025

So I made an attempt at building nightly and I think things are looking good, better? I guess? :)

On the bright side I see dist created doc artifacts:

arm-unknown-linux-gnueabihf ie:

./build/dist/rust-docs-nightly-arm-unknown-linux-gnueabihf.tar.xz

armv7-unknown-linux-gnueabihf with additional support for thumbv7neon-unknown-linux-gnueabihf target ie:

./build/dist/rust-docs-nightly-armv7-unknown-linux-gnueabihf.tar.xz
./build/dist/rust-docs-nightly-thumbv7neon-unknown-linux-gnueabihf.tar.xz

Downside being build failed due to unresolved symbols when linking miri (completely unrelated to this ticket):

...
          miri.c2ff4e6c5b969fe1-cgu.13:(.text._RINvNtNtCs2kcHjSSSugX_6libffi4high4call4calliECsgJXTj7Eb90P_4miri+0xa4): undefined reference to `ffi_call'
          /usr/bin/ld: /home/users/builder/rpm/BUILD/rust-1.87.0-build/rustc-nightly-src/build/armv7-unknown-linux-gnueabihf/stage1-tools/armv7-unknown-linux-gnueabihf/release/deps/libmiri-7f341be697efc667.rlib(miri-7f341be697efc667.miri.c2ff4e6c5b969fe1-cgu.13.rcgu.o): in function `libffi::high::call::call::<usize>':
...
          prep_cif.c:(.text.ffi_prep_cif_core+0x96): undefined reference to `ffi_prep_cif_machdep'

The static library libffi.a that was built has both these symbols undefined?

$ objdump -t libffi.a|grep 'ffi_call\|ffi_prep_cif_machdep'
00000000         *UND*	00000000 ffi_prep_cif_machdep
00000000         *UND*	00000000 ffi_prep_cif_machdep_var
00000000         *UND*	00000000 ffi_call
00000000         *UND*	00000000 ffi_call

I think these should be defined, no idea how it ends up like this. Maybe I messed something.

@RalfJung
Copy link
Member

RalfJung commented Mar 5, 2025

Yeah no idea what's going on with those linker errors, but let's not discuss that on this issue. :)

@apiraino
Copy link
Contributor

apiraino commented Mar 6, 2025

Assigning priority (discussion on Zulip).

Fixed in #137632

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 6, 2025
@truboxl
Copy link

truboxl commented Mar 6, 2025

would be great if you could test whether the latest nightly fixes this.

Thanks! I can confirm latest nightly is building for the 2 targets I listed.

@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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

9 participants