[LTP] [PATCH] syscalls/brk: add direct syscall tst_variant

* [LTP] [PATCH] syscalls/brk: add direct syscall tst_variant
@ 2022-12-06 12:26 Teo Couprie Diaz
  2022-12-06 14:04 ` Petr Vorel
  0 siblings, 1 reply; 3+ messages in thread
From: Teo Couprie Diaz @ 2022-12-06 12:26 UTC (permalink / raw)
  To: ltp

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 exit prematurely. Invoking the syscall directly allows
them to properly validate brk behavior. Add a new test variant handling if
the libc wrappers are not implemented and testing the direct syscall.

Use tst_syscall() and handle the failure cases ourselves, as
we don't depend on the libc to do it anymore.

The patch also works around the dependency on sbrk to get the current break
as it has the same issues. Instead, call tst_syscall(__NR_brk, 0) which
always returns the current break.

Update brk01 to use void* to unify it with brk02.

Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
---
v1: Reworked from RFC by adding a variant rather than replacing the
    existing tests. Thanks Petr and Cyril !

Not sure this is the best way to do it. Maybe it could use a shared header
for testing if the libc version is implemented ?

Green CI: https://github.com/Teo-CD/ltp/actions/runs/3629342833/jobs/6121433817

 testcases/kernel/syscalls/brk/brk01.c | 36 +++++++++++++++++------
 testcases/kernel/syscalls/brk/brk02.c | 42 ++++++++++++++++++++++-----
 2 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/brk/brk01.c b/testcases/kernel/syscalls/brk/brk01.c
index a9b89bce5..bd12940c2 100644
--- a/testcases/kernel/syscalls/brk/brk01.c
+++ b/testcases/kernel/syscalls/brk/brk01.c
@@ -9,14 +9,31 @@
 #include <errno.h>
 
 #include "tst_test.h"
+#include "lapi/syscalls.h"
 
 void verify_brk(void)
 {
-	uintptr_t cur_brk, new_brk;
-	uintptr_t inc = getpagesize() * 2 - 1;
+	void *cur_brk, *new_brk;
+	size_t inc = getpagesize() * 2 - 1;
 	unsigned int i;
 
-	cur_brk = (uintptr_t)sbrk(0);
+	if (tst_variant) {
+		tst_res(TINFO, "Testing syscall variant");
+		cur_brk = (void *)tst_syscall(__NR_brk, 0);
+	} else {
+		tst_res(TINFO, "Testing libc variant");
+		cur_brk = (void *)sbrk(0);
+
+		if (cur_brk == (void*)-1)
+			tst_brk(TCONF, "sbrk() not implemented");
+
+		/*
+		 * Check if brk itself is implemented: updating to the current break
+		 * should be a no-op.
+		 */
+		if ((void *)brk(cur_brk) != cur_brk)
+			tst_brk(TCONF, "brk() not implemented");
+	}
 
 	for (i = 0; i < 33; i++) {
 		switch (i % 3) {
@@ -31,16 +48,16 @@ void verify_brk(void)
 		break;
 		}
 
-		TST_EXP_PASS_SILENT(brk((void *)new_brk), "brk()");
-		if (!TST_PASS)
-			return;
-
-		cur_brk = (uintptr_t)sbrk(0);
+		if (tst_variant) {
+			cur_brk = (void *)tst_syscall(__NR_brk, new_brk);
+		} else {
+			cur_brk = (void *)brk(new_brk);
+		}
 
 		if (cur_brk != new_brk) {
 			tst_res(TFAIL,
 				"brk() failed to set address have %p expected %p",
-				(void *)cur_brk, (void *)new_brk);
+				cur_brk, new_brk);
 			return;
 		}
 
@@ -54,4 +71,5 @@ void verify_brk(void)
 
 static struct tst_test test = {
 	.test_all = verify_brk,
+	.test_variants = 2,
 };
diff --git a/testcases/kernel/syscalls/brk/brk02.c b/testcases/kernel/syscalls/brk/brk02.c
index 11e803cb4..d26ef2100 100644
--- a/testcases/kernel/syscalls/brk/brk02.c
+++ b/testcases/kernel/syscalls/brk/brk02.c
@@ -14,24 +14,51 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include "tst_test.h"
+#include "lapi/syscalls.h"
+
+inline void *brk_variants(void *addr)
+{
+	void *brk_addr;
+	if (tst_variant) {
+		brk_addr = (void *)tst_syscall(__NR_brk, addr);
+	} else {
+		brk_addr = (void *)brk(addr);
+	}
+	return brk_addr;
+}
 
 void brk_down_vmas(void)
 {
-	void *brk_addr = sbrk(0);
+	void *brk_addr;
 
-	if (brk_addr == (void *) -1)
-		tst_brk(TBROK, "sbrk() failed");
+	if (tst_variant) {
+		tst_res(TINFO, "Testing syscall variant");
+		brk_addr = (void *)tst_syscall(__NR_brk, 0);
+	} else {
+		tst_res(TINFO, "Testing libc variant");
+		brk_addr = (void *)sbrk(0);
+
+		if (brk_addr == (void*)-1)
+			tst_brk(TCONF, "sbrk() not implemented");
+
+		/*
+		 * Check if brk itself is implemented: updating to the current break
+		 * should be a no-op.
+		 */
+		if ((void *)brk(brk_addr) != brk_addr)
+			tst_brk(TCONF, "brk() not implemented");
+	}
 
 	unsigned long page_size = getpagesize();
 	void *addr = brk_addr + page_size;
 
-	if (brk(addr)) {
+	if (brk_variants(addr) < addr) {
 		tst_res(TFAIL | TERRNO, "Cannot expand brk() by page size");
 		return;
 	}
 
 	addr += page_size;
-	if (brk(addr)) {
+	if (brk_variants(addr) < addr) {
 		tst_res(TFAIL | TERRNO, "Cannot expand brk() by 2x page size");
 		return;
 	}
@@ -42,12 +69,12 @@ void brk_down_vmas(void)
 	}
 
 	addr += page_size;
-	if (brk(addr)) {
+	if (brk_variants(addr) < addr) {
 		tst_res(TFAIL | TERRNO, "Cannot expand brk() after mprotect");
 		return;
 	}
 
-	if (brk(brk_addr)) {
+	if (brk_variants(brk_addr) != brk_addr) {
 		tst_res(TFAIL | TERRNO, "Cannot restore brk() to start address");
 		return;
 	}
@@ -57,4 +84,5 @@ void brk_down_vmas(void)
 
 static struct tst_test test = {
 	.test_all = brk_down_vmas,
+	.test_variants = 2,
 };

base-commit: 990c0b48f8508a747863b1dea5556fba57e1c304
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 3+ messages in thread