Possible Undefined Behavior with Map::new in safe Rust

[cafce25]

As it's currently written the new in src/ctors.rs can lead to undefined behavior in safe Rust. You can invoke it with the following test:

#[test]
fn undefined() {
    let _m: Map<Vec<u8>, u8, 1> = Map::new();
}

I'm either getting a SIGILL or SIGSEGV depending on the actual conditions when running tests in release with

cargo test --release -- ctors::undefined

[yegor256]

@cafce25 can you please suggest how to solve this? We have to do here something very similar to what Vec::with_capacity() is doing: allocating memory for values that don't yet exist.

[yegor256]

@cafce25 in the meantime, I introduced a fix (0c81995), per this recommendation: https://stackoverflow.com/a/76091714/187141 However, the performance degraded because of this fix :(

[yegor256]

@rultor release, tag is 0.0.8

[rultor]

@rultor release, tag is 0.0.8

@yegor256 OK, I will release it now. Please check the progress here

[rultor]

@rultor release, tag is 0.0.8

@cafce25 @yegor256 Oops, I failed. You can see the full log here (spent 3min)

     Running `rustdoc --edition=2021 --crate-type lib --crate-name micromap --test /home/r/repo/src/lib.rs -L dependency=/home/r/repo/target/release/deps -L dependency=/home/r/repo/target/release/deps --test-args --nocapture --extern anyhow=/home/r/repo/target/release/deps/libanyhow-e76b10c6761c2811.rlib --extern bincode=/home/r/repo/target/release/deps/libbincode-f827f2a280ffe0fd.rlib --extern ctor=/home/r/repo/target/release/deps/libctor-516508eb3d680d6b.so --extern hashbrown=/home/r/repo/target/release/deps/libhashbrown-68832cec9ee9a4c8.rlib --extern indexmap=/home/r/repo/target/release/deps/libindexmap-7bfeb946ccdf20ea.rlib --extern linear_map=/home/r/repo/target/release/deps/liblinear_map-86b80b70500b259a.rlib --extern linked_hash_map=/home/r/repo/target/release/deps/liblinked_hash_map-d275c26307f81505.rlib --extern litemap=/home/r/repo/target/release/deps/liblitemap-5178d923608eeef1.rlib --extern log=/home/r/repo/target/release/deps/liblog-0e57e59038079a28.rlib --extern micromap=/home/r/repo/target/release/deps/libmicromap-8001cfc64a2ed4fd.rlib --extern nohash_hasher=/home/r/repo/target/release/deps/libnohash_hasher-48dc8f657769b4d0.rlib --extern rustc_hash=/home/r/repo/target/release/deps/librustc_hash-53e2f7bf5f27a195.rlib --extern serde=/home/r/repo/target/release/deps/libserde-c39e6318d95b6cd3.rlib --extern simple_logger=/home/r/repo/target/release/deps/libsimple_logger-047680921b4ed26a.rlib --extern tinymap=/home/r/repo/target/release/deps/libtinymap-12cb2cc9a80f0bb1.rlib -C embed-bitcode=no --cfg 'feature="serde"' --error-format human`

running 2 tests
test src/lib.rs - (line 29) ... ok
test src/eq.rs - eq::Map<K,V,N>::eq (line 28) ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.27s

+ cargo --color=never fmt --check
+ cargo --color=never clippy -- --no-deps
\u001b[0m\u001b[0m\u001b[1m\u001b[32m    Checking\u001b[0m micromap v0.0.8 (/home/r/repo)
\u001b[0m\u001b[0m\u001b[1m\u001b[32m    Finished\u001b[0m dev [unoptimized + debuginfo] target(s) in 0.40s
+ git commit -am 0.0.8
[__rultor bafdda5] 0.0.8
 3 files changed, 3 insertions(+), 3 deletions(-)
+ mkdir -p /home/r/.cargo
+ cp ../credentials /home/r/.cargo
+ cargo --color=never publish
    Updating crates.io index
   Packaging micromap v0.0.8 (/home/r/repo)
   Verifying micromap v0.0.8 (/home/r/repo)
   Compiling micromap v0.0.8 (/home/r/repo/target/package/micromap-0.0.8)
    Finished dev [unoptimized + debuginfo] target(s) in 0.48s
    Packaged 28 files, 53.4KiB (12.2KiB compressed)
   Uploading micromap v0.0.8 (/home/r/repo)
    Updating crates.io index
     Waiting on `micromap` to propagate to crates.io index (ctrl-c to wait asynchronously)
    Updating crates.io index
    Updating crates.io index
    Updating crates.io index
    Updating crates.io index
+ mv /home/r/repo .
++ whoami
+ chown -R root repo
+ '[' -n '' ']'
++ whoami
+ sudo chown -R rultor repo
+ cd repo
+ git checkout __rultor
Already on '__rultor'
+ git tag 0.0.8 -m '0.0.8: tagged by rultor.com'
+ git reset --hard
HEAD is now at bafdda5 0.0.8
+ git clean -fd
+ git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
+ git branch -D __rultor
Deleted branch __rultor (was bafdda5).
+ git push --all origin
To github.com:yegor256/micromap.git
 ! [rejected]        master -> master (fetch first)
error: failed to push some refs to 'git@github.com:yegor256/micromap.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
container f7280a135e87694d967ac89191e648e2c24d581509d5f636fc5b69fd707658cd is dead
Tue 25 Apr 2023 06:31:26 AM CEST

[yegor256]

@rultor release, tag is 0.0.9

[rultor]

@rultor release, tag is 0.0.9

@yegor256 OK, I will release it now. Please check the progress here

[rultor]

@rultor release, tag is 0.0.9

@yegor256 Done! FYI, the full log is here (took me 3min)

[cafce25]

I think MaybeUninit<Pair> is the fix, but you might want to

1. use MaybeUninit::write instead of creating i.e. replace

   self.pairs[i] = MaybeUninit::new(Absent);

   with

   self.pairs[i].write(Absent)

2. run your benchmarks in release mode cargo test --release -- --test benchmark --nocapture (might need some adjustments to ensure the actual usages aren't optimized away but will give more realistic numbers)

[yegor256]

@cafce25 thanks, I replaced assignments with write, as you suggested. Also, we run benchmarking scripts in release mode, see: https://github.com/yegor256/micromap/blob/master/rebuild_benchmark.sh#L21-L22

[cafce25]

I see, I only read tests/benchmark.rs and it suggests cargo test without --release