vcombey / fallible_collections

impl fallible collections in rust, quite as describe in RFC 2116
Apache License 2.0
31 stars 14 forks source link

Getting SIGSEGV: invalid memory reference when running tests #35

Open blinxen opened 1 year ago

blinxen commented 1 year ago

I am currently in the process of packaging this crate for fedora.

When trying to build the crate for the i686 architecture, I get the following error after running cargo test:

+ /usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 'RUSTFLAGS=-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Clink-arg=-Wl,-z,relro -Clink-arg=-Wl,-z,now -Clink-arg=-specs=/usr/lib/rpm/redhat/redhat-package-notes --cap-lints=warn' /usr/bin/cargo test -j6 -Z avoid-dev-deps --release --no-fail-fast vec
       Fresh version_check v0.9.4
       Fresh libc v0.2.139
       Fresh cfg-if v1.0.0
       Fresh once_cell v1.16.0
       Fresh getrandom v0.2.8
       Fresh ahash v0.7.6
       Fresh hashbrown v0.12.3
   Compiling fallible_collections v0.4.6 (/builddir/build/BUILD/fallible_collections-0.4.6)
     Running `/usr/bin/rustc --crate-name fallible_collections --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --emit=dep-info,link -C opt-level=3 -C embed-bitcode=no --test -C metadata=20f2dee70103e98f -C extra-filename=-20f2dee70103e98f --out-dir /builddir/build/BUILD/fallible_collections-0.4.6/target/release/deps -L dependency=/builddir/build/BUILD/fallible_collections-0.4.6/target/release/deps --extern hashbrown=/builddir/build/BUILD/fallible_collections-0.4.6/target/release/deps/libhashbrown-3d4950076ca73b1b.rlib -Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Clink-arg=-Wl,-z,relro -Clink-arg=-Wl,-z,now -Clink-arg=-specs=/usr/lib/rpm/redhat/redhat-package-notes --cap-lints=warn`
    Finished release [optimized] target(s) in 1.82s
     Running `/builddir/build/BUILD/fallible_collections-0.4.6/target/release/deps/fallible_collections-20f2dee70103e98f vec`
running 15 tests
test vec::tests::capacity_overflow ... ok
test vec::tests::oom ... ok
test vec::tests::extend_from_slice ... ok
test vec::tests::try_clone_vec ... ok
test vec::tests::try_extend_zst ... ok
test vec::tests::try_reserve ... ok
test vec::tests::try_reserve_idempotent ... ok
test vec::tests::try_reserve_zst ... ok
test vec::tests::tryvec_capacity_overflow ... ok
test vec::tests::tryvec_extend_from_slice ... ok
test vec::tests::tryvec_oom ... ok
test vec::tests::tryvec_reserve ... ok
test vec::tests::tryvec_reserve_idempotent ... ok
test vec::tests::try_clone_oom ... ok
error: test failed, to rerun pass `--lib`
Caused by:
  process didn't exit successfully: `/builddir/build/BUILD/fallible_collections-0.4.6/target/release/deps/fallible_collections-20f2dee70103e98f vec` (signal: 11, SIGSEGV: invalid memory reference)

Full build log

I could isolate the problem to the following test.

While I am trying to understand the underlying problem, I thought I would create this issue.

blinxen commented 1 year ago

In the meantime I found the following out:

The main issue here lies in the try_extend_from_slice_no_copy function. After trying to clone the 133937 (I don't really know if this index number is relevant or not) element, self.get_unchecked_mut(len) cannot return a valid reference and the program segfaults.

blinxen commented 1 year ago

It appears that when trying to reserve the memory for a new Vec, the allocator should not return a valid pointer. When running the test for 64 bit architectures, it obviously does not work because no machine has that much memory and the allocator return 0x0. But when running the test on a 32 bit architecture, allocating isize.MAX of memory is possible because that is around 2GB and all devices these days have more than 2 GB RAM.

cuviper commented 1 year ago

The new Vec is allocated fine, but the input is garbage, so it's eventually crashing on element.try_clone().

https://github.com/vcombey/fallible_collections/blob/ecd9e2b5c223a77561032e5001abb2ccca5d5132/src/vec.rs#L826-L829

That's allocating a single u8, then pretending it has length and capacity isize::MAX. That becomes the other slice, which isn't actually allocated for the length that it claims, so it segfaults when it reaches an invalid memory page.

bmickael commented 1 year ago

:cow:

The problem with this test when using fsanitize=address is initially because the requested allocation is too large. However, if we try to rewrite the code by making a small TryVec of 16mb first with the right layout, and then using try_clone() on it while retaining the references, and do this until there is no more memory at all, well, it takes hours to execute since try_clone() seems to make a rather slow copy...

It would be necessary to say that for this test only, TryClone should not make a copy. But I have no idea how to do that!