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

How to mark test as busted? #65

Open
Tracked by #60
bjorn3 opened this issue Jul 9, 2024 · 25 comments
Open
Tracked by #60

How to mark test as busted? #65

bjorn3 opened this issue Jul 9, 2024 · 25 comments

Comments

@bjorn3
Copy link
Contributor

bjorn3 commented Jul 9, 2024

I tried

diff --git a/src/report.rs b/src/report.rs
index acfce38..b3ab842 100644
--- a/src/report.rs
+++ b/src/report.rs
@@ -45,6 +45,18 @@ pub fn get_test_rules(test: &TestKey, caller: &dyn AbiImpl, callee: &dyn AbiImpl
     //
     // THIS AREA RESERVED FOR VENDORS TO APPLY PATCHES
 
+    if cfg!(target_arch = "aarch64") && test.test == "F32Array" && test.options.convention == CallingConvention::C {
+        result.check = Busted(Run);
+    }
+
+    if cfg!(target_arch = "aarch64") && test.test == "OptionU128" && test.options.convention == CallingConvention::Rust && test.options.repr == LangRepr::C {
+        result.check = Busted(Run);
+    }
+
+    if test.test == "f16" || test.test == "f128" {
+        result.check = Busted(Build);
+    }
+
     // END OF VENDOR RESERVED AREA
     //
     //

but I'm getting results like

OptionU128::conv_rust::repr_c::cgclif_calls_rustc                fixed (test was busted, congrats!) ( 69/71  passed)
  OptionU128::conv_rust::repr_c::cgclif_calls_rustc::val_in                   
  OptionU128::conv_rust::repr_c::cgclif_calls_rustc::ref_in                   
  OptionU128::conv_rust::repr_c::cgclif_calls_rustc::val_out                  failed!
  func val_out's values differed
    values (native-endian hex bytes):
      expect: 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F
      caller: 58 59 5A 5B 5C 5D 5E 5F 10 00 00 00 00 00 00 00
      callee: 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F
    the value was out0.Some.field0: u128
    whose arg was out0: OptionU128
  OptionU128::conv_rust::repr_c::cgclif_calls_rustc::val_in_out               failed!
  func val_in_out's values differed
    values (native-endian hex bytes):
      expect: 80 81 82 83 84 85 86 87 88 89 8A 8B 8C 8D 8E 8F
      caller: 88 89 8A 8B 8C 8D 8E 8F 10 00 00 00 00 00 00 00
      callee: 80 81 82 83 84 85 86 87 88 89 8A 8B 8C 8D 8E 8F
    the value was out0.Some.field0: u128
    whose arg was out0: OptionU128
[...]

It somehow marks the test as fixed despite clearly showing several failed subtests and the test summary shows

32 test sets run - 10 passed, 0 busted, 8 failed, 14 skipped
@Gankra
Copy link
Owner

Gankra commented Jul 9, 2024

For OptionU128 i believe you want Busted(Check) which is the case that involves us printing out bytes. I should make the diagnostic better indicate which phase failed.

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 9, 2024

That works, thanks!

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 9, 2024

I don't understand the failure at https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/9857308482/job/27216123414 I already specified that all f16 and f128 tests should be ignored with

+    if test.test == "f16" || test.test == "f128" {
+        result.check = Random;
+    }

@Gankra
Copy link
Owner

Gankra commented Jul 9, 2024

Hmm yeah not sure what's happening there, it's like the process aborts or something -- you can use result.run = Skip to make them not run at all, for the time being.

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 9, 2024

That seems to have skipped those tests, but now I'm getting effectively the same behavior on another test except there are no errors emitted by the C compiler: https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/9858813686/job/27221057461 And if you download the raw logs you will see more warnings than in the rendered view.

@Gankra
Copy link
Owner

Gankra commented Jul 9, 2024

EmptyEtruct is definitely another test that's expected to make compilers sad...

The raw logs are interesting here;

cd "/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/./build/abi-cafe" && CARGO_ENCODED_RUSTDOCFLAGS="" CARGO_ENCODED_RUSTFLAGS="" RUSTC="/home/runner/.rustup/toolchains/nightly-2024-07-09-x86_64-unknown-linux-gnu/bin/rustc" RUSTDOC="/home/runner/.rustup/toolchains/nightly-2024-07-09-x86_64-unknown-linux-gnu/bin/rustdoc" "/home/runner/.rustup/toolchains/nightly-2024-07-09-x86_64-unknown-linux-gnu/bin/cargo" "run" "--manifest-path" "/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/./build/abi-cafe/Cargo.toml" "--target-dir" "/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/./build/abi_cafe_target" "--locked" "--target" "x86_64-unknown-linux-gnu" "--" "--conventions" "c" "--conventions" "rust" "--pairs" "rustc_calls_cgclif" "--pairs" "cgclif_calls_rustc" "--pairs" "cgclif_calls_cc" "--pairs" "cc_calls_cgclif" "--add-rustc-codegen-backend" "cgclif:/home/runner/work/rustc_codegen_cranelift/rustc_codegen_cranelift/./build/cg_clif/x86_64-unknown-linux-gnu/release/librustc_codegen_cranelift.so"

exited with status ExitStatus(unix_wait_status(132))

What on earth is unix_wait_status(132)

@Gankra
Copy link
Owner

Gankra commented Jul 9, 2024

Assuming this is in fact a linux error code that would suggest it's SIGILL...

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 9, 2024

Do you by any chance use catch_unwind somewhere inside abi-cafe?

@Gankra
Copy link
Owner

Gankra commented Jul 9, 2024

I do not, but we do a lot of tokio async and running FFI code in-process so there's a lot of places for weird panic boundaries.

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

@bjorn3 it's possible the latest main is less prone to hard aborting? unfortunately I don't have a repro of this issue on my own systems.

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 13, 2024

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

Ah cool I can reproduce this on my computer with:

rustup toolchain add nightly
rustup component add rustc-codegen-cranelift-preview --toolchain nightly
rustup override set nightly

abi-cafe --add-rustc-codegen-backend=cranelift:cranelift --toolchains=cranelift

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

Aha it's OptionU128 that illegals

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

Specifically:

cargo run --release -- --add-rustc-codegen-backend=cranelift:cranelift --toolchains=cranelift,rustc --tests=OptionU128 --pairs=cranelift_calls_rustc,rustc_calls_cranelift --reprs=rust --conventions=rust

That is, both of these sigill:

OptionU128::conv_rust::repr_rust::cranelift_calls_rustc 
OptionU128::conv_rust::repr_rust::rustc_calls_cranelift 

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

Ok yes so!

This runs the suite to completion with no aborts:

    if is_rust && test.test == "OptionU128"  {
        result.run = Link;
    }

This sigills:

    if is_rust && test.test == "OptionU128"  {
        result.run = Run;
    }

The only gap between these two is dlopening and running the resulting dylib, so I'm reasonably confident the compiled program is so UBful that it sigills for real.

So for us to improve this specific situation we need #11, which is maybe overdue but also some really nontrivial work to make some kind of i/o protocol for the child to talk to the parent.

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

NB: i recommend just marking this test as "skip" for now

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 13, 2024

Thanks, that worked for Linux! On Windows it seems like I have a bunch of tests that fail with SIGILL too. https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/9921467869

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

Wow ok #11 was easier than I thought, I've got it locally and spitting out stuff like this
image

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

New main should hopefully no longer have chaos aborts?

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 13, 2024

@bjorn3
Copy link
Contributor Author

bjorn3 commented Jul 13, 2024

Almost there. It seems to hang on x86_64-pc-windows-gnu though: https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/9922164254/job/27410913291

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

pc-windows-gnu eh... I definitely don't run that in my CI and don't have that setup locally... hmm...

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

Running out of steam for the day, I would suggest trying to Skip on windows (just based on these being the most likely to fail):

EmptyStruct, EmptyStructInside, f16, f128

And hopefully that produces stable enough compiles to avoid the hangs? I'm considering adding process timeouts but honestly I don't even know if it's hanging in execution or compilation or...

@Gankra
Copy link
Owner

Gankra commented Jul 13, 2024

also FYI, last thing that requires you to actually vendor ABI-cafe instead of cargo installing it:

Goal is for the harness to be able to suggest the entries for you when your tests fail so you can just copy-paste exactly what happened and get CI green before investigating what went wrong.

(Unfortunately won't fix the damn infinite hang...)

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