Re: [PATCH] selftests/nolibc: libc-test: avoid -Wstringop-overflow warnings

2023-09-11 Thread Willy Tarreau
Hi Thomas,

On Sun, Sep 10, 2023 at 09:29:01PM +0200, Thomas Weißschuh wrote:
> Newer versions of glibc annotate the poll() function with
> __attribute__(access) which triggers a compiler warning inside the
> testcase poll_fault.
> Avoid this by using a plain NULL which is enough for the testcase.
> To avoid potential future warnings also adapt the other EFAULT
> testcases, except select_fault as NULL is a valid value for its
> argument.
(...)

Looks good to me. I wouldn't be surprised if we're soon forced to do
the same with select() on some archs where it might be emulated.

Feel free to push it to the shared repo.

Thanks!
Willy


[PATCH] selftests/nolibc: libc-test: avoid -Wstringop-overflow warnings

2023-09-10 Thread Thomas Weißschuh
Newer versions of glibc annotate the poll() function with
__attribute__(access) which triggers a compiler warning inside the
testcase poll_fault.
Avoid this by using a plain NULL which is enough for the testcase.
To avoid potential future warnings also adapt the other EFAULT
testcases, except select_fault as NULL is a valid value for its
argument.

nolibc-test.c: In function ‘run_syscall’:
nolibc-test.c:338:62: warning: ‘poll’ writing 8 bytes into a region of size 0 
overflows the destination [-Wstringop-overflow=]
  338 | do { if (!(cond)) result(llen, SKIPPED); else ret += 
expect_syserr2(expr, expret, experr1, experr2, llen); } while (0)
  |  
^~~~
nolibc-test.c:341:9: note: in expansion of macro ‘EXPECT_SYSER2’
  341 | EXPECT_SYSER2(cond, expr, expret, experr, 0)
  | ^
nolibc-test.c:905:47: note: in expansion of macro ‘EXPECT_SYSER’
  905 | CASE_TEST(poll_fault);EXPECT_SYSER(1, 
poll((void *)1, 1, 0), -1, EFAULT); break;
  |   ^~~~
cc1: note: destination object is likely at address zero
In file included from /usr/include/poll.h:1,
 from nolibc-test.c:33:
/usr/include/sys/poll.h:54:12: note: in a call to function ‘poll’ declared with 
attribute ‘access (write_only, 1, 2)’
   54 | extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
  |^~~~

Signed-off-by: Thomas Weißschuh 
---
 tools/testing/selftests/nolibc/nolibc-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c 
b/tools/testing/selftests/nolibc/nolibc-test.c
index e2b70641a1e7..a0478f8eaee8 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -895,14 +895,14 @@ int run_syscall(int min, int max)
CASE_TEST(lseek_0);   EXPECT_SYSER(1, lseek(0, 0, 
SEEK_SET), -1, ESPIPE); break;
CASE_TEST(mkdir_root);EXPECT_SYSER(1, mkdir("/", 0755), 
-1, EEXIST); break;
CASE_TEST(mmap_bad);  EXPECT_PTRER(1, mmap(NULL, 0, 
PROT_READ, MAP_PRIVATE, 0, 0), MAP_FAILED, EINVAL); break;
-   CASE_TEST(munmap_bad);EXPECT_SYSER(1, munmap((void *)1, 
0), -1, EINVAL); break;
+   CASE_TEST(munmap_bad);EXPECT_SYSER(1, munmap(NULL, 0), 
-1, EINVAL); break;
CASE_TEST(mmap_munmap_good);  EXPECT_SYSZR(1, 
test_mmap_munmap()); break;
CASE_TEST(open_tty);  EXPECT_SYSNE(1, tmp = 
open("/dev/null", 0), -1); if (tmp != -1) close(tmp); break;
CASE_TEST(open_blah); EXPECT_SYSER(1, tmp = 
open("/proc/self/blah", 0), -1, ENOENT); if (tmp != -1) close(tmp); break;
CASE_TEST(pipe);  EXPECT_SYSZR(1, test_pipe()); 
break;
CASE_TEST(poll_null); EXPECT_SYSZR(1, poll(NULL, 0, 
0)); break;
CASE_TEST(poll_stdout);   EXPECT_SYSNE(1, ({ struct pollfd 
fds = { 1, POLLOUT, 0}; poll(, 1, 0); }), -1); break;
-   CASE_TEST(poll_fault);EXPECT_SYSER(1, poll((void *)1, 
1, 0), -1, EFAULT); break;
+   CASE_TEST(poll_fault);EXPECT_SYSER(1, poll(NULL, 1, 0), 
-1, EFAULT); break;
CASE_TEST(prctl); EXPECT_SYSER(1, 
prctl(PR_SET_NAME, (unsigned long)NULL, 0, 0, 0), -1, EFAULT); break;
CASE_TEST(read_badf); EXPECT_SYSER(1, read(-1, , 
1), -1, EBADF); break;
CASE_TEST(rmdir_blah);EXPECT_SYSER(1, rmdir("/blah"), 
-1, ENOENT); break;
@@ -911,7 +911,7 @@ int run_syscall(int min, int max)
CASE_TEST(select_stdout); EXPECT_SYSNE(1, ({ fd_set fds; 
FD_ZERO(); FD_SET(1, ); select(2, NULL, , NULL, NULL); }), -1); 
break;
CASE_TEST(select_fault);  EXPECT_SYSER(1, select(1, (void 
*)1, NULL, NULL, 0), -1, EFAULT); break;
CASE_TEST(stat_blah); EXPECT_SYSER(1, 
stat("/proc/self/blah", _buf), -1, ENOENT); break;
-   CASE_TEST(stat_fault);EXPECT_SYSER(1, stat((void *)1, 
_buf), -1, EFAULT); break;
+   CASE_TEST(stat_fault);EXPECT_SYSER(1, stat(NULL, 
_buf), -1, EFAULT); break;
CASE_TEST(stat_timestamps);   EXPECT_SYSZR(1, 
test_stat_timestamps()); break;
CASE_TEST(symlink_root);  EXPECT_SYSER(1, symlink("/", 
"/"), -1, EEXIST); break;
CASE_TEST(unlink_root);   EXPECT_SYSER(1, unlink("/"), -1, 
EISDIR); break;

---
base-commit: f7a6e4791e3d685eddca29b5d16d183ee0407caa
change-id: 20230910-nolibc-poll-fault-4152a6836ef8

Best regards,
-- 
Thomas Weißschuh