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

Rust 1.67.0 breaks unit tests #26

Closed
robin-nitrokey opened this issue Jan 27, 2023 · 6 comments
Closed

Rust 1.67.0 breaks unit tests #26

robin-nitrokey opened this issue Jan 27, 2023 · 6 comments

Comments

@robin-nitrokey
Copy link
Member

---- fs::tests::remove_dir_all stdout ----
generated PathBuf dir p"\0" using i = 0
creating p"\0"
generated PathBuf dir p"/tmp\0" using i = 4
creating p"/tmp\0"
creating p"/tmp/test\0"
thread 'fs::tests::remove_dir_all' panicked at 'already borrowed: BorrowMutError', src/fs.rs:1218:29
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- fs::tests::todo stdout ----
blocks going in: 6
generated PathBuf dir p"\0" using i = 0
creating p"\0"
generated PathBuf dir p"/tmp\0" using i = 4
creating p"/tmp\0"
creating p"/tmp/test\0"
thread 'fs::tests::todo' panicked at 'already borrowed: BorrowMutError', src/fs.rs:1218:29

---- tests::test_create stdout ----
creating p"/tmp\0"
thread 'tests::test_create' panicked at 'already borrowed: BorrowMutError', src/fs.rs:284:69

---- tests::test_fs_with stdout ----
creating p"/tmp\0"
thread 'tests::test_fs_with' panicked at 'already borrowed: BorrowMutError', src/fs.rs:284:69
robin-nitrokey added a commit to Nitrokey/littlefs2 that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0, possibly related to changes to the
layout of types without a specified representation.
	trussed-dev#26
robin-nitrokey added a commit to Nitrokey/littlefs2 that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0, possibly related to changes to the
layout of types without a specified representation.
	trussed-dev#26
robin-nitrokey added a commit to Nitrokey/littlefs2 that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0, possibly related to changes to the
layout of types without a specified representation.
	trussed-dev#26
robin-nitrokey added a commit that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0, possibly related to changes to the
layout of types without a specified representation.
	#26
robin-nitrokey added a commit to robin-nitrokey/trussed that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0 and littlefs2, see:
	trussed-dev/littlefs2#26
robin-nitrokey added a commit to robin-nitrokey/trussed that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0 and littlefs2, see:
	trussed-dev/littlefs2#26
robin-nitrokey added a commit to robin-nitrokey/trussed that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0 and littlefs2, see:
	trussed-dev/littlefs2#26
robin-nitrokey added a commit to trussed-dev/trussed that referenced this issue Jan 27, 2023
This fixes an issue with Rust 1.67.0 and littlefs2, see:
	trussed-dev/littlefs2#26
@nickray
Copy link
Member

nickray commented Jan 30, 2023

Very odd. Here's a somewhat minimal failing test:

fn minimal() {
    let mut backend = OtherRam::default();
    let mut storage = OtherRamStorage::new(&mut backend);

    Filesystem::format(&mut storage).unwrap();
    Filesystem::mount_and_then(&mut storage, |fs| {
        fs.create_dir(b"/tmp\0".try_into().unwrap()).unwrap();
        fs.available_blocks().ok();
        Ok(())
    }).unwrap();
}

Both create_dir and available_blocks do a self.alloc.borrow_mut(), to pass in .state to ll:lfs_*. Even explicitly dropping these &mut self.alloc.borrow_mut().state doesn't fix.

@robin-nitrokey
Copy link
Member Author

@sosthene-nitrokey suggested that this may be caused by a struct with an undefined representation being used in the FFI causing undefined behavior as this is known to break stuff with 1.67.0.

https://octodon.social/@rust/109759251309886588
https://old.reddit.com/r/rust/comments/10lu5ah/announcing_rust_1670/j5z8rk4/?context=3

But as far as I see, all relevant structs already have #[repr(C)]. Strangely, using RUSTFLAGS="-Z randomize-layout" cargo +nightly test seemed to fix the problem though it should potentially make it worse.

@robin-nitrokey
Copy link
Member Author

I think I found the issue. It looks like this is just the lookahead buffer issue in disguise – the buffer overflow just manifests differently with 1.67.0. Fixed by #24.

robin-nitrokey added a commit to Nitrokey/littlefs2 that referenced this issue Feb 3, 2023
As we fixed the lookahead buffer overflow, we can now use the latest
stable Rust again.

Fixes trussed-dev#26
Fixes trussed-dev#28
@robin-nitrokey
Copy link
Member Author

Can also manifest as a segfault (e. g. if compiled with --release):

signal: 11, SIGSEGV: invalid memory reference

@nickray
Copy link
Member

nickray commented Feb 5, 2023

Oh, so it's our bug, and not a compiler regression?

@robin-nitrokey
Copy link
Member Author

Yes, I think so. The layout changes in Rust 1.67.0 probably just cause us to access a different memory region causing a different error.

robin-nitrokey added a commit to Nitrokey/littlefs2 that referenced this issue Feb 7, 2023
As we fixed the lookahead buffer overflow, we no longer have to pin the
Rust version.

Fixes trussed-dev#26
Fixes trussed-dev#28
robin-nitrokey added a commit to Nitrokey/littlefs2 that referenced this issue Feb 7, 2023
As we fixed the lookahead buffer overflow, we no longer have to pin the
Rust version.

Fixes trussed-dev#26
Fixes trussed-dev#28
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