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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add abi-checker to y.rs and run it on CI #1255

Merged
merged 7 commits into from
Aug 13, 2022

Conversation

afonso360
Copy link
Contributor

@afonso360 afonso360 commented Aug 6, 2022

馃憢 Hey,

I've been playing around with abi-checker since I think the MSVC failures are ABI related.

This PR adds CI support for abi-checker. Currently we only run it in native scenarios (i.e. no cross compile).

Failures so far:

ubuntu-latest fails on pair cgclif_calls_cc:

Test ui128::c::rustc_calls_cc::u128_val_in_3_perturbed_small        passed
Test ui128::c::rustc_calls_cc::u128_val_in_0_perturbed_big          failed!
test 170 arg3 field 0 mismatch 
caller: [30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 3A, 3B, 3C, 3D, 3E, 3F] 
callee: [38, 39, 3A, 3B, 3C, 3D, 3E, 3F, 40, 41, 42, 43, 44, 45, 46, 47]
Test ui128::c::rustc_calls_cc::u128_val_in_1_perturbed_big          failed!
test 171 arg3 field 0 mismatch 
caller: [30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 3A, 3B, 3C, 3D, 3E, 3F] 
callee: [38, 39, 3A, 3B, 3C, 3D, 3E, 3F, 40, 41, 42, 43, 44, 45, 46, 47]
Test ui128::c::rustc_calls_cc::u128_val_in_2_perturbed_big          failed!
...

Segfault on native aarch64-unknown-linux-gnu for pair rustc_calls_cgclif:

skipping ptr::vectorcall::rustc_calls_rustc: rustc doesn't support convention vectorcall
generating structs::c::rustc_calls_rustc
compiling  structs::c::rustc_calls_rustc
running    structs::c::rustc_calls_rustc
Segmentation fault (core dumped)

STATUS_ACCESS_VIOLATION for MSVC on pair rustc_calls_cgclif:

generating bool::c::rustc_calls_rustc
compiling  bool::c::rustc_calls_rustc
running    bool::c::rustc_calls_rustc
error: process didn't exit successfully: `target\debug\abi-checker.exe --pairs cgclif_calls_rustc --add-rustc-codegen-backend cgclif:C:\Users\Afonso\CLionProjects\rustc_codegen_cranelift\build\bin\rus
tc_codegen_cranelift.dll` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)

I suspect that the aarch64 failures may be related to the issues we were having on #1184 but haven't investigated so far.

Edit(bjorn3): Fixes #1251

build_system/abi_checker.rs Outdated Show resolved Hide resolved
build_system/mod.rs Outdated Show resolved Hide resolved
@bjorn3
Copy link
Member

bjorn3 commented Aug 6, 2022

Thanks again!

ubuntu-latest fails on pair cgclif_calls_cc

This test fails with cg_llvm too. See rust-lang/rust#54341 (comment) @Gankra is there a way to skip this specific test for rustc<->cc, but keep it for cg_llvm<->cg_clif?

@bjorn3
Copy link
Member

bjorn3 commented Aug 6, 2022

Segfault on native aarch64-unknown-linux-gnu for pair rustc_calls_cgclif:

So I tried to set up an aarch64 qemu vm to debug this. Turns out emulation is so slow installing a minimal debian vm takes almost 3 and a half hours...

@afonso360
Copy link
Contributor Author

Send me an ssh key via zulip and i'll create you a user on an aarch64 server

@bjorn3
Copy link
Member

bjorn3 commented Aug 10, 2022

All i128 failures on AArch64 I mentioned in bytecodealliance/wasmtime#4634 (comment) have struct arguments emulating i128 rather than real i128.

running    sysv_i128_emulation::handwritten::rustc_calls_cc
Test sysv_i128_emulation::handwritten::rustc_calls_cc::callee_native_layout                     passed
Test sysv_i128_emulation::handwritten::rustc_calls_cc::callee_emulated_layout                   passed
Test sysv_i128_emulation::handwritten::rustc_calls_cc::callee_unaligned_emulated_layout         passed
Test sysv_i128_emulation::handwritten::rustc_calls_cc::native_to_native                         passed
Test sysv_i128_emulation::handwritten::rustc_calls_cc::native_to_emulated                       passed
Test sysv_i128_emulation::handwritten::rustc_calls_cc::native_to_unaligned_emulated             passed
Test sysv_i128_emulation::handwritten::rustc_calls_cc::emulated_to_native                       failed!
test 6 arg5 field 0 mismatch 
caller: [82, 0A, 12, E0, 01, 0C, 32, 7A, 42, F1, EA, 23, 4D, 3C, 2B, 1A] 
callee: [42, F1, EA, 23, 4D, 3C, 2B, 1A, 9C, D9, B9, F7, FF, FF, 00, 00]
Test sysv_i128_emulation::handwritten::rustc_calls_cc::emulated_to_emulated                     failed!
test 7 arg5 field 0 mismatch 
caller: [82, 0A, 12, E0, 01, 0C, 32, 7A, 42, F1, EA, 23, 4D, 3C, 2B, 1A] 
callee: [42, F1, EA, 23, 4D, 3C, 2B, 1A, A4, E1, B9, F7, FF, FF, 00, 00]
Test sysv_i128_emulation::handwritten::rustc_calls_cc::emulated_to_unaligned_emulated           failed!
test 8 arg5 field 0 mismatch 
caller: [82, 0A, 12, E0, 01, 0C, 32, 7A, 42, F1, EA, 23, 4D, 3C, 2B, 1A] 
callee: [42, F1, EA, 23, 4D, 3C, 2B, 1A, 8C, EC, B9, F7, FF, FF, 00, 00]
Test sysv_i128_emulation::handwritten::rustc_calls_cc::unaligned_emulated_to_native             failed!
test 9 arg5 field 0 mismatch 
caller: [82, 0A, 12, E0, 01, 0C, 32, 7A, 42, F1, EA, 23, 4D, 3C, 2B, 1A] 
callee: [42, F1, EA, 23, 4D, 3C, 2B, 1A, 00, F8, B9, F7, FF, FF, 00, 00]
Test sysv_i128_emulation::handwritten::rustc_calls_cc::unaligned_emulated_to_emulated           failed!
test 10 arg5 field 0 mismatch 
caller: [82, 0A, 12, E0, 01, 0C, 32, 7A, 42, F1, EA, 23, 4D, 3C, 2B, 1A] 
callee: [42, F1, EA, 23, 4D, 3C, 2B, 1A, C4, 03, BA, F7, FF, FF, 00, 00]
Test sysv_i128_emulation::handwritten::rustc_calls_cc::unaligned_emulated_to_unaligned_emulated failed!
test 11 arg5 field 0 mismatch 
caller: [82, 0A, 12, E0, 01, 0C, 32, 7A, 42, F1, EA, 23, 4D, 3C, 2B, 1A] 
callee: [42, F1, EA, 23, 4D, 3C, 2B, 1A, 8C, 0F, BA, F7, FF, FF, 00, 00]
only 6/12 tests passed!


Final Results:
by_ref::c::rustc_calls_cc        all  10/10  passed
opaque_example::handwritten::rustc_calls_cc all   1/1   passed
structs::c::rustc_calls_cc       all   9/9   passed
sysv_i128_emulation::handwritten::rustc_calls_cc       6/12  passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::callee_native_layout                     passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::callee_emulated_layout                   passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::callee_unaligned_emulated_layout         passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::native_to_native                         passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::native_to_emulated                       passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::native_to_unaligned_emulated             passed
  sysv_i128_emulation::handwritten::rustc_calls_cc::emulated_to_native                       failed!
  sysv_i128_emulation::handwritten::rustc_calls_cc::emulated_to_emulated                     failed!
  sysv_i128_emulation::handwritten::rustc_calls_cc::emulated_to_unaligned_emulated           failed!
  sysv_i128_emulation::handwritten::rustc_calls_cc::unaligned_emulated_to_native             failed!
  sysv_i128_emulation::handwritten::rustc_calls_cc::unaligned_emulated_to_emulated           failed!
  sysv_i128_emulation::handwritten::rustc_calls_cc::unaligned_emulated_to_unaligned_emulated failed!


26 passed, 6 failed, 0 completely failed, 8 skipped

@bjorn3
Copy link
Member

bjorn3 commented Aug 10, 2022

I suspect it has something to do with Cranelift not knowing about argument alignment at all.

@Gankra
Copy link

Gankra commented Aug 10, 2022

fyi i'm working on cleaning up a bunch of stuff to make this workable as something in CI

@Gankra
Copy link

Gankra commented Aug 10, 2022

WIP work here: Gankra/abi-cafe#13

@bjorn3
Copy link
Member

bjorn3 commented Aug 12, 2022

As of Gankra/abi-cafe#13 it is now easy to patch abi-checker to disable specific tests in certain conditions. See the code section starting with // THIS AREA RESERVED FOR VENDORS TO APPLY PATCHES. @afonso360 would you mind adding a patch to disable all know failing tests with a comment (result.check = Busted(Check); would work best I think). That would allow merging this PR and working on fixing those tests later.

@afonso360
Copy link
Contributor Author

afonso360 commented Aug 12, 2022

Hey, I couldn't get Busted(Check) to work for aarch64, for some reason the test still fails whenever we run it.

Good news is that MSVC is less broken than I expected! I'll try to find some time to track down the bool issues.

@bjorn3 bjorn3 merged commit 8c407e0 into rust-lang:master Aug 13, 2022
@bjorn3
Copy link
Member

bjorn3 commented Aug 13, 2022

Thanks! Opened #1261 to track the remaining failures.

@Gankra
Copy link

Gankra commented Aug 14, 2022

Hey, I couldn't get Busted(Check) to work for aarch64, for some reason the test still fails whenever we run it.

Could you provide any logs/details of the failure? You might be running into the fact that the current busted/fails system doesn't like it if you fail on the wrong stage. So Busted(Check) won't work if you fail in the Build/Link/Run phases. I don't think I give good error messages for that though.

@afonso360
Copy link
Contributor Author

You might be running into the fact that the current busted/fails system doesn't like it if you fail on the wrong stage. So Busted(Check) won't work if you fail in the Build/Link/Run phases. I don't think I give good error messages for that though.

That sounds like exactly what was happening. The test was segfaulting on a old version of abi-checker (the latest version when the PR was opened). With the current latest version it no longer segfaults, but the behaviour is sort of the same, we get something like

generating structs::c::rustc_calls_rustc
compiling  structs::c::rustc_calls_rustc
running    structs::c::rustc_calls_rustc

And it stops the tests right there (although not with a segfault). I think this is the behaviour that if we specify a run mode of Run or above.

@uweigand
Copy link
Contributor

Hi @afonso360 I just noticed that this is failing on s390x, because it assumes the ui128 tests should fail, but they actually pass. This seems to be because of this code in src/report.rs:

    // llvm and gcc disagree on the u128 ABI everywhere but aarch64 (arm64).
    // This is Bad! Ideally we should check for all clang<->gcc pairs but to start
    // let's mark rust <-> C as disagreeing (because rust also disagrees with clang).
    if !cfg!(target_arch = "aarch64") && test.test_name == "ui128" && is_rust_and_c {
        result.check = Busted(Check);
    }

"disagree everywhere but aarch64" seems a bold statement :-)

@bjorn3
Copy link
Member

bjorn3 commented Aug 30, 2022

You might want to leave a comment on rust-lang/rust#65111 about that.

@afonso360
Copy link
Contributor Author

Hey, I think we should fix that in abi-checker. I'm not too familiar with that comment, but it looks like we can relax it to aarch64 & s390x (although I suspect that the issue is really only with x86?).

We can apply a patch here, but it wouldn't be the best way to solve it.

@uweigand
Copy link
Contributor

I've now opened Gankra/abi-cafe#14 . Thanks!

@bjorn3
Copy link
Member

bjorn3 commented Sep 1, 2022

Updated abi-checker in 0966118.

@uweigand
Copy link
Contributor

uweigand commented Sep 1, 2022

Updated abi-checker in 0966118.

Thanks! ./y.rs test now fully passes again on s390x.

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 this pull request may close these issues.

Run abi-checker on CI
4 participants