From a0f2bdf91ddd4e5662790c7cd877052c9009441d Mon Sep 17 00:00:00 2001 From: Slobodan Predolac Date: Fri, 27 Mar 2026 09:57:28 -0700 Subject: [PATCH] Fix missing negation in large_ralloc_no_move usize_min fallback The second expansion attempt in large_ralloc_no_move omitted the ! before large_ralloc_no_move_expand(), inverting the return value. On expansion failure, the function falsely reported success, making callers believe the allocation was expanded in-place when it was not. On expansion success, the function falsely reported failure, causing callers to unnecessarily allocate, copy, and free. Add unit test that verifies the return value matches actual size change. --- Makefile.in | 1 + include/jemalloc/internal/util.h | 3 ++ src/large.c | 2 +- test/unit/large_ralloc.c | 76 ++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 test/unit/large_ralloc.c diff --git a/Makefile.in b/Makefile.in index 459f98fb..435fc34d 100644 --- a/Makefile.in +++ b/Makefile.in @@ -248,6 +248,7 @@ TESTS_UNIT := \ $(srcroot)test/unit/junk_alloc.c \ $(srcroot)test/unit/junk_free.c \ $(srcroot)test/unit/json_stats.c \ + $(srcroot)test/unit/large_ralloc.c \ $(srcroot)test/unit/log.c \ $(srcroot)test/unit/mallctl.c \ $(srcroot)test/unit/malloc_conf_2.c \ diff --git a/include/jemalloc/internal/util.h b/include/jemalloc/internal/util.h index bf246c95..ecfa76b8 100644 --- a/include/jemalloc/internal/util.h +++ b/include/jemalloc/internal/util.h @@ -20,6 +20,9 @@ */ #define JEMALLOC_ARG_CONCAT(...) __VA_ARGS__ +/* Number of elements in a fixed-size array. */ +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + /* cpp macro definition stringification. */ #define STRINGIFY_HELPER(x) #x #define STRINGIFY(x) STRINGIFY_HELPER(x) diff --git a/src/large.c b/src/large.c index 56fa16ab..6ccf49d7 100644 --- a/src/large.c +++ b/src/large.c @@ -147,7 +147,7 @@ large_ralloc_no_move(tsdn_t *tsdn, edata_t *edata, size_t usize_min, } /* Try again, this time with usize_min. */ if (usize_min < usize_max && usize_min > oldusize - && large_ralloc_no_move_expand( + && !large_ralloc_no_move_expand( tsdn, edata, usize_min, zero)) { arena_decay_tick(tsdn, arena_get_from_edata(edata)); return false; diff --git a/test/unit/large_ralloc.c b/test/unit/large_ralloc.c new file mode 100644 index 00000000..1f08d125 --- /dev/null +++ b/test/unit/large_ralloc.c @@ -0,0 +1,76 @@ +#include "test/jemalloc_test.h" + +/* + * Test that large_ralloc_no_move causes a failure (returns true) when + * in-place extent expansion cannot succeed for either usize_max or + * usize_min. + * + * A previous bug omitted the ! negation on the second extent expansion + * attempt (usize_min fallback), causing false success (return false) when + * the expansion actually failed. + */ +TEST_BEGIN(test_large_ralloc_no_move_expand_fail) { + /* + * Allocate two adjacent large objects in the same arena to block + * in-place expansion of the first one. + */ + unsigned arena_ind; + size_t sz = sizeof(arena_ind); + expect_d_eq(mallctl("arenas.create", (void *)&arena_ind, &sz, NULL, 0), + 0, "Unexpected mallctl() failure"); + + int flags = MALLOCX_ARENA(arena_ind) | MALLOCX_TCACHE_NONE; + + size_t large_sz = SC_LARGE_MINCLASS; + /* Allocate several blocks to prevent expansion of the first. */ + void *blocks[8]; + for (size_t i = 0; i < ARRAY_SIZE(blocks); i++) { + blocks[i] = mallocx(large_sz, flags); + expect_ptr_not_null(blocks[i], "Unexpected mallocx() failure"); + } + + /* + * Try to expand blocks[0] in place. Use usize_min < usize_max to + * exercise the fallback path. + */ + tsd_t *tsd = tsd_fetch(); + edata_t *edata = emap_edata_lookup( + tsd_tsdn(tsd), &arena_emap_global, blocks[0]); + expect_ptr_not_null(edata, "Unexpected edata lookup failure"); + + size_t oldusize = edata_usize_get(edata); + size_t usize_min = sz_s2u(oldusize + 1); + size_t usize_max = sz_s2u(oldusize * 2); + + /* Ensure min and max are in different size classes. */ + if (usize_min == usize_max) { + usize_max = sz_s2u(usize_min + 1); + } + + bool ret = large_ralloc_no_move( + tsd_tsdn(tsd), edata, usize_min, usize_max, false); + + /* + * With adjacent allocations blocking expansion, this should fail. + * The bug caused ret == false (success) even when expansion failed. + */ + if (!ret) { + /* + * Expansion might actually succeed if adjacent memory + * is free. Verify the size actually changed. + */ + size_t newusize = edata_usize_get(edata); + expect_zu_ge(newusize, usize_min, + "Expansion reported success but size didn't change"); + } + + for (size_t i = 0; i < ARRAY_SIZE(blocks); i++) { + dallocx(blocks[i], flags); + } +} +TEST_END + +int +main(void) { + return test_no_reentrancy(test_large_ralloc_no_move_expand_fail); +}