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

std tests added #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

lordshashank
Copy link

@lordshashank
Copy link
Author

@antoyo interestingly all std tests passed.
However I got few outputs like this

thread 'io::buffered::t' panicked at library/std/src/io/buffered/tests.rs:497:13:
explicit panic
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic_display
   3: core::panicking::panic_explicit
   4: <std::io::buffered::tests::panic_in_write_doesnt_flush_in_drop::PanicWriter as std::io::Write>::write::panic_cold_explicit
   5: std::io::buffered::bufwriter::BufWriter<W>::flush_buf
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

As tests passed, I guess panic was expected, let me know if thats not the case.
Also let me know if any other changes are needed.

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

I have reasons to believe that this doesn't actually use the GCC codegen for the std tests.
To test, I added a panic at the beginning of the init() method and all the tests still pass (while the UI tests would fail with such a change).

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

I got a patch to attempts to run the std with the GCC codegen:

From fdb72558a19e2bc3b21acd569c9f6f6e9af8dc56 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <[email protected]>
Date: Wed, 27 Mar 2024 12:22:52 -0400
Subject: [PATCH] Attempt to run std tests with cg_gcc

---
 build_system/src/test.rs | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/build_system/src/test.rs b/build_system/src/test.rs
index 37cfa1a08ff..60a8f5b7aec 100644
--- a/build_system/src/test.rs
+++ b/build_system/src/test.rs
@@ -985,6 +985,13 @@ where

     env.get_mut("RUSTFLAGS").unwrap().clear();

+    let rustflags = format!(
+        "{test_flags} -Zcodegen-backend={backend} -Clto=off",
+        test_flags = env.get("TEST_FLAGS").unwrap_or(&String::new()),
+        backend = args.config_info.cg_backend_path,
+    );
+    env.insert("RUSTFLAGS".to_string(), rustflags);
+
     run_command_with_output_and_env(
         &[
             &"./x.py",
@@ -992,7 +999,8 @@ where
             &"--run",
             &"always",
             &"--stage",
-            &"0",
+            &"0", // TODO: try --stage 1 while setting codegen-backend in config.toml.
+            &"-v",
             &test_type,
             &"--rustc-args",
             &rustc_args,
@@ -1005,9 +1013,11 @@ where

 fn test_rustc(env: &Env, args: &TestArg) -> Result<(), String> {
     let std_tests = test_rustc_inner(env, args, |_| Ok(false), "library/std");
-    let run_make_tests = test_rustc_inner(env, args, |_| Ok(false), "tests/run-make");
-    let ui_tests = test_rustc_inner(env, args, |_| Ok(false), "tests/ui");
-    std_tests.and(run_make_tests).and(ui_tests)
+    std_tests
+    //let run_make_tests = test_rustc_inner(env, args, |_| Ok(false), "tests/run-make");
+    //let ui_tests = test_rustc_inner(env, args, |_| Ok(false), "tests/ui");
+    //ui_tests
+    //std_tests.and(run_make_tests).and(ui_tests)
 }

 fn test_failing_rustc(env: &Env, args: &TestArg) -> Result<(), String> {
--
2.44.0

But it fails with a bunch of errors like this:

error: expected expression, found `)`
 --> /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/build/rust/src/tools/build_helper/src/metrics.rs:3:10
  |
3 | #[derive(Serialize, Deserialize)]
  |          ^^^^^^^^^ expected expression
  |
  = note: this error originates in the derive macro `Serialize` (in Nightly builds, run with -Z macro-backtrace for more info)

error: proc-macro derive produced unparsable tokens
 --> /home/bouanto/Ordinateur/Programmation/Rust/Projets/rustc_codegen_gcc/build/rust/src/tools/build_helper/src/metrics.rs:3:10
  |
3 | #[derive(Serialize, Deserialize)]
  |

I have no idea how to debug this.

You could try debugging this if you have ideas.
Otherwise, you could revert this patch and try with --stage 1: this will possibly require setting the codegen here to ["gcc"].

I hope it helps.

@lordshashank
Copy link
Author

@antoyo would you like all the tests (ui and run-make) to run with stage 1 or just std ones?

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

I would say only when it's necessary, so only std likely.
I guess the most important right now is just having the std tests use the GCC codegen.
After that, we can think about this stuff.

@lordshashank
Copy link
Author

with stage 1 and "gcc" flag as well , it is panicking with weird errors like this:

thread 'opt cgu.0' panicked at src/lib.rs:348:9:
not implemented
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: <rustc_codegen_gcc::GccCodegenBackend as rustc_codegen_ssa::traits::write::WriteBackendMethods>::prepare_thin
   4: rustc_codegen_ssa::back::write::execute_optimize_work_item::<rustc_codegen_gcc::GccCodegenBackend>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: please attach the file at `/home/wsl-ubuntu/blockchain/rust/rustc_codegen_gcc/build/rust/rustc-ice-2024-03-27T19_48_14-13743.txt` to your bug report

note: compiler flags: --crate-type lib -C opt-level=3 -C embed-bitcode=no -Z unstable-options -C symbol-mangling-version=legacy -Z unstable-options -Z unstable-options -Z macro-backtrace -C split-debuginfo=off -C prefer-dynamic -Z inline-mir -C link-args=-Wl,-z,origin -C link-args=-Wl,-rpath,$ORIGIN/../lib -C embed-bitcode=yes -Z crate-attr=doc(html_root_url="https://doc.rust-lang.org/nightly/") -Z binary-dep-depinfo -Z force-unstable-if-unmarked

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

Yes, this is because Thin LTO is not supported.
Please add -Clto=off like in the patch above, possibly in rustc_args directly.

@lordshashank
Copy link
Author

Tried that and few things, still same. Could you explain why it didn't worked in current state as was for test suits?

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

It seems the std tests were ran differently, directly using cargo, so bypassing the arguments we sent them.
It seems like setting RUSTFLAGS is one way to use the GCC codegen, but I got stuck with undebuggable proc-macro stuff at some point.

@lordshashank
Copy link
Author

I am not even reaching there with stage 1 (still getting same errs as above) , not sure where's the problem.

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

Let me try with --stage 1.

@antoyo
Copy link
Owner

antoyo commented Mar 27, 2024

I got the same Thin LTO error, but I was able to continue by adding this code before running x.py:

    let rustflags = "-Clto=off".to_string();
    env.insert("RUSTFLAGS".to_string(), rustflags);

At this point, there's this panic to debug:

thread 'rustc' panicked at src/intrinsic/mod.rs:1058:9:
not implemented

I can post a full patch if you cannot reach this point.

@lordshashank
Copy link
Author

Ah!, am stuck here. From what I understood, compiler is trying to use an intrinsic function that is not yet implemented in the rustc_codegen_gcc backend. Am not able to reach the specific functionality causing this, got nothing much from full backtrace as well.
@antoyo your views on this?

@antoyo
Copy link
Owner

antoyo commented Apr 5, 2024

This is reaching this line.
Strangely, the following line is not the the Cargo.toml in build/rust/compiler/rustc_codegen_gcc:

default = ["master"]

and adding it allows to get past this panic.

The line is even upstream so I guess we clone an earlier version for the tests?
Thus, I guess you need to check which version we clone in the tests and that might help you.

Ideally, we would want to run the tests on the current version, not some old version, but I guess an old version is a good start.

I hope this helps you to continue debugging this.

@lordshashank
Copy link
Author

lordshashank commented Apr 5, 2024

ok so we clone and checkout to the commit as per rustc version which currently does not have default = ["master"] in cargo.toml, but it has been included in more recent commits. imo we should wait for toolchain update, tests could run after that.

@antoyo
Copy link
Owner

antoyo commented Apr 5, 2024

It seems like it was added upstream on March 5th, unless I don't understand something, so this should already be OK.

@lordshashank
Copy link
Author

lordshashank commented Apr 5, 2024

ya it was added then but mybe it was not on release, we use rustc -vV to get commit hash and it shows the following info

rustc 1.78.0-nightly (2d24fe591 2024-03-09)
binary: rustc
commit-hash: 2d24fe591f30386d6d5fc2bb941c78d7266bf10f
commit-date: 2024-03-09
host: x86_64-unknown-linux-gnu
release: 1.78.0-nightly
LLVM version: 18.1.0

commit 2d24fe591f30386d6d5fc2bb941c78d7266bf10f doesn't have that patch.

@antoyo
Copy link
Owner

antoyo commented Apr 5, 2024

Ok, I'll attempt to do the sync quickly, then.

@lordshashank
Copy link
Author

Let me know once its done.

@antoyo
Copy link
Owner

antoyo commented Apr 9, 2024

I will. The sync is harder to do this time because a bug was introduced.

@antoyo
Copy link
Owner

antoyo commented Jul 5, 2024

The sync was finally done, so you can probably continue working on this.
Thanks a lot for your patience!

@antoyo
Copy link
Owner

antoyo commented Jul 17, 2024

It seems that's linking the std twice.
@GuillaumeGomez @bjorn3: Do you have an idea of how we should run the std tests with cg_gcc?

@GuillaumeGomez
Copy link
Collaborator

Normally with the provided sysroot, it should work.

@antoyo
Copy link
Owner

antoyo commented Jul 17, 2024

Normally with the provided sysroot, it should work.

I believe the problem is actually because we provide the sysroot, since this will provide a second std crate.

@GuillaumeGomez
Copy link
Collaborator

Oh, I see.

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 the tests from std in the CI
3 participants