rustc_codegen_cranelift icon indicating copy to clipboard operation
rustc_codegen_cranelift copied to clipboard

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

Open afonso360 opened this issue 3 years ago • 7 comments

👋 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 https://github.com/bjorn3/rustc_codegen_cranelift/issues/1251

afonso360 avatar Aug 06 '22 15:08 afonso360

Thanks again!

ubuntu-latest fails on pair cgclif_calls_cc

This test fails with cg_llvm too. See https://github.com/rust-lang/rust/issues/54341#issuecomment-1064729606 @Gankra is there a way to skip this specific test for rustc<->cc, but keep it for cg_llvm<->cg_clif?

bjorn3 avatar Aug 06 '22 16:08 bjorn3

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...

bjorn3 avatar Aug 06 '22 21:08 bjorn3

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

afonso360 avatar Aug 06 '22 21:08 afonso360

All i128 failures on AArch64 I mentioned in https://github.com/bytecodealliance/wasmtime/pull/4634#issuecomment-1210649224 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 avatar Aug 10 '22 13:08 bjorn3

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

bjorn3 avatar Aug 10 '22 13:08 bjorn3

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

Gankra avatar Aug 10 '22 14:08 Gankra

WIP work here: https://github.com/Gankra/abi-checker/pull/13

Gankra avatar Aug 10 '22 18:08 Gankra

As of https://github.com/Gankra/abi-checker/pull/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.

bjorn3 avatar Aug 12 '22 14:08 bjorn3

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.

afonso360 avatar Aug 12 '22 23:08 afonso360

Thanks! Opened https://github.com/bjorn3/rustc_codegen_cranelift/issues/1261 to track the remaining failures.

bjorn3 avatar Aug 13 '22 06:08 bjorn3

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.

Gankra avatar Aug 14 '22 03:08 Gankra

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.

afonso360 avatar Aug 14 '22 17:08 afonso360

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 :-)

uweigand avatar Aug 30 '22 13:08 uweigand

You might want to leave a comment on https://github.com/rust-lang/rust/issues/65111 about that.

bjorn3 avatar Aug 30 '22 13:08 bjorn3

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.

afonso360 avatar Aug 31 '22 09:08 afonso360

I've now opened https://github.com/Gankra/abi-checker/pull/14 . Thanks!

uweigand avatar Aug 31 '22 11:08 uweigand

Updated abi-checker in 096611855c90561188cb73d0e4a656e27a710d91.

bjorn3 avatar Sep 01 '22 09:09 bjorn3

Updated abi-checker in 0966118.

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

uweigand avatar Sep 01 '22 11:09 uweigand