Gankra / abi-cafe

Pair your compilers up at The ABI Cafe!
https://faultlore.com/abi-cafe/book/
213 stars 14 forks source link

How to mark test as busted? #65

Open bjorn3 opened 3 weeks ago

bjorn3 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

That works, thanks!

bjorn3 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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

bjorn3 commented 3 weeks ago

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

Gankra commented 3 weeks ago

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 commented 3 weeks ago

@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 commented 3 weeks ago

Crashes even earlier: https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/9920450664/job/27407348652

Gankra commented 3 weeks ago

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 commented 3 weeks ago

Aha it's OptionU128 that illegals

Gankra commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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

bjorn3 commented 3 weeks ago

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 commented 3 weeks ago

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

Gankra commented 3 weeks ago
Gankra commented 3 weeks ago

New main should hopefully no longer have chaos aborts?

bjorn3 commented 3 weeks ago

Indeed, it now reports the test failures: https://github.com/rust-lang/rustc_codegen_cranelift/actions/runs/9922087277/job/27410745118

bjorn3 commented 3 weeks ago

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 commented 3 weeks ago

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

Gankra commented 3 weeks ago

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 commented 3 weeks ago

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