Re: [PATCH 09/13] selftests: bpf: Add libarena selftests for the bump allocator

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Thu Jan 22, 2026 at 11:58 AM EST, bot+bpf-ci wrote:
>> diff --git a/tools/testing/selftests/bpf/libarena/selftests/st_asan_bump.bpf.c b/tools/testing/selftests/bpf/libarena/selftests/st_asan_bump.bpf.c
>> new file mode 100644
>> index 000000000000..a8f0e6e01e4b
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/libarena/selftests/st_asan_bump.bpf.c
>
> [ ... ]
>
>> +int asan_test_bump_blob_one(void)
>> +{
>> +	volatile struct blob __arena *blob;
>> +	const size_t alignment = 1;
>> +
>> +	blob = bump_alloc(sizeof(blob) - 1, alignment);
>                          ^^^^^^^^^^^^
>
> Is sizeof(blob) intended here? Since blob is declared as a pointer
> (volatile struct blob __arena *blob), sizeof(blob) returns the size of
> the pointer (8 bytes on 64-bit) rather than sizeof(struct blob) which
> is 60 bytes.
>
> This means the allocation is only 7 bytes, but the test later accesses
> blob->oob at offset 59. Should this be sizeof(*blob) - 1 instead?
>
> [ ... ]
>

Ack, the test is currently passing because we test only offsets 0 and
sizeof(*blob) + 1. Will fix.

>> diff --git a/tools/testing/selftests/bpf/libarena/selftests/st_bump.bpf.c b/tools/testing/selftests/bpf/libarena/selftests/st_bump.bpf.c
>> new file mode 100644
>> index 000000000000..99caae452343
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/libarena/selftests/st_bump.bpf.c
>
> [ ... ]
>
>> +#define ST_MAX_PAGES 8
>> +#define ST_MAX_BYTES (ST_MAX_PAGES * __PAGE_SIZE)
>
> [ ... ]
>
>> +static int bump_selftest_alloc_multiple(u64 bytes, u64 alignment)
>> +{
>> +	void __arena *mem1, *mem2;
>> +	int ret;
>> +
>> +	/* Initialize the allocator */
>> +	ret = bump_init(ST_MAX_PAGES);
>> +	if (ret) {
>> +		bpf_printk("bump_init failed with %d", ret);
>> +		return ret;
>> +	}
>> +
>> +	mem1 = ALLOC_OR_FAIL(bytes, alignment);
>> +	st_memset(mem1, ST_PATTERN1, bytes);
>> +
>> +	mem2 = ALLOC_OR_FAIL(bytes, alignment);
>> +	st_memset(mem2, ST_PATTERN1, ST_PATTERN2);
>                         ^^^^^^^^^^^  ^^^^^^^^^^^
>
> The st_memset signature is st_memset(mem, byte, size). This call passes
> ST_PATTERN1 (0xAA) as the byte pattern and ST_PATTERN2 (0x55 = 85) as
> the size, but later CHECK_OR_FAIL(mem2, ST_PATTERN2, bytes) expects mem2
> to contain ST_PATTERN2.
>
> Should this be st_memset(mem2, ST_PATTERN2, bytes)?
>
> Note: This code is currently unreachable due to the loop condition issue
> described below.
>

Ack, will update.

> [ ... ]
>
>> +SEC("syscall")
>> +int bump_selftest(void)
>> +{
>> +	u64 bytes = 128;
>> +	u64 alignment = 1;
>> +
>> +	for (bytes = __PAGE_SIZE; bytes <= ST_MAX_PAGES && can_loop;
>                                   ^^^^^^^^^^^^^^^^^^^^^^^
>
> The loop condition compares bytes (in bytes, initialized to __PAGE_SIZE
> which is typically 4096) against ST_MAX_PAGES (8, a page count). Since
> 4096 > 8 is always true, the condition bytes <= ST_MAX_PAGES is always
> false and this loop never executes.
>
> Should this compare against ST_MAX_BYTES instead?
>

Ack, will use ST_MAX_PAGES * __PAGE_SIZE as the bound.

>> +	     bytes <<= 1) {
>> +		for (alignment = 1; alignment <= ST_MAX_ALIGNMENT && can_loop;
>> +		     alignment <<= 1) {
>> +			/* Each test manages its own allocator lifecycle */
>> +			BUMP_ALLOC_SELFTEST(alloc_single, bytes, alignment);
>> +			BUMP_ALLOC_SELFTEST(alloc_multiple, bytes, alignment);
>> +		}
>> +	}
>> +
>> +	BUMP_ALLOC_SELFTEST(alloc_aligned);
>> +
>> +	for (alignment = __PAGE_SIZE; bytes <= ST_MAX_PAGES && can_loop;
>             ^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^^^^^
>> +	     bytes <<= 1)
>
> This loop initializes alignment but tests and advances bytes. The
> condition bytes <= ST_MAX_PAGES is false (bytes retains its value from
> the previous loop), so this loop also never executes.
>
> Was the intent to iterate over different alignment values? If so, both
> the condition and the increment should use alignment.
>

Ack, same issue and solution as above.

>> +		BUMP_ALLOC_SELFTEST(alloc_exhaustion,
>> +				    ST_MAX_PAGES * __PAGE_SIZE, alignment);
>> +
>> +	return 0;
>> +}
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21256973563