search

fitzgen / bumpalo

A fast bump allocation arena for Rust
https://docs.rs/bumpalo
Apache License 2.0
1.34k stars 109 forks source link

`allocator_api` tests failing #230

Closed overlookmotel closed 4 months ago

overlookmotel commented 4 months ago

The CI failure discovered in #229 is also failing on main (reproduced locally on M1 Macbook Pro).

cargo test --features allocator_api on nightly is sufficient to trigger it.

The failing test is: https://github.com/fitzgen/bumpalo/blob/f8278d6fff870d34bda7c2d03173710b2c9d12ea/tests/all/allocator_api.rs#L163-L211

Have traced it to: https://github.com/fitzgen/bumpalo/blob/f8278d6fff870d34bda7c2d03173710b2c9d12ea/src/lib.rs#L1705-L1745

That test seems to produce a lot of calls to Bump::shrink where old_size and new_size are both 1, and ptr and new_ptr are equal (so the copy is indeed overlapping).

In these cases, delta == 0 and old_size / 2 == 0, so (delta >= old_size / 2) == true.

Changing that test to delta > 0 && delta >= old_size / 2 solves this specific problem (cargo test --all-features passes), but I'm not sure if it still would allow other illegal cases through.

I'm afraid I don't understand the logic well enough to make a PR, but I hope this may be helpful in finding a fix.

overlookmotel commented 4 months ago

Maybe delta >= (old_size + 1) / 2 so the division rounds up instead of down?

overlookmotel commented 4 months ago

Yes, I think delta >= (old_size + 1) / 2 is right. Have made a PR #232.