ltp.lists.linux.it archive mirror
 help / color / mirror / Atom feed
From: Teo Couprie Diaz <teo.coupriediaz@arm.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] syscalls/brk: add direct syscall tst_variant
Date: Tue,  6 Dec 2022 12:26:04 +0000	[thread overview]
Message-ID: <20221206122604.212528-1-teo.coupriediaz@arm.com> (raw)

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

             reply	other threads:[~2022-12-06 12:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-06 12:26 Teo Couprie Diaz [this message]
2022-12-06 14:04 ` [LTP] [PATCH] syscalls/brk: add direct syscall tst_variant Petr Vorel
2022-12-06 14:45   ` Teo Couprie Diaz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221206122604.212528-1-teo.coupriediaz@arm.com \
    --to=teo.coupriediaz@arm.com \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).