From: Paul Floyd Date: Sat, 30 Mar 2024 15:31:12 +0000 (+0100) Subject: Bug 484480 - False positives when using sem_trywait X-Git-Tag: VALGRIND_3_23_0~76 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=485cea48a38db8db608bdaff0c695ff50bbe16b5;p=thirdparty%2Fvalgrind.git Bug 484480 - False positives when using sem_trywait --- diff --git a/.gitignore b/.gitignore index f8b3cc0d5..347be07de 100644 --- a/.gitignore +++ b/.gitignore @@ -668,6 +668,7 @@ /helgrind/tests/bug322621 /helgrind/tests/bug327548 /helgrind/tests/bug392331 +/helgrind/tests/bug484480 /helgrind/tests/cond_init_destroy /helgrind/tests/cond_timedwait_invalid /helgrind/tests/cond_timedwait_test @@ -690,6 +691,7 @@ /helgrind/tests/pth_mempcpy_false_races /helgrind/tests/rwlock_race /helgrind/tests/rwlock_test +/helgrind/tests/sem_timedwait /helgrind/tests/shmem_abits /helgrind/tests/stackteardown /helgrind/tests/t2t diff --git a/NEWS b/NEWS index 3160bbe21..818901e7f 100644 --- a/NEWS +++ b/NEWS @@ -74,6 +74,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 483786 Incorrect parameter indexing in FreeBSD clock_nanosleep syscall wrapper 484002 Add suppression for invalid read in glibc's __wcpncpy_avx2() via wcsxfrm() 484426 aarch64: 0.5 gets rounded to 0 +484480 False positives when using sem_trywait n-i-bz Add redirect for memccpy To see details of a given bug, visit diff --git a/configure.ac b/configure.ac index 8279683ac..e7bfe4185 100755 --- a/configure.ac +++ b/configure.ac @@ -4971,7 +4971,8 @@ AC_CHECK_FUNCS([ \ wcsncpy \ free_aligned_sized \ wcpncpy \ - wcsxfrm + wcsxfrm \ + sem_timedwait ]) # AC_CHECK_LIB adds any library found to the variable LIBS, and links these @@ -5015,6 +5016,8 @@ AM_CONDITIONAL([HAVE_WCPNCPY], [test x$ac_cv_func_wcpncpy = xyes]) AM_CONDITIONAL([HAVE_WCSXFRM], [test x$ac_cv_func_wcsxfrm = xyes]) +AM_CONDITIONAL([HAVE_SEM_TIMEDWAIT], + [test x$ac_cv_func_sem_timedwait = xyes]) if test x$VGCONF_PLATFORM_PRI_CAPS = xMIPS32_LINUX \ -o x$VGCONF_PLATFORM_PRI_CAPS = xMIPS64_LINUX \ diff --git a/helgrind/hg_intercepts.c b/helgrind/hg_intercepts.c index 8c98e4ee0..28dca3c71 100644 --- a/helgrind/hg_intercepts.c +++ b/helgrind/hg_intercepts.c @@ -3192,6 +3192,132 @@ static int sem_wait_WRK(sem_t* sem) # error "Unsupported OS" #endif +//----------------------------------------------------------- +// glibc: sem_trywait +// glibc: sem_trywait@GLIBC_2.0 +// glibc: sem_trywait@@GLIBC_2.1 +// darwin: sem_trywait +// Solaris: sema_trywait (sem_teywait is built on top of sema_trywait) +// FreeBSD: sem_trywait (libc) +// +/* trywait: decrement semaphore if non-zero otherwise return error */ +__attribute__((noinline)) +static int sem_trywait_WRK(sem_t* sem) +{ + OrigFn fn; + int ret; + VALGRIND_GET_ORIG_FN(fn); + + if (TRACE_SEM_FNS) { + fprintf(stderr, "<< sem_trywait(%p) ", sem); + fflush(stderr); + } + + DO_CREQ_v_W(_VG_USERREQ__HG_POSIX_SEM_WAIT_PRE, sem_t*,sem); + + CALL_FN_W_W(ret, fn, sem); + + DO_CREQ_v_WW(_VG_USERREQ__HG_POSIX_SEM_WAIT_POST, sem_t*,sem, + long, (ret == 0) ? True : False); + + if (ret != 0) { + DO_PthAPIerror( "sem_trywait", SEM_ERROR ); + } + + if (TRACE_SEM_FNS) { + fprintf(stderr, " sem_trywait -> %d >>\n", ret); + fflush(stderr); + } + + return ret; +} + +#if defined(VGO_linux) +PTH_FUNC(int, semZutrywait, sem_t* sem) { /* sem_trywait */ + return sem_trywait_WRK(sem); +} +PTH_FUNC(int, semZutrywaitZAZa, sem_t* sem) { /* sem_trywait@* */ + return sem_trywait_WRK(sem); +} +#elif defined(VGO_darwin) +PTH_FUNC(int, semZutrywait, sem_t* sem) { /* sem_trywait */ + return sem_wait_WRK(sem); +} +#elif defined(VGO_freebsd) +LIBC_FUNC(int, semZutrywait, sem_t* sem) { /* sem_trywait */ + return sem_trywait_WRK(sem); +} +#elif defined(VGO_solaris) +PTH_FUNC(int, semaZutrywait, sem_t *sem) { /* sema_trywait */ + return sem_trywait_WRK(sem); +} +#else +# error "Unsupported OS" +#endif + +//----------------------------------------------------------- +// glibc: sem_timedwait +// glibc: sem_timedwait@GLIBC_2.0 +// glibc: sem_timedwait@@GLIBC_2.1 +// darwin: does not exist +// Solaris: sema_timedwait (sem_timedwait is built on top of sema_timedwait) +// FreeBSD: sem_timedwait (libc) +// +/* trywait: decrement semaphore if non-zero otherwise return error */ +__attribute__((noinline)) +static int sem_timedwait_WRK(sem_t* sem, const struct timespec* abs_timeout) +{ + OrigFn fn; + int ret; + VALGRIND_GET_ORIG_FN(fn); + + if (TRACE_SEM_FNS) { + fprintf(stderr, "<< sem_timedwait(%p, %p) ", sem, abs_timeout); + fflush(stderr); + } + + DO_CREQ_v_W(_VG_USERREQ__HG_POSIX_SEM_WAIT_PRE, sem_t*,sem); + + CALL_FN_W_WW(ret, fn, sem, abs_timeout); + + DO_CREQ_v_WW(_VG_USERREQ__HG_POSIX_SEM_WAIT_POST, sem_t*,sem, + long, (ret == 0) ? True : False); + + if (ret != 0) { + DO_PthAPIerror( "sem_timedwait", SEM_ERROR ); + } + + if (TRACE_SEM_FNS) { + fprintf(stderr, " sem_timedwait -> %d >>\n", ret); + fflush(stderr); + } + + return ret; +} + +#if defined(VGO_linux) +PTH_FUNC(int, semZutimedwait, sem_t* sem, const struct timespec* abs_timeout) { /* sem_timedwait */ + return sem_timedwait_WRK(sem, abs_timeout); +} +PTH_FUNC(int, semZutimedwaitZAZa, sem_t* sem, const struct timespec* abs_timeout) { /* sem_timedwait@* */ + return sem_timedwait_WRK(sem, abs_timeout); +} +#elif defined(VGO_darwin) + +// does not exist + +#elif defined(VGO_freebsd) +LIBC_FUNC(int, semZutimedwait, sem_t* sem, const struct timespec* abs_timeout) { /* sem_timedwait */ + return sem_timedwait_WRK(sem, abs_timeout); +} +#elif defined(VGO_solaris) +PTH_FUNC(int, semaZutimedwait, sem_t *sem, const struct timespec* abs_timeout) { /* sema_timedwait */ + return sem_timedwait_WRK(sem, abs_timeout); +} +#else +# error "Unsupported OS" +#endif + //----------------------------------------------------------- // glibc: sem_post diff --git a/helgrind/tests/Makefile.am b/helgrind/tests/Makefile.am index 28c079329..927e11116 100755 --- a/helgrind/tests/Makefile.am +++ b/helgrind/tests/Makefile.am @@ -24,6 +24,7 @@ EXTRA_DIST = \ bug392331.vgtest bug392331.stdout.exp bug392331.stderr.exp \ bug392331_supp.vgtest bug392331_supp.stdout.exp bug392331_supp.stderr.exp \ bug392331.supp \ + bug484480.vgtest bug484480.stderr.exp bug484480.stdout.exp \ cond_init_destroy.vgtest cond_init_destroy.stderr.exp \ cond_timedwait_invalid.vgtest cond_timedwait_invalid.stdout.exp \ cond_timedwait_invalid.stderr.exp \ @@ -71,6 +72,7 @@ EXTRA_DIST = \ pth_spinlock.vgtest pth_spinlock.stdout.exp pth_spinlock.stderr.exp \ rwlock_race.vgtest rwlock_race.stdout.exp rwlock_race.stderr.exp \ rwlock_test.vgtest rwlock_test.stdout.exp rwlock_test.stderr.exp \ + sem_timedwait.vgtest sem_timedwait.stdout.exp sem_timedwait.stderr.exp \ shared_timed_mutex.vgtest shared_timed_mutex.stderr.exp \ shmem_abits.vgtest shmem_abits.stdout.exp shmem_abits.stderr.exp \ stackteardown.vgtest stackteardown.stdout.exp stackteardown.stderr.exp \ @@ -152,6 +154,7 @@ noinst_HEADERS = safe-pthread.h safe-semaphore.h check_PROGRAMS = \ annotate_hbefore \ bug327548 \ + bug484480 \ cond_init_destroy \ cond_timedwait_invalid \ cond_timedwait_test \ @@ -244,6 +247,10 @@ if HAVE_GETADDRINFO check_PROGRAMS += getaddrinfo endif +if HAVE_SEM_TIMEDWAIT +check_PROGRAMS += sem_timedwait +endif + AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) diff --git a/helgrind/tests/bug484480.c b/helgrind/tests/bug484480.c new file mode 100644 index 000000000..8faae4bfc --- /dev/null +++ b/helgrind/tests/bug484480.c @@ -0,0 +1,33 @@ +#include +#include +#include + +static char* result; +static sem_t sem; + +static void* func(void* data) +{ + result = "Finished!"; + sem_post(&sem); + return NULL; +} + +int main(void) +{ + sem_init(&sem, 0, 0); + + pthread_t tid; + pthread_create(&tid, NULL, func, NULL); + + while (sem_trywait(&sem)) + ; // do nothing but keep retrying + + /* The above loop could be replaced this instead: + * if (sem_trywait(&sem)) + * sem_wait(&sem); + */ + + printf("%s\n", result); + + pthread_join(tid, NULL); +} diff --git a/helgrind/tests/bug484480.stderr.exp b/helgrind/tests/bug484480.stderr.exp new file mode 100644 index 000000000..e69de29bb diff --git a/helgrind/tests/bug484480.stdout.exp b/helgrind/tests/bug484480.stdout.exp new file mode 100644 index 000000000..1264c9437 --- /dev/null +++ b/helgrind/tests/bug484480.stdout.exp @@ -0,0 +1 @@ +Finished! diff --git a/helgrind/tests/bug484480.vgtest b/helgrind/tests/bug484480.vgtest new file mode 100644 index 000000000..b130a2744 --- /dev/null +++ b/helgrind/tests/bug484480.vgtest @@ -0,0 +1,2 @@ +vgopts: -q +prog: bug484480 diff --git a/helgrind/tests/sem_timedwait.c b/helgrind/tests/sem_timedwait.c new file mode 100644 index 000000000..4d75acac8 --- /dev/null +++ b/helgrind/tests/sem_timedwait.c @@ -0,0 +1,42 @@ +// this is a variation of bug484480.c using sem_timedwait + +#include +#include +#include +#include +#include +#include + +static char* result; +static sem_t sem; + +static void* func(void* data) +{ + result = "Finished!"; + sem_post(&sem); + return NULL; +} + +int main(void) +{ + sem_init(&sem, 0, 0); + + pthread_t tid; + pthread_create(&tid, NULL, func, NULL); + + struct timespec ts; + + if (clock_gettime(CLOCK_REALTIME, &ts) == -1) { + perror("clock_gettime"); + exit(EXIT_FAILURE); + } + + ts.tv_nsec += 100000; + + while (sem_timedwait(&sem, &ts)) + ; // do nothing but keep retrying + + printf("%s\n", result); + + pthread_join(tid, NULL); +} diff --git a/helgrind/tests/sem_timedwait.stderr.exp b/helgrind/tests/sem_timedwait.stderr.exp new file mode 100644 index 000000000..e69de29bb diff --git a/helgrind/tests/sem_timedwait.stdout.exp b/helgrind/tests/sem_timedwait.stdout.exp new file mode 100644 index 000000000..1264c9437 --- /dev/null +++ b/helgrind/tests/sem_timedwait.stdout.exp @@ -0,0 +1 @@ +Finished! diff --git a/helgrind/tests/sem_timedwait.vgtest b/helgrind/tests/sem_timedwait.vgtest new file mode 100644 index 000000000..a1c11996c --- /dev/null +++ b/helgrind/tests/sem_timedwait.vgtest @@ -0,0 +1,3 @@ +prereq: test -e ./sem_timedwait +vgopts: -q +prog: sem_timedwait