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

Track test suite failures on s390x #105383

Closed
8 tasks done
uweigand opened this issue Dec 6, 2022 · 3 comments
Closed
8 tasks done

Track test suite failures on s390x #105383

uweigand opened this issue Dec 6, 2022 · 3 comments

Comments

@uweigand
Copy link
Contributor

uweigand commented Dec 6, 2022

Running the ./x.py test suite on s390x currently results in a number of failing tests due to various issues. I've gone through and fixed all of those, resulting in a clean run with all my local fixes applied. Opening this issue to track getting all requires fixes upstream.

Specifically, I'm seeing the following failures caused by actual code problems:

In addition, a number of test cases show (false positive) failures due to assumptions that aren't correct for the platform:

  • LLD does not support the s390x target:
    src/test/run-make/issue-71519
    (Test needs to be ignored.)
  • The abi_efiapi feature is not supported on s390x:
    src/doc/unstable-book/src/language-features/abi-efiapi.md
    (Test needs to be ignored.)
  • A number of LLVM code-gen tests make invalid assumptions on how the IR looks on s390x:
    src/test/codegen/catch-unwind.rs
    src/test/codegen/remap_path_prefix/main.rs
    src/test/codegen/repr-transparent-aggregates-1.rs
    src/test/codegen/repr-transparent.rs
    src/test/codegen/uninit-consts.rs
    (I have fixes for these.)
  • Several test cases make little-endian assumptions in output files that are tested against - UI tests
    src/test/ui/const-ptr/forbidden_slices.rs
    src/test/ui/consts/const-eval/ub-enum.rs
    src/test/ui/consts/const-eval/ub-nonnull.rs
    src/test/ui/consts/const-eval/ub-ref-ptr.rs
    src/test/ui/consts/const-eval/ub-uninhabit.rs
    src/test/ui/consts/const-eval/ub-wide-ptr.rs
    src/test/ui/consts/issue-83182.rs
    src/test/ui/consts/std/alloc.rs
    src/test/ui/consts/validate_never_arrays.rs
    Some (but not all) of these are listed in 1.53.0 broke ui/consts/const-eval/ub-*.rs tests on s390x (big-endian, 64-bit) (regression) #89577, with various solutions being proposed. Options would be to either ignore those on big-endian hosts, provide multiple versions of the output to match against, and/or try to handle big- and little-endian outputs within the same file (e.g. via some transformation).
  • Several test cases make little-endian assumptions in output files that are tested against - MIR tests
    src/test/mir-opt/const_prop/mutable_variable_no_prop.rs
    src/test/mir-opt/issues/issue_75439.rs
    src/test/mir-opt/building/custom/consts.rs

Longer term, I'm wondering what the best way would be to ensure that the test suite remains clean on the platform - can this be included in CI somehow (either via a native machine somewhere or via qemu-based testing)?

CC @cuviper

@cuviper
Copy link
Member

cuviper commented Dec 6, 2022

Thanks for your attention on this!

  • Several test cases make little-endian assumptions in output files that are tested against

I think I caught most of these in #102379, but only by ignoring them. There's a suggestion that we might apply more fine-grained filtering on that, but I haven't gotten around to implementing that yet.

Longer term, I'm wondering what the best way would be to ensure that the test suite remains clean on the platform - can this be included in CI somehow (either via a native machine somewhere or via qemu-based testing)?

Generally speaking, that's defined by Platform Support and the related Target Tier Policy, where tier-2 (like s390x) is only cross-built in CI, and tier-1 adds test gating. Aarch64-linux is currently the only non-x86 target at tier-1. Part of the challenge is that most developers simply don't have access to s390x hardware to diagnose problems, so it would be burdensome to make that a blocking target.

Perhaps you have resources that could be applied more ad-hoc? e.g. Google has their own CI building Rust with the latest LLVM main branch, from which they report issues (or usually direct fixes). If you could set up similar external CI for s390x testing, we could get a shorter feedback loop without making it a blocking tier-1 target.

uweigand added a commit to uweigand/rust that referenced this issue Dec 6, 2022
Several codegen tests are currently failing due to making
assumptions that are not valid for the s390x architecture:

- catch-unwind.rs: fails due to inlining differences.
  Already ignored on another platform for the same reason.
  Solution: Ignore on s390x.

- remap_path_prefix/main.rs: fails due to different alignment
  requirement for string constants.
  Solution: Do not test for the alignment requirement.

- repr-transparent-aggregates-1.rs: many ABI assumptions.
  Already ignored on many platforms for the same reason.
  Solution: Ignore on s390x.

- repr-transparent.rs: no vector ABI by default on s390x.
  Already ignored on another platform for a similar reason.
  Solution: Ignore on s390x.

- uninit-consts.rs: hard-coded little-endian constant.
  Solution: Match both little- and big-endian versions.

Fixes part of rust-lang#105383.
@uweigand
Copy link
Contributor Author

uweigand commented Dec 6, 2022

Perhaps you have resources that could be applied more ad-hoc? e.g. Google has their own CI building Rust with the latest LLVM main branch, from which they report issues (or usually direct fixes). If you could set up similar external CI for s390x testing, we could get a shorter feedback loop without making it a blocking tier-1 target.

This is certainly something we could set up. I guess I'm mostly concerned about endian issues, which seem to crop up regularly. Having at least one big-endian target in the CI might be useful ... But we can start with our own external CI, and see how that goes over time.

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 10, 2022
…Simulacrum

Fix failing codegen tests on s390x

Several codegen tests are currently failing due to making assumptions that are not valid for the s390x architecture:

- catch-unwind.rs: fails due to inlining differences. Already ignored on another platform for the same reason. Solution: Ignore on s390x.

- remap_path_prefix/main.rs: fails due to different alignment requirement for string constants. Solution: Do not test for the alignment requirement.

- repr-transparent-aggregates-1.rs: many ABI assumptions. Already ignored on many platforms for the same reason. Solution: Ignore on s390x.

- repr-transparent.rs: no vector ABI by default on s390x. Already ignored on another platform for a similar reason. Solution: Ignore on s390x.

- uninit-consts.rs: hard-coded little-endian constant. Solution: Match both little- and big-endian versions.

Fixes part of rust-lang#105383.
RalfJung pushed a commit to RalfJung/miri that referenced this issue Dec 11, 2022
Fix failing codegen tests on s390x

Several codegen tests are currently failing due to making assumptions that are not valid for the s390x architecture:

- catch-unwind.rs: fails due to inlining differences. Already ignored on another platform for the same reason. Solution: Ignore on s390x.

- remap_path_prefix/main.rs: fails due to different alignment requirement for string constants. Solution: Do not test for the alignment requirement.

- repr-transparent-aggregates-1.rs: many ABI assumptions. Already ignored on many platforms for the same reason. Solution: Ignore on s390x.

- repr-transparent.rs: no vector ABI by default on s390x. Already ignored on another platform for a similar reason. Solution: Ignore on s390x.

- uninit-consts.rs: hard-coded little-endian constant. Solution: Match both little- and big-endian versions.

Fixes part of rust-lang/rust#105383.
uweigand added a commit to uweigand/rust that referenced this issue Dec 19, 2022
The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case
added to verify rust-lang#74564 still
crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash
happens at a nesting depth of over 17000 stack frames when using the
default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack
call to the with_lint_attrs routine (which also should take care
of all the other mutually recursive visitors here).

Fixes part of rust-lang#105383.
uweigand added a commit to uweigand/rust that referenced this issue Dec 27, 2022
A number of tests under ui/const-ptr and ui/consts are currently
failing on big-endian platforms as the binary encoding of some
constants is hard-coded in the stderr test files.  Fix this by
providing a normalize-stderr-test rule that strips out the
raw bytes hex dump, so the comparison can be done in an
endianness-independent manner.  Note that in most cases, this
means the tests are now also independent of word size, so the
32bit and 64bit cases can be re-unified.

To keep tests that verify the details of those raw bytes dumps,
a new test case raw-bytes.rs performs the tests where the hex
dumps were stripped out a second time, but only on little-
endian platforms.

In addition, src/test/ui/const-ptr/forbidden_slices.rs exposes
an endian-specific difference in this diagnostic output:
   constructing invalid value at .<deref>[0]: encountered 0x11,
   but expected a boolean
depending on which byte of D0 is not a boolean value (0 or 1).
Fixed this by choosing a value of D0 that differs from 0 or 1
in all bytes.

Fixes part of rust-lang#105383.
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 9, 2023
…r=oli-obk

Fix ui constant tests for big-endian platforms

A number of tests under ui/const-ptr and ui/consts are currently failing on big-endian platforms as the binary encoding of some constants is hard-coded in the stderr test files.

Fix this by a combination of two types of changes:

- Where possible (i.e. where the particular value of a constant does not affect the purpose of the test), choose constant values that have the same encoding on big- and little-endian platforms.

- Where this is not possible, provide a normalize-stderr-test rule that transforms the printed big-endian encoding of such constants into the corresponding little-endian form.

Fixes part of rust-lang#105383.
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 9, 2023
…r=oli-obk

Fix ui constant tests for big-endian platforms

A number of tests under ui/const-ptr and ui/consts are currently failing on big-endian platforms as the binary encoding of some constants is hard-coded in the stderr test files.

Fix this by a combination of two types of changes:

- Where possible (i.e. where the particular value of a constant does not affect the purpose of the test), choose constant values that have the same encoding on big- and little-endian platforms.

- Where this is not possible, provide a normalize-stderr-test rule that transforms the printed big-endian encoding of such constants into the corresponding little-endian form.

Fixes part of rust-lang#105383.
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this issue Jan 9, 2023
…r=oli-obk

Fix ui constant tests for big-endian platforms

A number of tests under ui/const-ptr and ui/consts are currently failing on big-endian platforms as the binary encoding of some constants is hard-coded in the stderr test files.

Fix this by a combination of two types of changes:

- Where possible (i.e. where the particular value of a constant does not affect the purpose of the test), choose constant values that have the same encoding on big- and little-endian platforms.

- Where this is not possible, provide a normalize-stderr-test rule that transforms the printed big-endian encoding of such constants into the corresponding little-endian form.

Fixes part of rust-lang#105383.
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 11, 2023
…strieb

Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang#105383.
uweigand added a commit to uweigand/rust that referenced this issue Jan 12, 2023
The test cases src/test/mir-opt/building/custom/consts.rs and
src/test/mir-opt/const_prop/mutable_variable_no_prop.rs are
currently failing on big-endian platforms as the binary encoding
of some constants is hard-coded in the MIR test files.  Fix this
by choosing constant values that have the same encoding on big-
and little-endian platforms.

The test case src/test/mir-opt/issues/issue_75439.rs is failing
as well, but since the purpose of the test is to validate handling
of big-endian integer encodings on a little-endian platform, it does
not make much sense to run it on big-endian platforms in the first
place - we can just ignore it there.

Fixed part of rust-lang#105383.
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jan 13, 2023
Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang/rust#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang/rust#105383.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2023
… r=Mark-Simulacrum

Fix mir-opt tests for big-endian platforms

The test cases src/test/mir-opt/building/custom/consts.rs and src/test/mir-opt/const_prop/mutable_variable_no_prop.rs are currently failing on big-endian platforms as the binary encoding of some constants is hard-coded in the MIR test files.  Fix this by choosing constant values that have the same encoding on big- and little-endian platforms.

The test case src/test/mir-opt/issues/issue_75439.rs is failing as well, but since the purpose of the test is to validate handling of big-endian integer encodings on a little-endian platform, it does not make much sense to run it on big-endian platforms in the first place - we can just ignore it there.

Fixed part of rust-lang#105383.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2023
… r=Mark-Simulacrum

Fix mir-opt tests for big-endian platforms

The test cases src/test/mir-opt/building/custom/consts.rs and src/test/mir-opt/const_prop/mutable_variable_no_prop.rs are currently failing on big-endian platforms as the binary encoding of some constants is hard-coded in the MIR test files.  Fix this by choosing constant values that have the same encoding on big- and little-endian platforms.

The test case src/test/mir-opt/issues/issue_75439.rs is failing as well, but since the purpose of the test is to validate handling of big-endian integer encodings on a little-endian platform, it does not make much sense to run it on big-endian platforms in the first place - we can just ignore it there.

Fixed part of rust-lang#105383.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2023
… r=Mark-Simulacrum

Fix mir-opt tests for big-endian platforms

The test cases src/test/mir-opt/building/custom/consts.rs and src/test/mir-opt/const_prop/mutable_variable_no_prop.rs are currently failing on big-endian platforms as the binary encoding of some constants is hard-coded in the MIR test files.  Fix this by choosing constant values that have the same encoding on big- and little-endian platforms.

The test case src/test/mir-opt/issues/issue_75439.rs is failing as well, but since the purpose of the test is to validate handling of big-endian integer encodings on a little-endian platform, it does not make much sense to run it on big-endian platforms in the first place - we can just ignore it there.

Fixed part of rust-lang#105383.
@uweigand
Copy link
Contributor Author

All failures tracked here have now be resolved; the Rust test suite is currently clean on s390x.

uweigand added a commit to uweigand/rust that referenced this issue Aug 24, 2023
As of commit 7767cbb,
the tests/ui/consts/const-eval/ub-int-array.rs test is
failing on big-endian platforms (in particular s390x),
as the stderr output contains a hex dump that depends
on endianness.

Since this point intentionally verifies the hex dump to
check the uninitialized byte markers, I think we should
not simply standardize away the hex dump as is done with
some of the other tests in this directory.

However, most of the test is already endian-independent.
The only exception is one line of hex dump, which can
also be made endian-independent by choosing appropriate
constants in the source code.

Since the 32bit and 64bit stderr outputs were already
(and remain) identical, I've merged them and removed
the stderr-per-bitwidth marker.

Fixes (again) rust-lang#105383.
weihanglo added a commit to weihanglo/rust that referenced this issue Aug 24, 2023
…RalfJung

Fix ub-int-array test for big-endian platforms

As of commit 7767cbb, the tests/ui/consts/const-eval/ub-int-array.rs test is failing on big-endian platforms (in particular s390x), as the stderr output contains a hex dump that depends on endianness.

Since this point intentionally verifies the hex dump to check the uninitialized byte markers, I think we should not simply standardize away the hex dump as is done with some of the other tests in this directory.

However, most of the test is already endian-independent. The only exception is one line of hex dump, which can also be made endian-independent by choosing appropriate constants in the source code.

Since the 32bit and 64bit stderr outputs were already (and remain) identical, I've merged them and removed the stderr-per-bitwidth marker.

Fixes (again) rust-lang#105383.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang/rust#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang/rust#105383.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Fix stack overflow in recursive AST walk in early lint

The src/test/ui/issues/issue-74564-if-expr-stack-overflow.rs test case added to verify rust-lang/rust#74564 still crashes with a stack overflow on s390x-ibm-linux.

Symptom is a very deep recursion in compiler/rustc_lint/src/early.rs:
    fn visit_expr(&mut self, e: &'a ast::Expr) {
        self.with_lint_attrs(e.id, &e.attrs, |cx| {
            lint_callback!(cx, check_expr, e);
            ast_visit::walk_expr(cx, e);
        })
    }
(where walk_expr recursively calls back into visit_expr).  The crash happens at a nesting depth of over 17000 stack frames when using the default 8 MB stack size on s390x.

This patch fixes the problem by adding a ensure_sufficient_stack call to the with_lint_attrs routine (which also should take care of all the other mutually recursive visitors here).

Fixes part of rust-lang/rust#105383.
uweigand added a commit to uweigand/rust that referenced this issue Jul 10, 2024
A number of mir-opt test cases are failing again on big-endian
platforms, due to endian-specific binary encodings in the output
files that are being checked by the test.  To fix those, one of
the following two strategies is used:

a) Where a binary encoded value directly originates from some
immediate constant in the test case source code, change that
constant to an endian-invariant value (like 0x11000011).

b) Where that is not easily possible or would change the essence
of what the test is checking for, disable the test on big-endian
platforms.

Fixes (again) rust-lang#105383.
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

No branches or pull requests

2 participants