<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof">Hi Alistair,</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"><br>
</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof">The patch you submitted doesn't pass on x86 and x86-64.</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"><br>
</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof">I have written an alternative patch that works on the 32/64 variants
of ARM and x86. I could only verify that it builds on RISC-V 64.</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"><br>
</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof">Are you able to compile-test it on RISC-V 32?</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"><br>
</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof ContentPasted0"><a href="https://review.lttng.org/c/lttng-tools/+/8907" id="LPlnk816749">https://review.lttng.org/c/lttng-tools/+/8907</a><br>
</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"><br>
</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof">Thanks,</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof">Jérémie</span></div>
<div class="elementToProof"><span style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" class="elementToProof"><br>
</span></div>
<div class="elementToProof">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="Signature">
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
--</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Jérémie Galarneau</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<div>EfficiOS Inc.</div>
<a href="https://www.efficios.com">https://www.efficios.com</a><br>
</div>
</div>
</div>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> lttng-dev <lttng-dev-bounces@lists.lttng.org> on behalf of Alistair Francis via lttng-dev <lttng-dev@lists.lttng.org><br>
<b>Sent:</b> December 13, 2022 00:09<br>
<b>To:</b> Alistair Francis <alistair.francis@opensource.wdc.com><br>
<b>Cc:</b> lttng-dev@lists.lttng.org <lttng-dev@lists.lttng.org><br>
<b>Subject:</b> Re: [lttng-dev] [PATCH v2] Tests: select_poll_epoll: Add support for _time64</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Thu, Oct 27, 2022 at 3:54 PM Alistair Francis<br>
<alistair.francis@opensource.wdc.com> wrote:<br>
><br>
> From: Alistair Francis <alistair.francis@wdc.com><br>
><br>
> Add support for the 64-bit time_t syscalls SYS_ppoll_time64<br>
> and SYS_pselect6_time64.<br>
><br>
> These are the syscalls that exist 32-bit platforms since the 5.1 kernel.<br>
> 32-bit platforms with a 64-bit time_t only have these and don't have the<br>
> original syscalls (such as 32-bit RISC-V).<br>
><br>
> Fixes: <a href="https://github.com/lttng/lttng-tools/pull/162">https://github.com/lttng/lttng-tools/pull/162</a><br>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com><br>
<br>
Ping!<br>
<br>
Alistair<br>
<br>
> ---<br>
> To keep the test_cases[] array clean I have implemented the functions<br>
> for all builds, but the functions are a no-op if the syscall is missing.<br>
><br>
> v2:<br>
> - Split out a seperate _time64 test<br>
><br>
> tests/regression/kernel/select_poll_epoll.cpp | 184 ++++++++++++++++++<br>
> 1 file changed, 184 insertions(+)<br>
><br>
> diff --git a/tests/regression/kernel/select_poll_epoll.cpp b/tests/regression/kernel/select_poll_epoll.cpp<br>
> index c0b688217..dfaab52c8 100644<br>
> --- a/tests/regression/kernel/select_poll_epoll.cpp<br>
> +++ b/tests/regression/kernel/select_poll_epoll.cpp<br>
> @@ -5,6 +5,7 @@<br>
> *<br>
> */<br>
><br>
> +#include <errno.h><br>
> #include <fcntl.h><br>
> #include <limits.h><br>
> #include <poll.h><br>
> @@ -48,10 +49,14 @@ int lttng_opt_quiet, lttng_opt_verbose, lttng_opt_mi;<br>
><br>
> static void run_working_cases(FILE *validation_output_file);<br>
> static void pselect_invalid_fd(FILE *validation_output_file);<br>
> +static void pselect_time64_invalid_fd(FILE *validation_output_file);<br>
> static void test_ppoll_big(FILE *validation_output_file);<br>
> static void ppoll_fds_buffer_overflow(FILE *validation_output_file);<br>
> +static void ppoll_time64_fds_buffer_overflow(FILE *validation_output_file);<br>
> static void pselect_invalid_pointer(FILE *validation_output_file);<br>
> +static void pselect_time64_invalid_pointer(FILE *validation_output_file);<br>
> static void ppoll_fds_ulong_max(FILE *validation_output_file);<br>
> +static void ppoll_time64_fds_ulong_max(FILE *validation_output_file);<br>
> static void epoll_pwait_invalid_pointer(FILE *validation_output_file);<br>
> static void epoll_pwait_int_max(FILE *validation_output_file);<br>
> static void ppoll_concurrent_write(FILE *validation_output_file);<br>
> @@ -69,10 +74,14 @@ const struct test_case {<br>
> { .run = run_working_cases, .produces_validation_info = true, .timeout = -1 },<br>
> { .run = run_working_cases, .produces_validation_info = true, .timeout = 1 },<br>
> { .run = pselect_invalid_fd, .produces_validation_info = false, .timeout = 0 },<br>
> + { .run = pselect_time64_invalid_fd, .produces_validation_info = false, .timeout = 0 },<br>
> { .run = test_ppoll_big, .produces_validation_info = false, .timeout = 0 },<br>
> { .run = ppoll_fds_buffer_overflow, .produces_validation_info = false, .timeout = 0 },<br>
> + { .run = ppoll_time64_fds_buffer_overflow, .produces_validation_info = false, .timeout = 0 },<br>
> { .run = pselect_invalid_pointer, .produces_validation_info = false, .timeout = 0 },<br>
> + { .run = pselect_time64_invalid_pointer, .produces_validation_info = false, .timeout = 0 },<br>
> { .run = ppoll_fds_ulong_max, .produces_validation_info = false, .timeout = 0 },<br>
> + { .run = ppoll_time64_fds_ulong_max, .produces_validation_info = false, .timeout = 0 },<br>
> { .run = epoll_pwait_invalid_pointer, .produces_validation_info = true, .timeout = 0 },<br>
> { .run = epoll_pwait_int_max, .produces_validation_info = true, .timeout = 0 },<br>
> { .run = ppoll_concurrent_write, .produces_validation_info = false, .timeout = 0 },<br>
> @@ -440,6 +449,44 @@ end:<br>
> return;<br>
> }<br>
><br>
> +/*<br>
> + * Ask for 100 FDs in a buffer for allocated for only 1 FD, should<br>
> + * segfault (eventually with a "*** stack smashing detected ***" message).<br>
> + * The event should contain an array of 100 FDs filled with garbage.<br>
> + */<br>
> +static<br>
> +void ppoll_time64_fds_buffer_overflow(<br>
> + FILE *validation_output_file __attribute__((unused)))<br>
> +{<br>
> +#ifdef SYS_ppoll_time64<br>
> + struct pollfd ufds[NB_FD];<br>
> + char buf[BUF_SIZE];<br>
> + int ret;<br>
> +<br>
> + ufds[0].fd = wait_fd;<br>
> + ufds[0].events = POLLIN|POLLPRI;<br>
> +<br>
> + /*<br>
> + * As there is no timeout value, we don't convert to/from<br>
> + * 64/32-bit time_t.<br>
> + */<br>
> + ret = syscall(SYS_ppoll_time64, ufds, 100, NULL, NULL);<br>
> + /*<br>
> + * There is no fallback to SYS_ppoll, we expect SYS_ppoll_time64<br>
> + * to work and if it doesn't we fail.<br>
> + */<br>
> +<br>
> + if (ret < 0) {<br>
> + PERROR("ppoll_time64");<br>
> + } else if (ret > 0) {<br>
> + ret = read(wait_fd, buf, BUF_SIZE);<br>
> + if (ret < 0) {<br>
> + PERROR("[ppoll_time64] read");<br>
> + }<br>
> + }<br>
> +#endif<br>
> +}<br>
> +<br>
> /*<br>
> * Ask for 100 FDs in a buffer for allocated for only 1 FD, should<br>
> * segfault (eventually with a "*** stack smashing detected ***" message).<br>
> @@ -449,6 +496,7 @@ static<br>
> void ppoll_fds_buffer_overflow(<br>
> FILE *validation_output_file __attribute__((unused)))<br>
> {<br>
> +#ifdef SYS_ppoll<br>
> struct pollfd ufds[NB_FD];<br>
> char buf[BUF_SIZE];<br>
> int ret;<br>
> @@ -466,6 +514,46 @@ void ppoll_fds_buffer_overflow(<br>
> PERROR("[ppoll] read");<br>
> }<br>
> }<br>
> +#endif<br>
> +}<br>
> +<br>
> +/*<br>
> + * Ask for ULONG_MAX FDs in a buffer for allocated for only 1 FD, should<br>
> + * cleanly fail with a "Invalid argument".<br>
> + * The event should contain an empty array of FDs and overflow = 1.<br>
> + */<br>
> +static<br>
> +void ppoll_time64_fds_ulong_max(FILE *validation_output_file __attribute__((unused)))<br>
> +{<br>
> +#ifdef SYS_ppoll_time64<br>
> + struct pollfd ufds[NB_FD];<br>
> + char buf[BUF_SIZE];<br>
> + int ret;<br>
> +<br>
> + ufds[0].fd = wait_fd;<br>
> + ufds[0].events = POLLIN|POLLPRI;<br>
> +<br>
> + /*<br>
> + * As there is no timeout value, we don't convert to/from<br>
> + * 64/32-bit time_t.<br>
> + */<br>
> + ret = syscall(SYS_ppoll_time64, ufds, ULONG_MAX, NULL, NULL);<br>
> + /*<br>
> + * There is no fallback to SYS_ppoll, we expect SYS_ppoll_time64<br>
> + * to work and if it doesn't we fail.<br>
> + */<br>
> +<br>
> + if (ret < 0 && errno != ENOSYS) {<br>
> + /* Expected error. */<br>
> + } else if (errno == ENOSYS) {<br>
> + PERROR("[ppoll_time64] missing syscall");<br>
> + } else if (ret > 0) {<br>
> + ret = read(wait_fd, buf, BUF_SIZE);<br>
> + if (ret < 0) {<br>
> + PERROR("[ppoll_time64] read");<br>
> + }<br>
> + }<br>
> +#endif<br>
> }<br>
><br>
> /*<br>
> @@ -476,6 +564,7 @@ void ppoll_fds_buffer_overflow(<br>
> static<br>
> void ppoll_fds_ulong_max(FILE *validation_output_file __attribute__((unused)))<br>
> {<br>
> +#ifdef SYS_ppoll<br>
> struct pollfd ufds[NB_FD];<br>
> char buf[BUF_SIZE];<br>
> int ret;<br>
> @@ -492,6 +581,59 @@ void ppoll_fds_ulong_max(FILE *validation_output_file __attribute__((unused)))<br>
> PERROR("[ppoll] read");<br>
> }<br>
> }<br>
> +#endif<br>
> +}<br>
> +<br>
> +/*<br>
> + * Pass an invalid file descriptor to pselect6(). The syscall should return<br>
> + * -EBADF. The recorded event should contain a "ret = -EBADF (-9)".<br>
> + */<br>
> +static<br>
> +void pselect_time64_invalid_fd(FILE *validation_output_file __attribute__((unused)))<br>
> +{<br>
> +#ifdef SYS_pselect6_time64<br>
> + fd_set rfds;<br>
> + int ret;<br>
> + int fd;<br>
> + char buf[BUF_SIZE];<br>
> +<br>
> + /*<br>
> + * Open a file, close it and use the closed FD in the pselect6 call.<br>
> + */<br>
> + fd = open("/dev/null", O_RDONLY);<br>
> + if (fd == -1) {<br>
> + PERROR("open");<br>
> + goto error;<br>
> + }<br>
> +<br>
> + ret = close(fd);<br>
> + if (ret == -1) {<br>
> + PERROR("close");<br>
> + goto error;<br>
> + }<br>
> +<br>
> + FD_ZERO(&rfds);<br>
> + FD_SET(fd, &rfds);<br>
> +<br>
> + ret = syscall(SYS_pselect6_time64, fd + 1, &rfds, NULL, NULL, NULL, NULL);<br>
> + /*<br>
> + * There is no fallback to SYS_pselect6, we expect SYS_pselect6_time64<br>
> + * to work and if it doesn't we fail.<br>
> + */<br>
> +<br>
> + if (ret == -1 && errno != ENOSYS) {<br>
> + /* Expected error. */<br>
> + } else if (errno == ENOSYS) {<br>
> + PERROR("[pselect_time64] missing syscall");<br>
> + } else if (ret) {<br>
> + ret = read(wait_fd, buf, BUF_SIZE);<br>
> + if (ret < 0) {<br>
> + PERROR("[pselect_time64] read");<br>
> + }<br>
> + }<br>
> +error:<br>
> + return;<br>
> +#endif<br>
> }<br>
><br>
> /*<br>
> @@ -501,6 +643,7 @@ void ppoll_fds_ulong_max(FILE *validation_output_file __attribute__((unused)))<br>
> static<br>
> void pselect_invalid_fd(FILE *validation_output_file __attribute__((unused)))<br>
> {<br>
> +#ifdef SYS_pselect6<br>
> fd_set rfds;<br>
> int ret;<br>
> int fd;<br>
> @@ -525,6 +668,7 @@ void pselect_invalid_fd(FILE *validation_output_file __attribute__((unused)))<br>
> FD_SET(fd, &rfds);<br>
><br>
> ret = syscall(SYS_pselect6, fd + 1, &rfds, NULL, NULL, NULL, NULL);<br>
> +<br>
> if (ret == -1) {<br>
> /* Expected error. */<br>
> } else if (ret) {<br>
> @@ -535,6 +679,44 @@ void pselect_invalid_fd(FILE *validation_output_file __attribute__((unused)))<br>
> }<br>
> error:<br>
> return;<br>
> +#endif<br>
> +}<br>
> +<br>
> +/*<br>
> + * Invalid pointer as writefds, should output a ppoll event<br>
> + * with 0 FDs.<br>
> + */<br>
> +static<br>
> +void pselect_time64_invalid_pointer(<br>
> + FILE *validation_output_file __attribute__((unused)))<br>
> +{<br>
> +#ifdef SYS_pselect6_time64<br>
> + fd_set rfds;<br>
> + int ret;<br>
> + char buf[BUF_SIZE];<br>
> + void *invalid = (void *) 0x42;<br>
> +<br>
> + FD_ZERO(&rfds);<br>
> + FD_SET(wait_fd, &rfds);<br>
> +<br>
> + ret = syscall(SYS_pselect6_time64, 1, &rfds, (fd_set *) invalid, NULL, NULL,<br>
> + NULL);<br>
> + /*<br>
> + * There is no fallback to SYS_pselect6, we expect SYS_pselect6_time64<br>
> + * to work and if it doesn't we fail.<br>
> + */<br>
> +<br>
> + if (ret == -1 && errno != ENOSYS) {<br>
> + /* Expected error. */<br>
> + } else if (errno == ENOSYS) {<br>
> + PERROR("[pselect_time64] missing syscall");<br>
> + } else if (ret) {<br>
> + ret = read(wait_fd, buf, BUF_SIZE);<br>
> + if (ret < 0) {<br>
> + PERROR("[pselect_time64] read");<br>
> + }<br>
> + }<br>
> +#endif<br>
> }<br>
><br>
> /*<br>
> @@ -545,6 +727,7 @@ static<br>
> void pselect_invalid_pointer(<br>
> FILE *validation_output_file __attribute__((unused)))<br>
> {<br>
> +#ifdef SYS_pselect6<br>
> fd_set rfds;<br>
> int ret;<br>
> char buf[BUF_SIZE];<br>
> @@ -563,6 +746,7 @@ void pselect_invalid_pointer(<br>
> PERROR("[pselect] read");<br>
> }<br>
> }<br>
> +#endif<br>
> }<br>
><br>
> /*<br>
> --<br>
> 2.37.3<br>
><br>
_______________________________________________<br>
lttng-dev mailing list<br>
lttng-dev@lists.lttng.org<br>
<a href="https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev">https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev</a><br>
</div>
</span></font></div>
</body>
</html>