From: Aaron Merey via Valgrind-developers Date: Mon, 13 Apr 2026 03:46:18 +0000 (-0400) Subject: helgrind: fix lock-kind mismatch assert failure X-Git-Tag: VALGRIND_3_27_0~21 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=83fe5c31ecb5bf106ceec06d75187ff40da1bf3e;p=thirdparty%2Fvalgrind.git helgrind: fix lock-kind mismatch assert failure tl_assert(lk->kind == LK_rdwr) in evhH__post_thread_r_acquires_lock may fail in cases where memory containing an rwlock or mutex is reused for a lock of a different kind without having called pthread_*_destroy on the first lock. Fix this by having map_locks_lookup_or_create detect when a lock kind at a given guest address has changed from an rwlock to mutex or mutex to rwlock. The old lock record is removed and a new one is added reflecting the current lock kind. https://bugs.kde.org/show_bug.cgi?id=513598 --- diff --git a/.gitignore b/.gitignore index a185e7dab..547249762 100644 --- a/.gitignore +++ b/.gitignore @@ -682,6 +682,7 @@ /helgrind/tests/bug327548 /helgrind/tests/bug392331 /helgrind/tests/bug484480 +/helgrind/tests/bug513598 /helgrind/tests/cond_init_destroy /helgrind/tests/cond_timedwait_invalid /helgrind/tests/cond_timedwait_test diff --git a/NEWS b/NEWS index ae2449c0f..d2ff4de3d 100644 --- a/NEWS +++ b/NEWS @@ -97,6 +97,8 @@ are not entered into bugzilla tend to get forgotten about or ignored. 513522 m_libcassert.c: 'ordered comparison of pointer with integer zero' compiler warning 513475 Add SSE4.1 PMULLD instruction for x86 32 bit +513598 Helgrind should detect locks without pthread_{rwlock,mutex}_destroy + being called - Assertion 'lk->kind == LK_rdwr' failed. 514094 readlink("/proc/self/exe") overwrites buffer beyond its return value 514206 Assertion '!sr_isError(sr)' failed - mmap fd points to an open descriptor to a PCI device diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index 27739c37a..f282b8dd6 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -750,6 +750,10 @@ static Int HG_(get_pthread_create_nesting_level)(ThreadId tid) { /*--- map_locks :: WordFM guest-Addr-of-lock Lock* ---*/ /*----------------------------------------------------------------*/ +static void map_locks_delete ( Addr ga ); /* fwds */ +__attribute__((noinline)) +static void laog__handle_one_lock_deletion ( Lock* lk ); /* fwds */ + /* Make sure there is a lock table entry for the given (lock) guest address. If not, create one of the stated 'kind' in unheld state. In any case, return the address of the existing or new Lock. */ @@ -772,6 +776,33 @@ Lock* map_locks_lookup_or_create ( LockKind lkk, Addr ga, ThreadId tid ) tl_assert(oldlock != NULL); tl_assert(HG_(is_sane_LockN)(oldlock)); tl_assert(oldlock->guestaddr == ga); + /* Check for a mismatch between an rwlock and a mutex of either + kind. evh__HG_PTHREAD_MUTEX_LOCK_POST uses LK_mbRec as the + fallback kind, so a non-recursive mutex first encountered + at lock time rather than init time is expected to mismatch. */ + Bool old_is_mutex = (oldlock->kind == LK_nonRec + || oldlock->kind == LK_mbRec); + Bool new_is_mutex = (lkk == LK_nonRec || lkk == LK_mbRec); + if (old_is_mutex != new_is_mutex) { + /* At address GA a mutex has replaced an rwlock or an rwlock + has replaced a mutex. Remove the stale oldlock record. */ + if (oldlock->heldBy) { + remove_Lock_from_locksets_of_all_owning_Threads(oldlock); + VG_(deleteBag)(oldlock->heldBy); + oldlock->heldBy = NULL; + oldlock->heldW = False; + oldlock->acquired_at = NULL; + } + if (HG_(clo_track_lockorders)) + laog__handle_one_lock_deletion(oldlock); + map_locks_delete(ga); + del_LockN(oldlock); + Lock* lock = mk_LockN( lkk, ga ); + lock->appeared_at = VG_(record_ExeContext)( tid, 0 ); + tl_assert(HG_(is_sane_LockN)(lock)); + VG_(addToFM)( map_locks, (UWord)ga, (UWord)lock ); + return lock; + } return oldlock; } } @@ -1008,8 +1039,6 @@ static void all__sanity_check ( const HChar* who ) { static void laog__pre_thread_acquires_lock ( Thread*, Lock* ); /* fwds */ //static void laog__handle_lock_deletions ( WordSetID ); /* fwds */ static inline Thread* get_current_Thread ( void ); /* fwds */ -__attribute__((noinline)) -static void laog__handle_one_lock_deletion ( Lock* lk ); /* fwds */ /* Block-copy states (needed for implementing realloc()). */ diff --git a/helgrind/tests/Makefile.am b/helgrind/tests/Makefile.am index f7b2b1980..029c54307 100644 --- a/helgrind/tests/Makefile.am +++ b/helgrind/tests/Makefile.am @@ -15,6 +15,7 @@ noinst_SCRIPTS = \ filter_stderr EXTRA_DIST = \ + bug513598.vgtest \ annotate_hbefore.vgtest \ annotate_rwlock.vgtest annotate_rwlock.stderr.exp \ annotate_smart_pointer.vgtest annotate_smart_pointer.stderr.exp \ @@ -146,6 +147,7 @@ noinst_HEADERS = safe-pthread.h safe-semaphore.h # should be conditionally compiled like tc20_verifywrap is. check_PROGRAMS = \ annotate_hbefore \ + bug513598 \ bug327548 \ bug484480 \ cond_init_destroy \ diff --git a/helgrind/tests/bug513598.c b/helgrind/tests/bug513598.c new file mode 100644 index 000000000..9cc66fe5f --- /dev/null +++ b/helgrind/tests/bug513598.c @@ -0,0 +1,22 @@ +/* Verify that no assert failure occurs when memory holding a mutex + is reused for an rwlock without calling pthread_mutex_destroy first. */ + +#include + +int main(void) +{ + /* Force both locks to occupy the same address. */ + union { + pthread_mutex_t mutex; + pthread_rwlock_t rwlock; + } u; + + pthread_mutex_init(&u.mutex, NULL); + /* Deliberately skip pthread_mutex_destroy, simulating memory reuse + with a different lock type. */ + pthread_rwlock_init(&u.rwlock, NULL); + pthread_rwlock_rdlock(&u.rwlock); + pthread_rwlock_unlock(&u.rwlock); + pthread_rwlock_destroy(&u.rwlock); + return 0; +} diff --git a/helgrind/tests/bug513598.vgtest b/helgrind/tests/bug513598.vgtest new file mode 100644 index 000000000..99a2ae995 --- /dev/null +++ b/helgrind/tests/bug513598.vgtest @@ -0,0 +1,3 @@ +prereq: test -e bug513598 +prog: bug513598 +vgopts: -q