Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Comments for "https://os.phil-opp.com/testing/" #591

Closed
utterances-bot opened this issue Apr 28, 2019 · 130 comments
Closed

Comments for "https://os.phil-opp.com/testing/" #591

utterances-bot opened this issue Apr 28, 2019 · 130 comments

Comments

@utterances-bot
Copy link

utterances-bot commented Apr 28, 2019

This is a general purpose comment thread for the Testing post.

Copy link

I'm not sure if I did something wrong, but by following along with this guide exactly I was receiving an error about fmt being undeclared after setting up the CompareMessage implementation. I fixed it by adding use core::fmt; and use core::fmt::Write; at the top of the file, and increasing the PANIC_LINE by 2 to compensate. Did I miss a step along the way or is there a missing step in the guide?

@phil-opp
Copy link
Owner

@leggettc18 You did nothing wrong, I forgot to mention these imports. Fixed in 1f97103. Thanks for reporting!

@phil-opp phil-opp changed the title https://os.phil-opp.com/testing/ Comments for "https://os.phil-opp.com/testing/" Apr 28, 2019
@Beidah
Copy link

Beidah commented May 4, 2019

I'm unable to run cargo xtest. Instead I keep keeping the message

error[E0463]: can't find crate for std
|
= note: the x86_64-blog_os-1954185736335484285 target may not be installed

cargo xbuild works fine, though.

@AntoineSebert
Copy link
Contributor

AntoineSebert commented May 4, 2019

Try cargo xtest --target x86_64-blog_os.json.

I have the same problem but couldn't figure out why the file isn't automatically found.

@phil-opp
Copy link
Owner

phil-opp commented May 4, 2019

@AntoineSebert Do you have a build.target key defined in your .cargo/config file?

@Beidah Sounds like forgot a #![no_std] attribute (perhaps the conditional attribute in your lib.rs?). Or you accidentally use a dependency that uses std.

@AntoineSebert
Copy link
Contributor

@phil-opp Yes it's there. The problem seems to have disappread today, and anyway it's not a big deal.

@AntoineSebert
Copy link
Contributor

The recent improvements in the test framework now produce text outputs that are not colored in green, thus making the test result less visible within the terminal.
Is there a way to specify a color for the text when calling print/println macros ?

@phil-opp
Copy link
Owner

phil-opp commented May 7, 2019

Yes, there is! You can use ANSI escape codes for coloring. For example, you can create a type to print something in green:

use core::fmt;

struct Green(&'static str);

impl fmt::Display for Green {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { 
        write!(f, "\x1B[32m")?; // prefix code
        write!(f, "{}", self.0)?;
        write!(f, "\x1B[0m")?; // postfix code
        Ok(())
    }
}

Now you can do serial_println!("{}", Green("[ok]")); at the end of your test.

I opened #603 for adding this to the post.

Copy link

bytesnail commented May 14, 2019

In the process of adding tests, I encountered some strange errors. In this test, I simply created a writer, but failed, and judging from the error message, it seems that it did not execute any tests at all, because I did not see the information printed by test_runner or panic_handler on the console.

截屏-20190514233300-1920x1052

lws@lsw:/document/project/flandre-os/FlandreOS$ cargo xtest test_default_writer
   Compiling flandre_os v0.0.1 (/document/project/flandre-os/FlandreOS)
    Finished dev [unoptimized + debuginfo] target(s) in 0.33s
     Running target/x86_64-flandre_os/debug/deps/flandre_os-d120248f92b70111
Building bootloader
   Compiling bootloader v0.6.0 (/work/tool/rust/cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/bootloader-0.6.0)
    Finished release [optimized + debuginfo] target(s) in 0.72s
Running: `qemu-system-x86_64 -drive format=raw,file=/document/project/flandre-os/FlandreOS/target/x86_64-flandre_os/debug/deps/bootimage-flandre_os-d120248f92b70111.bin -device isa-debug-exit,iobase=0xf4,iosize=0x04 -serial stdio -display none test_default_writer`
qemu-system-x86_64: -display none: drive with bus=0, unit=0 (index=0) exists
error: test failed, to rerun pass '--lib'

Copy link

I did all kinds of printing tests and the printing function seemed to work normally, but when I tried to add some tests to ensure that these functions would not be damaged in future modifications, I got the above errors.

@bjorn3
Copy link
Contributor

bjorn3 commented May 14, 2019

I think passing a test name to cargo xtest doesnt work. Try running just cargo xtest.

@phil-opp
Copy link
Owner

I think passing a test name to cargo xtest doesnt work. Try running just cargo xtest.

Yes, as you see from the "Running: " output, the test_default_writer is directly passed as an argument to QEMU, which does not understand it and exits with an error. Passing the name of a single test is a feature of the default test runner and not available in our bare-metal environment because our kernel does not get any arguments passed when it's started.

What you can do is to run the tests of the library component through cargo xtest --lib and the tests of the binary through cargo xtest --bin blog_os. You can also run an individual integration test through cargo xtest --test test_name.

Copy link

xj42 commented May 22, 2019

Now we can run the test using cargo xtest --test panic_handler. We see that it passes, which means that the reported panic info is correct. If we use a wrong line number in PANIC_LINE or panic with an additional character through panic!("{}x", MESSAGE), we see that the test indeed fails.

I tried adding the "x" as you indicated in the above paragraph. The test did not fail. WIth some more fooling about i see that the fn write_str(&mut self, s: &str) -> fmt::Result {} function is called 4 times. The third time it is called, it sets equals to true. Because there is no else setting false, the test does not work as expected.

I have submitted a pull request #610

@phil-opp
Copy link
Owner

phil-opp commented May 22, 2019

@xj42 Thanks a lot for reporting! I created #611 to fix this.

Copy link

ta5een commented Jun 14, 2019

Hi, thank you for taking your time and effort into making this brilliant blog! I've learnt so much about operating systems and the Rust language in general! Can't wait to see how this blog unfolds and what new things you'll bring.

I just wanted to ask if it is the expected behaviour for serial_print! and serial_println! to only work in test mode? With cargo xtest, the serial print macros work as expected; they print to my host's terminal. However, with cargo xrun, nothing is shown in my host's terminal, nor my QEMU instance. Oddly enough, the system hasn't panicked, so it somehow worked (but the bytes don't show up).

It seems to me that perhaps this was the desired behaviour? If so, is there a way for me to send data to the serial port while the QEMU instance is running as normal?

@phil-opp
Copy link
Owner

@tahscenery Thanks so much for the nice words!

Yes, this is the expected behavior. The reason is that the package.metadata.bootimage.test-args configuration key in your Cargo.toml only applies to cargo xtest and is ignored for cargo xrun. So the -serial stdio argument is only added to Q.EMU in test mode. Without it, QEMU simply ignores the serial output.

To also print the serial output for cargo xrun, you can add a run-args = ["-serial", "stdio"] key to the [package.metadata.bootimage] section in your Cargo.toml. The run-args key is the counterpart to the test-args key: it applies only to cargo xrun, but not to cargo xtest. See the bootimage Readme for more information.

@ta5een
Copy link

ta5een commented Jun 15, 2019

@phil-opp Of course, I knew it was that simple 🤦‍♂️! Thank you so much!

Copy link

Hi, thank you so much for writing this awesome blog. I'm a very fun of it!

To make our test_runner available to executables and integration tests, we don't apply the cfg(test) attribute to it and make it public.

I'm not sure why we need to remove cfg(test) attribute to make it accessible to src/main.rs on test mode. Is src/main.rs on test mode linked to src/lib.rs on non-test mode?

@phil-opp
Copy link
Owner

Is src/main.rs on test mode linked to src/lib.rs on non-test mode?

Yes, exactly. Only the leaf crate is compiled in test mode, dependencies are not. And the lib.rs is treated like a normal dependency.

Copy link

I'm having trouble
I did till Custom Test Frameworks
But keep telling me that error[E0463]: can't find crate for test

I did exactly as said in the tutirial

@phil-opp
Copy link
Owner

@GrayJack Did you add the #![feature(custom_test_frameworks)] and #![test_runner(crate::test_runner)] attributes? If yes, do you have your code available somewhere so that we can take a look?

Copy link

@phil-opp
Copy link
Owner

@GrayJack Thanks! You already have a lib.rs, which we introduce later in the post. Rust tests your main.rs and lib.rs separately, so you need to specify the #![test_runner(crate::test_runner) attribute in both files. Otherwise, the default test runner will be used for your lib.rs, which results in the error you saw.

@GrayJack
Copy link

Thanks, now it's complaining that that there is no #[panic_handler] function on the lib part, do I have to create a panic handler inside lib.rs as well?

@phil-opp
Copy link
Owner

Yes, see the "Create a Library" section of the post:

Since our lib.rs is tested independently of our main.rs, we need to add a _start entry point and a panic handler when the library is compiled in test mode. By using the cfg_attr crate attribute, we conditionally enable the no_main attribute in this case.

Copy link

I keep getting this supremely annoying error when trying to write the panic handler test:

error[E0152]: duplicate lang item found: `panic_impl`.
  --> tests/panic_handler.rs:23:1
   |
23 | / fn panic(info: &PanicInfo) -> ! {
24 | |     check_location(info);
25 | |     check_message(info);
26 | |
...  |
29 | |     loop {}
30 | | }
   | |_^
   |
   = note: first defined in crate `daedalos`.

It seems like the panic handler defined in lib.rs "leaks" into everything that uses the lib, not sure why/how that's happening. Source is on GitHub.

Any ideas @phil-opp

@phil-opp
Copy link
Owner

Great to hear that!

@tomadimitrie
Copy link

Hi, thanks for the wonderful tutorials! I have a problem. When I run cargo test the project is compiled 3 times - first 2 with "Running 0 tests" and only the last one actually running the tests. What should I do?

@phil-opp
Copy link
Owner

@tomadimitrie The reason for that is that the library, binary, and integration tests are tested separately. So there are indeed multiple compilations. If your binary, for example, contains no tests, you see the "running 0 tests" output. This is expected and also the way that the Rust test framework works for normal projects.

If you want to run tests only for one component, you can use cargo test --lib (run only tests of your library), cargo test --bin blog_os (run only the test of the binary named blog_os), or cargo test --test basic_boot (run only the basic_boot integration test).

@tomadimitrie
Copy link

@tomadimitrie The reason for that is that the library, binary, and integration tests are tested separately. So there are indeed multiple compilations. If your binary, for example, contains no tests, you see the "running 0 tests" output. This is expected and also the way that the Rust test framework works for normal projects.

If you want to run tests only for one component, you can use cargo test --lib (run only tests of your library), cargo test --bin blog_os (run only the test of the binary named blog_os), or cargo test --test basic_boot (run only the basic_boot integration test).

Oh, I understand now. Thank you so much for your help and for your tutorials!

Copy link

After making the required changes to the src/main as suggested in Custom Test section, cargo test throws error as follow

 cargo test
   Compiling compiler_builtins v0.1.35
   Compiling core v0.0.0 (/home/jaskeerat/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core)
   Compiling bootloader v0.9.11
   Compiling rustc-std-workspace-core v1.99.0 (/home/jaskeerat/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/rustc-std-workspace-core)
   Compiling spin v0.5.2
   Compiling volatile v0.2.7
   Compiling lazy_static v1.4.0
   Compiling os v0.1.0 (/home/jaskeerat/Projects/rust/OS)
    Finished test [unoptimized + debuginfo] target(s) in 25.32s
     Running target/x86_64-blog_os/debug/deps/os-bf10a39171b61c6d
error: test failed, to rerun pass '--bin os'

Caused by:
  process didn't exit successfully: `/home/jaskeerat/Projects/rust/OS/target/x86_64-blog_os/debug/deps/os-bf10a39171b61c6d` (signal: 11, SIGSEGV: invalid memory reference)

I did follow the note and removed the panic = abort from the Cargo.toml file.

I am running it on ubuntu 18.04 64bit. here are the other environment details

cargo --version
cargo 1.49.0-nightly (12db56cde 2020-10-14)

rustc --version
rustc 1.49.0-nightly (dd7fc54eb 2020-10-15)

uname -a
Linux jaskeerat-HP-Notebook 5.4.0-48-generic #52~18.04.1-Ubuntu SMP Thu Sep 10 12:50:22 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

The weird thing is that the cargo build and cargo bootimage passes without any such error.

@bjorn3
Copy link
Contributor

bjorn3 commented Oct 16, 2020

Did you add

[build]
target = "x86_64-blog_os.json"

and

[target.'cfg(target_os = "none")']
runner = "bootimage runner"

to .cargo/config.toml?

@ShardulNalegave
Copy link

Hi! Thanks for the great tutorials! I have encountered one problem with bootimage crate but I thought it might be a problem from my side, so thought that it would be better to ask here first.
As told in the tutorial, in the .cargo/config.toml file we customize bootimage under [package.metadata.bootimage]. But I am having a problem doing that. I did all the steps and it ran fine, except one thing. I saw that bootimage ran qemu without the options I had told it to pass under the test-args array.
What should I do to solve this?

My .cargo/config.toml file =>

[target.'cfg(target_os = "none")']
runner = "bootimage runner"

[build]
target = "../x86_64-catalyst.json"

[unstable]
build-std = ["core", "compiler_builtins"]
build-std-features = ["compiler-builtins-mem"]

[package.metadata.bootimage]
test-args = [
    "-serial", "stdio",
]

@phil-opp
Copy link
Owner

@ShardulNalegave See https://github.com/rust-osdev/bootimage#configuration for all configuration options of bootimage. The test-args values are only used when running cargo test, so maybe that's the problem (they are not passed for cargo run).

@ShardulNalegave
Copy link

@phil-opp That is the problem! cargo runruns perfectly fine but the test-args are not passed to qemu when I run cargo test. I have already checked all the configuration options to check if there was any kind of spelling error, etc but it seems that my config is correct.

@phil-opp
Copy link
Owner

Do you have your code online somewhere so that I can take a look?

@ShardulNalegave
Copy link

@phil-opp Yes, the code is at https://github.com/ShardulNalegave/catalyst-os
Its probably not well written 😅

@phil-opp
Copy link
Owner

The [package.metadata.bootimage] should be in your Cargo.toml, not your .cargo/config.toml. After changing this, the test command passes the argument as expected

> cargo test --target ../x86_64-catalyst.json \
  -Z build-std="core,compiler_builtins" \
  -Z build-std-features="compiler-builtins-mem"

Running: `qemu-system-x86_64 -drive format=raw,file=/[…]/catalyst-os/kernel/target/x86_64-catalyst/debug/deps/bootimage-catalyst-8eb501c886f2e973.bin -no-reboot -serial stdio`

@ShardulNalegave
Copy link

@phil-opp Oh! I don't know why I put it there. Anyways, thanks a lot! 😅

Copy link

iwahbe commented Nov 20, 2020

Thanks for the blog, its great. I'm following along on my computer, and when I run integration tests, I get this:

❯ RUST_BACKTRACE=full cargo test Updating crates.io index warning: Patch `adler v0.2.3 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/adler)` was not used in the crate graph. Patch `gimli v0.22.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/gimli)` was not used in the crate graph. Patch `addr2line v0.13.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/addr2line)` was not used in the crate graph. Patch `object v0.20.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/object)` was not used in the crate graph. Patch `miniz_oxide v0.4.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/miniz_oxide)` was not used in the crate graph. Check that the patched package version and available features are compatible with the dependency requirements. If the patch has a different version from what is locked in the Cargo.lock file, run `cargo update` to use the new version. This may also occur with an optional dependency that is not enabled. Compiling blog_os v0.1.0 (/Users/ianwahbe/Projects/BlogOS) warning: unused import: `blog_os::println` --> src/main.rs:7:5 | 7 | use blog_os::println; | ^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

warning: 1 warning emitted

Finished test [unoptimized + debuginfo] target(s) in 2.67s
 Running target/x86_64-blog_os/debug/deps/blog_os-5c53e8c48a10e9ce

Building bootloader
Updating crates.io index
warning: Patch adler v0.2.3 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/adler) was not used in the crate graph.
Patch gimli v0.22.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/gimli) was not used in the crate graph.
Patch addr2line v0.13.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/addr2line) was not used in the crate graph.
Patch object v0.20.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/object) was not used in the crate graph.
Patch miniz_oxide v0.4.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/miniz_oxide) was not used in the crate graph.
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
what is locked in the Cargo.lock file, run cargo update to use the new
version. This may also occur with an optional dependency that is not enabled.
Compiling bootloader v0.9.11 (/Users/ianwahbe/.cargo/registry/src/github.com-1ecc6299db9ec823/bootloader-0.9.11)
Finished release [optimized + debuginfo] target(s) in 1.47s
Running: qemu-system-x86_64 -drive format=raw,file=/Users/ianwahbe/Projects/BlogOS/target/x86_64-blog_os/debug/deps/bootimage-blog_os-5c53e8c48a10e9ce.bin -no-reboot -device isa-debug-exit,iobase=0xf4,iosize=0x04 -serial stdio -display none
Running 4 tests
blog_os::vga_buffer::test_println_output... [ok]
blog_os::vga_buffer::test_println_many... [ok]
blog_os::simple_math... [ok]
blog_os::hard_math... [ok]
Running target/x86_64-blog_os/debug/deps/blog_os-70947e7b0d437fea
Building bootloader
Updating crates.io index
warning: Patch adler v0.2.3 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/adler) was not used in the crate graph.
Patch gimli v0.22.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/gimli) was not used in the crate graph.
Patch addr2line v0.13.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/addr2line) was not used in the crate graph.
Patch object v0.20.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/object) was not used in the crate graph.
Patch miniz_oxide v0.4.0 (/Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/vendor/miniz_oxide) was not used in the crate graph.
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
what is locked in the Cargo.lock file, run cargo update to use the new
version. This may also occur with an optional dependency that is not enabled.
Compiling bootloader v0.9.11 (/Users/ianwahbe/.cargo/registry/src/github.com-1ecc6299db9ec823/bootloader-0.9.11)
error: failed to run custom build command for bootloader v0.9.11 (/Users/ianwahbe/.cargo/registry/src/github.com-1ecc6299db9ec823/bootloader-0.9.11)

Caused by:
process didn't exit successfully: /Users/ianwahbe/Projects/BlogOS/target/bootimage/bootloader/release/build/bootloader-6c6b9b5606b9de74/build-script-build (exit code: 101)
--- stderr
thread 'main' panicked at 'Kernel executable has an empty text section. Perhaps the entry point was set incorrectly?

Kernel executable at /Users/ianwahbe/Projects/BlogOS/target/x86_64-blog_os/debug/deps/blog_os-70947e7b0d437fea
', build.rs:159:9
stack backtrace:
0: 0x101d24804 - std::backtrace_rs::backtrace::libunwind::trace::h0c0ed6308c83ebb2
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/../../backtrace/src/backtrace/libunwind.rs:100:5
1: 0x101d24804 - std::backtrace_rs::backtrace::trace_unsynchronized::h9b58f9e61eb269de
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
2: 0x101d24804 - std::sys_common::backtrace::_print_fmt::h491cef471f8e6976
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/sys_common/backtrace.rs:67:5
3: 0x101d24804 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hdd3dec39c1a33910
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/sys_common/backtrace.rs:46:22
4: 0x101d44be0 - core::fmt::write::h0ce880d33cd2a300
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/core/src/fmt/mod.rs:1078:17
5: 0x101d211c6 - std::io::Write::write_fmt::h79e61fa9a8977466
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/io/mod.rs:1519:15
6: 0x101d26549 - std::sys_common::backtrace::_print::h675afe66127002d6
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/sys_common/backtrace.rs:49:5
7: 0x101d26549 - std::sys_common::backtrace::print::h245c22f8642ebc7d
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/sys_common/backtrace.rs:36:9
8: 0x101d26549 - std::panicking::default_hook::{{closure}}::h9ce6a7ef98d13a43
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:208:50
9: 0x101d260d0 - std::panicking::default_hook::h5ffdc5dc799b78b2
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:225:9
10: 0x101d26bcb - std::panicking::rust_panic_with_hook::h67a114d545465c5e
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:591:17
11: 0x101d266f5 - std::panicking::begin_panic_handler::{{closure}}::hfe4c5262b6866715
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:497:13
12: 0x101d24ce8 - std::sys_common::backtrace::__rust_end_short_backtrace::hfa6dbb55bdbe072e
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/sys_common/backtrace.rs:141:18
13: 0x101d2665a - rust_begin_unwind
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:493:5
14: 0x101d4b71b - std::panicking::begin_panic_fmt::hf99ae7cf8fee4c57
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:435:5
15: 0x101c9494f - build_script_build::main::hc2166eb2045d0f39
at /Users/ianwahbe/.cargo/registry/src/github.com-1ecc6299db9ec823/bootloader-0.9.11/build.rs:159:9
16: 0x101c9dfae - core::ops::function::FnOnce::call_once::he3ea39856812dfa9
at /Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
17: 0x101c9d851 - std::sys_common::backtrace::__rust_begin_short_backtrace::h05714fb9dea9733f
at /Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
18: 0x101c98e04 - std::rt::lang_start::{{closure}}::h66e04c4fe0c63cf1
at /Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
19: 0x101d26f44 - core::ops::function::impls::<impl core::ops::function::FnOnce for &F>::call_once::he8afe04e4eaa1d4d
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/core/src/ops/function.rs:259:13
20: 0x101d26f44 - std::panicking::try::do_call::hcc26559fc13534e5
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:379:40
21: 0x101d26f44 - std::panicking::try::h0758aa68596e43be
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panicking.rs:343:19
22: 0x101d26f44 - std::panic::catch_unwind::ha3b49e3979eb6aba
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/panic.rs:396:14
23: 0x101d26f44 - std::rt::lang_start_internal::he9bad79e3373fa3d
at /rustc/fe982319aa0aa5bbfc2795791a753832292bd2ba/library/std/src/rt.rs:51:25
24: 0x101c98de1 - std::rt::lang_start::hef59f713b645d732
at /Users/ianwahbe/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:65:5
25: 0x101c96742 - _main
Error: Bootloader build failed.
Stderr:

Stack backtrace:
0: std::backtrace::Backtrace::create
1: std::backtrace::Backtrace::capture
2: bootimage::main
3: std::sys_common::backtrace::__rust_begin_short_backtrace
4: std::rt::lang_start::{{closure}}
5: std::rt::lang_start_internal
6: _main
error: test failed, to rerun pass '--bin blog_os'

It looks like my tests work, then cargo rebuilds the boot loader, which crashes. Any ideas?
I'm using rustup 1.22.1 (b01adbbc3 2020-07-08) and cargo 1.49.0-nightly (2af662e22 2020-11-12) running on a Mac.

@phil-opp
Copy link
Owner

@iwahbe Cargo runs the tests for each compilation unit of your crate separately. This means that it first runs the tests for your library and then for your binary. The test output you're seeing is from the library tests, which seem to work. Testing the binary, however, throws the error. The error message indicates the problem:

Kernel executable has an empty text section. Perhaps the entry point was set incorrectly?

So the test executable for your main.rs binary does not contain any code (i.e. it has an emty .text section). The common cause for this is that the linker didn't find an entry point function, so it threw all code away. Ensure that you have a _start function or entry_point macro in your binary that is also enabled when compiling the tests.

If that doesn't fix it, I can take a look at your code if you upload it somewhere.

Copy link

iwahbe commented Nov 20, 2020

@phil-opp your right, that fixed it. I thought that running cargo test would use lib.rs's _start function. Thanks for the incredible response time. Feel free to ignore this, but do you have any idea what the patch warnings are?

@phil-opp
Copy link
Owner

@iwahbe I think the patch warnings are a bug in cargo itself. See #885 for more info.

@YJDoc2
Copy link

YJDoc2 commented Jan 17, 2021

Hey, Great blog, learning so much from this, and really like the writing style and lots of external reference links !

After implementing the should_panic style tests, I saw the note that it cannot execute more than one test, and then tried to implement something that can work. Then as I was reading through the comments I found
#591 (comment) with @CAD97 and @bjorn3 which proposed something similar and referenced the stack issue, so I modified my code to use resetting of SP so that stack overflow does not happen.

Note, that this probably goes against all Rust conventions, and uses asm!, which, I read somewhere, is never going to be stabilized, but I wanted to try something, and someone might find a better approach from this 😅

This is the code modified from Tests that should panic section, (before No harness tests) :

  • First we impose static lifetime on tests slice , and create a mutable static option:
static mut REMAINING_TESTS:Option<&'static [&'static dyn Fn()]> = None;
...
pub fn test_runner(tests: &'static [&'static dyn Fn()]) {
...
}
  • To overcome SP issue, we make another mut static:
static mut SP:u64 = 0;
  • Then we allow asm feature, and use an unsafe block:
#![feature(asm)]
...
pub fn test_runner(tests: &'static [&'static dyn Fn()]) {
    serial_println!("Running {} tests", tests.len());
    unsafe{
        REMAINING_TESTS = Some(tests);  // store tests in static, will always be valid reference 
                                       // as lifetime is static, and running tests is the only thing we do
        asm!("mov {}, rsp",sym SP);  // save stack pointer in global variable
        test_runner_helper();     // call the function which will actually run the code
    }
}
  • The test_runner_helper, itself, is an unsafe function:
pub unsafe fn test_runner_helper()->! {    
    match REMAINING_TESTS{
        Some(t) =>{
            if t.len() == 1{ // last test
                REMAINING_TESTS = None;
            }else{ // more tests remaining
                REMAINING_TESTS = Some(&t[1..]); // shift starting of REMAINING_TESTS to next element
            }
            t[0](); // call the test
            exit_qemu(QemuExitCode::Failed); // as test should panic, reaching here is failure
            loop{}
        },
        None => { // no tests remaining
            exit_qemu(QemuExitCode::Success);
            loop {} // just to satisfy diverging function
        },
    }
}
  • Modify the panic handler:
#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
    serial_println!("[ok]");
    // changes
    unsafe{
    // First move the Stack Pointer to the value before we started tests.
    // As test functions are meant to panic at some point, only which, will lead here,
    // we can reset stack pointer, as the panicking test would not need the stack anymore
   // Then we call the test_runner_helper again
        asm!(
            "mov rsp, {}","call {}",
            sym SP,sym test_runner_helper,
        )
    }
    exit_qemu(QemuExitCode::Failed); // This is theoretically unreachable, I think... 
    loop {} // for divergent function
}

I checked this using five test functions, all five ran; and SP as well as BP was same for each function testing (tested by printing the value before invoking test function in test_runner_helper).
As said in #591 (comment), this is probably not simple enough for blog, but might help someone who wants to try, or someone might find a better way from this.
Anyway, Thanks for the amazing blog!

@phil-opp
Copy link
Owner

@YJDoc2 Thanks for your comment! I'm glad that you like the blog :).

Nice solution, even if it's probably a bit too hacky to be included in the post. Some small suggestions to make this a bit safer:

  • Instead of using a static mut, you should be able to use a spinlock-protected static. To avoid a deadlock, you would need to keep the critical section short, e.g. by splitting off the first element through slice::split_at_mut and then immediately releasing the lock.
  • The test_runner_helper function should use the C calling convention (extern "C" fn), otherwise the call from assembly is dangerous since Rust's calling convention might change in the future.
  • To ensure that the stack is saved immediately before the test_runner_helper function, it would make sense to call the function from inline asm in test_runner too. This way, you can also be sure that the stack pointer is the same for each invocation.

Note, that this probably goes against all Rust conventions, and uses asm!, which, I read somewhere, is never going to be stabilized, but I wanted to try something, and someone might find a better approach from this 😅

Using inline assembly in some cases is fine, you just have to be careful. What you read was probably about the old LLVM-style inline assembly macro, which is now named llvm_asm!. The new asm! macro was specifically designed to make stabilization possible, so it should be possible to also use it on stable some day.

@YJDoc2
Copy link

YJDoc2 commented Jan 17, 2021

Hey, Thanks for the quick reply :)
yes, it is quite a bit hacky 😁 I'll keep the points in mind, thanks for the improvements :)

Copy link

weihsiu commented Feb 26, 2021

i got multiple should_panic tests working without using the SP register hack. but my rust-fu isn't great enough to get rid of the "static mut" bit. please have a look:

static mut TESTS: &'static [&'static dyn Testable] = &[];

pub fn test_runner(tests: &'static [&'static dyn Testable]) {
    serial_println!("Running {} tests", tests.len());
    unsafe {
        TESTS = tests;
        test_runner_helper();
    }
}

unsafe fn test_runner_helper() -> ! {
    match TESTS.split_first() {
        None => exit_qemu(QemuExitCode::Success),
        Some((test, tests)) => {
            TESTS = tests;
            test.run();
            serial_println!("[test did not panic]");
            exit_qemu(QemuExitCode::Failed);
        }
    }
    loop {}
}

#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
    serial_println!("[ok]");
    unsafe {
        test_runner_helper();
    }
}

#[test_case]
fn should_fail1() {
    assert_eq!(0, 1);
}

#[test_case]
fn should_fail2() {
    assert_eq!(0, 1);
}

@apogeeoak
Copy link

Building off the solutions of @CAD97, @YJDoc2, and @weihsiu I was able to get multiple should panic tests to technically work. However, the stack does not seem to be properly aligned afterwards. The stack pointer is saved in the test runner function then reset before each test invocation in the panic handler. However, when emulating the stack pointer reset functionality on Linux a segmentation fault is thrown after completing the test run. All the tests are run successfully, without a stack overflow, but the segmentation fault occurs when attempting to return from the test run. When run on the custom os target execution never continues beyond the test run so a misaligned stack is never discovered.

tests/should_panic.rs

#![no_std]
#![no_main]
#![feature(custom_test_frameworks)]
#![test_runner(os::test::should_panic::runner)]
#![reexport_test_harness_main = "test_main"]

use core::panic::PanicInfo;

// Entry point.
#[no_mangle]
pub extern "C" fn _start() -> ! {
    test_main();
    loop {}
}

// Panic handler.
#[panic_handler]
fn panic(info: &PanicInfo) -> ! { os::test::should_panic::panic(info); }

// Tests.
#[cfg(test)]
mod tests {
    #[test_case]
    pub fn one() {
        let actual = 1;
        assert_eq!(0, actual);
    }
    ...
}

src/library/test/should_panic.rs

use super::qemu;
use crate::serial_print;
use crate::serial_println;
use core::panic::PanicInfo;
use core::slice::Iter;
use spin::Mutex;
use spin::Once;

pub trait Testable: Sync {
    fn run(&self);
}

impl<T: Fn() + Sync> Testable for T {
    fn run(&self) {
        serial_print!("{} ... ", core::any::type_name::<T>());
        self();
        serial_println!("failed");
        qemu::exit(qemu::ExitCode::Failure);
    }
}

// Test runner.
pub fn runner(tests: &'static [&dyn Testable]) {
    serial_println!("Running {} test(s).", tests.len());

    // Save tests and stack pointer.
    let rsp: u64;
    unsafe {
        asm!("mov {}, rsp", out(reg) rsp);
    }
    Tests::init(tests, rsp);

    run_next();
}

// Test mode panic handler.
pub fn panic(_: &PanicInfo) -> ! {
    serial_println!("ok");

    // Reset stack pointer.
    if let Some(tests) = Tests::get() {
        let rsp = tests.stack_pointer();
        unsafe { asm!("mov rsp, {}", in(reg) rsp) }
    }

    run_next();
    loop {}
}

fn run_next() {
    let next = Tests::get().and_then(|mut i| i.next());
    match next {
        // All test ran successfully.
        None => qemu::exit(qemu::ExitCode::Success),
        // Run next test.
        Some(&test) => test.run(),
    }
}

struct TestsIter {
    iter: Iter<'static, &'static dyn Testable>,
    stack_pointer: u64,
}

impl TestsIter {
    fn new(items: &'static [&dyn Testable], stack_pointer: u64) -> TestsIter {
        let iter = items.iter();
        TestsIter { iter, stack_pointer }
    }
}

struct Tests {
    inner: &'static Mutex<TestsIter>,
}

impl Iterator for Tests {
    type Item = &'static &'static dyn Testable;

    fn next(&mut self) -> Option<Self::Item> { self.inner.lock().iter.next() }
}

impl Tests {
    fn instance() -> &'static Once<Mutex<TestsIter>> {
        static INSTANCE: Once<Mutex<TestsIter>> = Once::new();
        &INSTANCE
    }

    pub fn init(items: &'static [&dyn Testable], stack_pointer: u64) -> Tests {
        let inner = Tests::instance().call_once(|| Mutex::new(TestsIter::new(items, stack_pointer)));
        Tests { inner }
    }

    pub fn get() -> Option<Tests> { Tests::instance().get().map(|inner| Tests { inner }) }

    pub fn stack_pointer(&self) -> u64 { self.inner.lock().stack_pointer }
}

Full code on GitHub.

@apogeeoak
Copy link

Changing the os::test::should_panic::panic function from a diverging function to a returning function allows the function to return which allows Rust to discover the misaligned stack. With this configuration all the test still pass but an error is thrown at the end because of the segmentation fault. This does not solve the problem of aligning the stack but just further illustrates the problem. The changes are shown below:

tests/should_panic.rs

#[panic_handler]
fn panic(info: &PanicInfo) -> ! {
    os::test::should_panic::panic(info); // Causes segmentation fault on return.
    loop {}
}

src/library/test/should_panic.rs

pub fn panic(_: &PanicInfo) {
    serial_println!("ok");

    // Reset stack pointer.
    if let Some(tests) = Tests::get() {
        let rsp = tests.stack_pointer();
        unsafe { asm!("mov rsp, {}", in(reg) rsp) }
    }

    run_next();
}

fn run_next() {
    let next = Tests::get().and_then(|mut i| i.next());
    match next {
        // All test ran successfully. Return to demonstrate segmentation fault.
        None => (),
        // Run next test.
        Some(&test) => test.run(),
    }
}

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 13, 2021

It is not allowed to change the stack pointer inside asm!() without restoring it before exiting the asm!() block. LLVM assumes that the stack pointer remains constant across asm!() blocks and could for example spill arbitrary variables and then expect that the original value is read back after the asm!() block. In addition putting asm!() blocks in a separate function is a valid implementation strategy. It would be a better idea to write a full setjmp/longjmp implementation. Do note that setjmp needs to be annotated with #[returns_twice]. (might have gotten the name slightly wrong)

@apogeeoak
Copy link

Restoring the stack pointer at the end of the asm!() block still causes a segmentation fault. The only correct implementation seems to be setjmp/longjmp.

@byoboo
Copy link

byoboo commented Mar 24, 2021

Thanks for the great blog! I ran into a strange issue where it doesn't appear my tests are running once, I complete this part of the blog: https://os.phil-opp.com/testing/#custom-test-frameworks The Hello line prints but it doesn't look like the tests run when I run "cargo test"

Any thoughts? My code is posted here: https://github.com/byoboo/tiny_os

@byoboo
Copy link

byoboo commented Mar 26, 2021

Thanks for the great blog! I ran into a strange issue where it doesn't appear my tests are running once, I complete this part of the blog: https://os.phil-opp.com/testing/#custom-test-frameworks The Hello line prints but it doesn't look like the tests run when I run "cargo test"

Any thoughts? My code is posted here: https://github.com/byoboo/tiny_os

I figured out my issue above, in /cargo.toml I had the following below [package.metadata.bootimage]

[package.metadata.bootimage]
run-command = ["qemu-system-x86_64", "-drive", "format=raw,file=target/x86_64-tiny_os/debug/bootimage-tiny_os.bin"]
run-args = ["-L", "C:/Program Files/qemu"]
test-args = [
    "-L", "C:/Program Files/qemu", 
   "-device", "isa-debug-exit,iobase=0xf4,iosize=0x04",
   "-serial", "stdio"
]
test-success-exit-code = 33 # (0x10 << =1) | 1

I commented out the "run-command" line and everything worked like a charm.

Repository owner locked and limited conversation to collaborators Jun 12, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests