From 25c7d1f4a9c83183dd7c8d7b167c16eccdc55763 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Wed, 14 Feb 2024 02:10:31 +0000 Subject: [PATCH 1/2] Fix `Bump::shrink` illegal copies Fixes #230. --- src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 600f6ad..1b00f50 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -1724,7 +1724,10 @@ impl Bump { // Only reclaim the excess space (which requires a copy) if it // is worth it: we are actually going to recover "enough" space // and we can do a non-overlapping copy. - && delta >= old_size / 2 + // `(old_size + 1) / 2` so division rounds up rather than down, + // so this test fails for e.g. `old_size` = 1, `new_size` = 1 + // or `old_size` = 5, `new_size` = 3 (which would overlap). + && delta >= (old_size + 1) / 2 { let footer = self.current_chunk_footer.get(); let footer = footer.as_ref(); From 1be6a2274c5daf3c7758944a0f58cc5b6f17f990 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Wed, 14 Feb 2024 09:36:07 -0800 Subject: [PATCH 2/2] Expand comment about why we round up in the `shrink` check --- src/lib.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1b00f50..1ff980c 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -1724,9 +1724,31 @@ impl Bump { // Only reclaim the excess space (which requires a copy) if it // is worth it: we are actually going to recover "enough" space // and we can do a non-overlapping copy. - // `(old_size + 1) / 2` so division rounds up rather than down, - // so this test fails for e.g. `old_size` = 1, `new_size` = 1 - // or `old_size` = 5, `new_size` = 3 (which would overlap). + // + // We do `(old_size + 1) / 2` so division rounds up rather than + // down. Consider when: + // + // old_size = 5 + // new_size = 3 + // + // If we do not take care to round up, this will result in: + // + // delta = 2 + // (old_size / 2) = (5 / 2) = 2 + // + // And the the check will succeed even though we are have + // overlapping ranges: + // + // |--------old-allocation-------| + // |------from-------| + // |-------to--------| + // +-----+-----+-----+-----+-----+ + // | a | b | c | . | . | + // +-----+-----+-----+-----+-----+ + // + // But we MUST NOT have overlapping ranges because we use + // `copy_nonoverlapping` below! Therefore, we round the + // division up to avoid this issue. && delta >= (old_size + 1) / 2 { let footer = self.current_chunk_footer.get();