Re: [PATCH 13/13] tools/nolibc: sys_gettimeofday: riscv: use __NR_clock_gettime64 for rv32

On 2023-05-25 02:03:32+0800, Zhangjin Wu wrote:

rv32 uses the generic include/uapi/asm-generic/unistd.h and it has no __NR_gettimeofday and __NR_clock_gettime after kernel commit d4c08b9776b3 ("riscv: Use latest system call ABI"), use __NR_clock_gettime64 instead.

This code is based on src/time/gettimeofday.c of musl and sysdeps/unix/sysv/linux/clock_gettime.c of glibc.

Both __NR_clock_gettime and __NR_clock_gettime64 are added for sys_gettimeofday() for they share most of the code.

Notes:

• Both tv and tz are not directly passed to kernel clock_gettime* syscalls, so, it isn't able to check the pointer automatically with the get_user/put_user helpers just like kernel gettimeofday syscall does. instead, we emulate (but not completely) such checks in our new __NR_clock_gettime* branch of nolibc.

• kernel clock_gettime* syscalls can not get tz info, just like musl and glibc do, we set tz to zero to avoid a random number.

Signed-off-by: Zhangjin Wu falcon@tinylab.org

tools/include/nolibc/sys.h | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h index 2642b380c6aa..ad38cc3856be 100644 --- a/tools/include/nolibc/sys.h +++ b/tools/include/nolibc/sys.h @@ -26,6 +26,7 @@ #include "arch.h" #include "errno.h" +#include "string.h" #include "types.h" @@ -51,6 +52,11 @@

• should not be placed here.

*/ +/*

• • This is the first address past the end of the text segment (the program code).
• */

+extern char etext; /*

• int brk(void *addr);

@@ -554,7 +560,47 @@ long getpagesize(void) static __attribute__((unused)) int sys_gettimeofday(struct timeval *tv, struct timezone *tz) { +#ifdef __NR_gettimeofday return my_syscall2(__NR_gettimeofday, tv, tz); +#elif defined(__NR_clock_gettime) || defined(__NR_clock_gettime64) +#ifdef __NR_clock_gettime

• struct timespec ts;

+#else

• struct timespec64 ts;

+#define __NR_clock_gettime __NR_clock_gettime64 +#endif

• int ret;
• /* make sure tv pointer is at least after code segment */
• if (tv != NULL && (char *)tv <= &etext)

• return -EFAULT;
  

To me the weird etext comparisions don't seem to be worth it, to be honest.

• /* set tz to zero to avoid random number */
• if (tz != NULL) {

• if ((char *)tz > &etext)
  

• 	memset(tz, 0, sizeof(struct timezone));
  

• else
  

• 	return -EFAULT;
  

• }
• if (tv == NULL)

• return 0;
  

• ret = my_syscall2(__NR_clock_gettime, CLOCK_REALTIME, &ts);
• if (ret)

• return ret;
  

• tv->tv_sec = (time_t) ts.tv_sec;

+#ifdef __NR_clock_gettime64

Nitpick:

While this ifdef works and is correct its logic is a bit indirect. If it is copied to some other function in the future it may be incorrect there.

Without the #ifdef the compiler should still be able to optimize the code away.

• if (tv->tv_sec != ts.tv_sec)

• return -EOVERFLOW;
  

+#endif

• tv->tv_usec = ts.tv_nsec / 1000;
• return 0;

+#else +#error None of __NR_gettimeofday, __NR_clock_gettime and __NR_clock_gettime64 defined, cannot implement sys_gettimeofday() +#endif } static __attribute__((unused)) -- 2.25.1