Hi Teo,
On 15-11-2022 17:01, Teo Couprie Diaz wrote:
Direct usage of brk is discouraged in favor of using malloc. Also, brk was removed from POSIX in POSIX.1-2001. In particular, the musl libc's brk always returns -ENOMEM which causes the LTP tests to fail. Invoking the syscall directly allows them to pass.
Instead, use tst_syscall() and handle the failure cases ourselves, as we don't depend on the libc to do it anymore.
The patch also removes the dependency on sbrk to get the current break as it has the same issues. Instead, call tst_syscall(__NR_brk, 0) which should always fail, returning the current break.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
I believe this patch could be upstreamed, maybe at first as an RFC to gather interest, as it would fix the tests for Musl distributions.
testcases/kernel/syscalls/brk/brk01.c | 9 +++------ testcases/kernel/syscalls/brk/brk02.c | 14 ++++++-------- 2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/testcases/kernel/syscalls/brk/brk01.c b/testcases/kernel/syscalls/brk/brk01.c index 5f482b943..919435755 100644 --- a/testcases/kernel/syscalls/brk/brk01.c +++ b/testcases/kernel/syscalls/brk/brk01.c @@ -9,6 +9,7 @@ #include <errno.h> #include "tst_test.h" +#include "lapi/syscalls.h" void verify_brk(void) { @@ -16,7 +17,7 @@ void verify_brk(void) size_t inc = getpagesize() * 2 - 1; unsigned int i;
- cur_brk = (uintptr_t)sbrk(0);
- cur_brk = (uintptr_t)tst_syscall(__NR_brk, 0);
for (i = 0; i < 33; i++) { switch (i % 3) { @@ -31,11 +32,7 @@ void verify_brk(void) break; }
TST_EXP_PASS_SILENT(brk((void *)new_brk), "brk()");if (!TST_PASS)return;cur_brk = (uintptr_t)sbrk(0);
cur_brk = tst_syscall(__NR_brk, new_brk);
nit: Should you have a cast to uintptr_t here as well? Or even better, make cur_brk and new_brk to be void* as in brk02.c? Up to you.
Also this patch needs e5d2a05a90e5 ("regen.sh: Use intptr_t for tst_syscall return") to be cherry-picked from upstream, right? Maybe add a note to it in the cover letter? Otherwise than that, I have no further comments on this series. Well done!
Thanks, Tudor
if (cur_brk != new_brk) { tst_res(TFAIL, diff --git a/testcases/kernel/syscalls/brk/brk02.c b/testcases/kernel/syscalls/brk/brk02.c index 11e803cb4..dabda30c2 100644 --- a/testcases/kernel/syscalls/brk/brk02.c +++ b/testcases/kernel/syscalls/brk/brk02.c @@ -14,24 +14,22 @@ #include <unistd.h> #include <sys/mman.h> #include "tst_test.h" +#include "lapi/syscalls.h" void brk_down_vmas(void) {
- void *brk_addr = sbrk(0);
- if (brk_addr == (void *) -1)
tst_brk(TBROK, "sbrk() failed");
- void *brk_addr = (void *)tst_syscall(__NR_brk, 0);
unsigned long page_size = getpagesize(); void *addr = brk_addr + page_size;
- if (brk(addr)) {
- if ((void *)tst_syscall(__NR_brk, addr) < addr) { tst_res(TFAIL | TERRNO, "Cannot expand brk() by page size"); return; }
addr += page_size;
- if (brk(addr)) {
- if ((void *)tst_syscall(__NR_brk, addr) < addr) { tst_res(TFAIL | TERRNO, "Cannot expand brk() by 2x page size"); return; }
@@ -42,12 +40,12 @@ void brk_down_vmas(void) } addr += page_size;
- if (brk(addr)) {
- if ((void *)tst_syscall(__NR_brk, addr) < addr) { tst_res(TFAIL | TERRNO, "Cannot expand brk() after mprotect"); return; }
- if (brk(brk_addr)) {
- if ((void *)tst_syscall(__NR_brk, brk_addr) != brk_addr) { tst_res(TFAIL | TERRNO, "Cannot restore brk() to start address"); return; }