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

1.15.1 (and earlier) armhf failing Items_and_attributes_15 doc test, passes on other architectures #40260

Closed
infinity0 opened this issue Mar 4, 2017 · 10 comments · Fixed by rust-lang/reference#71
Assignees

Comments

@infinity0
Copy link
Contributor

8 months ago back in version 1.10.0 this test was called Items_and_attributes_14 and it hung rather than failed, here is a stacktrace.

We can ignore it with this patch:

--- rustc.git.orig/src/doc/reference.md
+++ rustc.git/src/doc/reference.md
@@ -1133,7 +1133,7 @@ bodies defined in Rust code _can be call
 in the same way as any other Rust function, except that they have the `extern`
 modifier.
 
-```
+```{.ignore}
 // Declares an extern fn, the ABI defaults to "C"
 extern fn new_i32() -> i32 { 0 }
 
@infinity0 infinity0 changed the title 1.15.1 armhf failing Items_and_attributes_15 doc test, passes on other architectures 1.15.1 (and earlier) armhf failing Items_and_attributes_15 doc test, passes on other architectures Mar 4, 2017
@Mark-Simulacrum
Copy link
Member

@infinity0 I'm not sure what the bug is here. Is the test (now here) hanging, crashing, or failing today?

@infinity0
Copy link
Contributor Author

infinity0 commented Jun 15, 2017

Still failing today with 1.17.0 armv7, I tested on asachi.debian.org in a armv8l 32-bit chroot, with a clean upstream tarball (i.e. not the Debian package).

failures:

---- /home/infinity0/upstream/rustc-1.17.0-src/src/doc/reference/src/items.md - Items (line 425) stdout ----
        error[E0570]: The ABI `"stdcall"` is not supported for the current target
 --> <anon>:6:1
  |
6 | extern "stdcall" fn new_i32_stdcall() -> i32 { 0 }
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error(s)

thread 'rustc' panicked at 'Box<Any>', src/librustc/session/mod.rs:203
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    /home/infinity0/upstream/rustc-1.17.0-src/src/doc/reference/src/items.md - Items (line 425)

test result: FAILED. 37 passed; 1 failed; 11 ignored; 0 measured



command did not execute successfully: "/home/infinity0/upstream/rustc-1.17.0-src/build/armv7-unknown-linux-gnueabihf/stage2/bin/rustdoc" "--test" "/home/infinity0/upstream/rustc-1.17.0-src/src/doc/reference/src/items.md" "--test-args" ""
expected success, got: exit code: 101

Due to #41291 I can't use rust's own 1.16 as stage0, I'm using Debian's 1.16 dynamically linked against LLVM, which recently has had the fix for #41291 backported into it. I don't think this would be affecting this issue, but in case it is, here is my ./configure line:

rustc-1.17.0-src$ ./configure --enable-llvm-link-shared --llvm-root=/usr/lib/llvm-3.9 --enable-local-rust

and to make the build work (i.e. to reach this point where it fails) I have to use these patches:

@arielb1
Copy link
Contributor

arielb1 commented Jun 15, 2017

The problematic test is:

// Declares an extern fn, the ABI defaults to "C"
extern fn new_i32() -> i32 { 0 }

// Declares an extern fn with "stdcall" ABI
extern "stdcall" fn new_i32_stdcall() -> i32 { 0 }

"stdcall" only exists on x86. Looks like the test needs to be marked as rust,ignore, or as @kmcallister suggested, you can do

// Declares an extern fn, the ABI defaults to "C"
extern fn new_i32() -> i32 { 0 }

// Declares an extern fn with "stdcall" ABI
# #[cfg(target_arch = "x86_64")]
extern "stdcall" fn new_i32_stdcall() -> i32 { 0 }

@arielb1
Copy link
Contributor

arielb1 commented Jun 15, 2017

That is supposed to hide the test from the docs. I'll try to open a PR early next week if you won't open one before.

@arielb1 arielb1 self-assigned this Jun 15, 2017
@infinity0
Copy link
Contributor Author

infinity0 commented Jun 16, 2017

The #[cfg] guard could work yes, but this type of problem (ABI mismatch vs architecture) has come up before, e.g. #24958 and we have ignore-arm, ignore-aarch64 in src/test/debuginfo/type-names.rs still marked as FIXME.

Apparently there was some work done (#36421) to define which architectures support / don't support which ABIs and then this information could be used by the test runners to automatically ignore certain tests like this. I don't know what the progress is though, I only skimmed the ticket.

@arielb1
Copy link
Contributor

arielb1 commented Jun 16, 2017

Test runners are not supposed to automatically ignore tests. Maybe we should have a require-stdcall-abi flag or whatever that you can put on the test that makes it ignored on all archs that do not support stdcall, but the test runner itself should not start parsing tests and ignoring them.

@infinity0
Copy link
Contributor Author

Then perhaps one could extend #36421 to support a #[cfg(target_abi.contains("XXX"))] gate instead of hard-coding arch/ABI assumptions everywhere?

@arielb1
Copy link
Contributor

arielb1 commented Jun 16, 2017

Local problems, local fixes (implementing more features in cfg takes an RFC and a stabilization cycle and...).

@infinity0
Copy link
Contributor Author

infinity0 commented Jun 16, 2017

I understand. It would have wider applications though, outside of disabling tests in rustc. Suppose I want to write a -sys crate that links against some non-rust library. Then I could use this, instead of having to hard-code target_arch gates and having to maintain and update these for new architectures. Maybe something to consider for the future.

@arielb1
Copy link
Contributor

arielb1 commented Jun 16, 2017

@infinity0

Such a change would definitely require an RFC (in https://github.com/rust-lang/rfcs). Feel free to write one, but it will take at least half a year or so to get implemented.

arielb1 added a commit to arielb1/reference that referenced this issue Jun 21, 2017
The "stdcall" ABI only exists on x86, so this causes tests to fail on
other architectures. This should fix rust-lang/rust#40260 next time we
update the reference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants