From nobody Mon Feb 9 18:46:22 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1499240230649835.704226191536; Wed, 5 Jul 2017 00:37:10 -0700 (PDT) Received: from localhost ([::1]:44586 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSerz-0008PF-Ks for importer@patchew.org; Wed, 05 Jul 2017 03:37:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSeWt-0004mR-Il for qemu-devel@nongnu.org; Wed, 05 Jul 2017 03:15:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dSeWp-0007im-3J for qemu-devel@nongnu.org; Wed, 05 Jul 2017 03:15:19 -0400 Received: from mail-wm0-x232.google.com ([2a00:1450:400c:c09::232]:34906) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dSeWo-0007iG-PC for qemu-devel@nongnu.org; Wed, 05 Jul 2017 03:15:15 -0400 Received: by mail-wm0-x232.google.com with SMTP id w126so212855156wme.0 for ; Wed, 05 Jul 2017 00:15:14 -0700 (PDT) Received: from 640k.lan (94-39-191-51.adsl-ull.clienti.tiscali.it. [94.39.191.51]) by smtp.gmail.com with ESMTPSA id y35sm22202793wrc.51.2017.07.05.00.15.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Jul 2017 00:15:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=+JzI6HLR1rfvQw7ixfxpkD+Go/4Hv7tDNfgBBWmb4ik=; b=CSnvLii/+NdaFHem+7YybvpU5EcJZSt/U+RuK1QhKn4H7tPZ6Yt1l/KUFMDNSxNaXk oOr+3kEImN/NgmoXkAdzLckC60i4xr0nQYanZZsqMdaoMAlSVavfB+c/F671euBU3M5o +sXHP7Nnq93pejZbhQuBzS6yU+lWSM6a/sHqsw6m/y9AV4FJ9vScCCUUDxoBMVpxNkIy pZaRIl/NQTkFoZNv6l7oM/CA/+JtuN9r78jZobPjXMvhzJP0vDmzrZp9+30neyx0r/g8 Uyf7Sa5KsCjWScGXYRbiVMp382UFcmoNZf7mHEU1GMip6+WrC+gmZORCsSyrqyMBKCda 9jlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=+JzI6HLR1rfvQw7ixfxpkD+Go/4Hv7tDNfgBBWmb4ik=; b=rI1CJZCk6JOvM3ZLwkm9xHlHRlIQy/DNU06hPeDcR9wW9SZRtSapGsfv5GxI0XBtMh iFcF5eL/RFLUO+l4k+i0NlmSsJmIeKWBx9gLQWfvv5PG3wYRIXph5cEpWB70IUbsLwZn ZrEpVfk/oXJbXrJDOa93z+ceWwECV0dW7tx+NhPYOrbn+1Dz9X9GLUm+784OxYC/GeyT csdzYYtRB0ZP80FZ8XWXmuLrrm1vWNqwbM9uBanVkCwxr+yIm8ov72dCru4/jxFcnsNe L+gyt65pFx/+ZX8VgK91pI7YTo7nBhLRmBYdLCSSle+MzpQ/XHGLgsWBCC4OSpSTHvYP 024g== X-Gm-Message-State: AKS2vOx4qULwBYy0cnSYUYub/AiKx4fU2Z5cuHqyB5Y0DwcgX2WWXEQ0 pqHL6B4Fj1anLjXPiR8= X-Received: by 10.28.229.209 with SMTP id c200mr26559206wmh.43.1499238913380; Wed, 05 Jul 2017 00:15:13 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 5 Jul 2017 09:14:23 +0200 Message-Id: <1499238885-26161-21-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1499238885-26161-1-git-send-email-pbonzini@redhat.com> References: <1499238885-26161-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:400c:c09::232 Subject: [Qemu-devel] [PULL 20/42] qemu-thread: Assert locks are initialized before using X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Fam Zheng Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Fam Zheng Not all platforms check whether a lock is initialized before used. In particular Linux seems to be more permissive than OSX. Check initialization state explicitly in our code to catch such bugs earlier. Signed-off-by: Fam Zheng Message-Id: <20170704122325.25634-1-famz@redhat.com> Signed-off-by: Paolo Bonzini --- include/qemu/thread-posix.h | 4 ++++ include/qemu/thread-win32.h | 5 +++++ util/qemu-thread-posix.c | 27 +++++++++++++++++++++++++++ util/qemu-thread-win32.c | 34 +++++++++++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h index 09d1e15..e5e3a0f 100644 --- a/include/qemu/thread-posix.h +++ b/include/qemu/thread-posix.h @@ -12,10 +12,12 @@ typedef QemuMutex QemuRecMutex; =20 struct QemuMutex { pthread_mutex_t lock; + bool initialized; }; =20 struct QemuCond { pthread_cond_t cond; + bool initialized; }; =20 struct QemuSemaphore { @@ -26,6 +28,7 @@ struct QemuSemaphore { #else sem_t sem; #endif + bool initialized; }; =20 struct QemuEvent { @@ -34,6 +37,7 @@ struct QemuEvent { pthread_cond_t cond; #endif unsigned value; + bool initialized; }; =20 struct QemuThread { diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h index 4c4a261..3a05e3b 100644 --- a/include/qemu/thread-win32.h +++ b/include/qemu/thread-win32.h @@ -5,11 +5,13 @@ =20 struct QemuMutex { SRWLOCK lock; + bool initialized; }; =20 typedef struct QemuRecMutex QemuRecMutex; struct QemuRecMutex { CRITICAL_SECTION lock; + bool initialized; }; =20 void qemu_rec_mutex_destroy(QemuRecMutex *mutex); @@ -19,15 +21,18 @@ void qemu_rec_mutex_unlock(QemuRecMutex *mutex); =20 struct QemuCond { CONDITION_VARIABLE var; + bool initialized; }; =20 struct QemuSemaphore { HANDLE sema; + bool initialized; }; =20 struct QemuEvent { int value; HANDLE event; + bool initialized; }; =20 typedef struct QemuThreadData QemuThreadData; diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index eacd99e..4e95d27 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -43,12 +43,15 @@ void qemu_mutex_init(QemuMutex *mutex) err =3D pthread_mutex_init(&mutex->lock, NULL); if (err) error_exit(err, __func__); + mutex->initialized =3D true; } =20 void qemu_mutex_destroy(QemuMutex *mutex) { int err; =20 + assert(mutex->initialized); + mutex->initialized =3D false; err =3D pthread_mutex_destroy(&mutex->lock); if (err) error_exit(err, __func__); @@ -58,6 +61,7 @@ void qemu_mutex_lock(QemuMutex *mutex) { int err; =20 + assert(mutex->initialized); err =3D pthread_mutex_lock(&mutex->lock); if (err) error_exit(err, __func__); @@ -69,6 +73,7 @@ int qemu_mutex_trylock(QemuMutex *mutex) { int err; =20 + assert(mutex->initialized); err =3D pthread_mutex_trylock(&mutex->lock); if (err =3D=3D 0) { trace_qemu_mutex_locked(mutex); @@ -84,6 +89,7 @@ void qemu_mutex_unlock(QemuMutex *mutex) { int err; =20 + assert(mutex->initialized); trace_qemu_mutex_unlocked(mutex); err =3D pthread_mutex_unlock(&mutex->lock); if (err) @@ -102,6 +108,7 @@ void qemu_rec_mutex_init(QemuRecMutex *mutex) if (err) { error_exit(err, __func__); } + mutex->initialized =3D true; } =20 void qemu_cond_init(QemuCond *cond) @@ -111,12 +118,15 @@ void qemu_cond_init(QemuCond *cond) err =3D pthread_cond_init(&cond->cond, NULL); if (err) error_exit(err, __func__); + cond->initialized =3D true; } =20 void qemu_cond_destroy(QemuCond *cond) { int err; =20 + assert(cond->initialized); + cond->initialized =3D false; err =3D pthread_cond_destroy(&cond->cond); if (err) error_exit(err, __func__); @@ -126,6 +136,7 @@ void qemu_cond_signal(QemuCond *cond) { int err; =20 + assert(cond->initialized); err =3D pthread_cond_signal(&cond->cond); if (err) error_exit(err, __func__); @@ -135,6 +146,7 @@ void qemu_cond_broadcast(QemuCond *cond) { int err; =20 + assert(cond->initialized); err =3D pthread_cond_broadcast(&cond->cond); if (err) error_exit(err, __func__); @@ -144,6 +156,7 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) { int err; =20 + assert(cond->initialized); trace_qemu_mutex_unlocked(mutex); err =3D pthread_cond_wait(&cond->cond, &mutex->lock); trace_qemu_mutex_locked(mutex); @@ -174,12 +187,15 @@ void qemu_sem_init(QemuSemaphore *sem, int init) error_exit(errno, __func__); } #endif + sem->initialized =3D true; } =20 void qemu_sem_destroy(QemuSemaphore *sem) { int rc; =20 + assert(sem->initialized); + sem->initialized =3D false; #if defined(__APPLE__) || defined(__NetBSD__) rc =3D pthread_cond_destroy(&sem->cond); if (rc < 0) { @@ -201,6 +217,7 @@ void qemu_sem_post(QemuSemaphore *sem) { int rc; =20 + assert(sem->initialized); #if defined(__APPLE__) || defined(__NetBSD__) pthread_mutex_lock(&sem->lock); if (sem->count =3D=3D UINT_MAX) { @@ -238,6 +255,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) int rc; struct timespec ts; =20 + assert(sem->initialized); #if defined(__APPLE__) || defined(__NetBSD__) rc =3D 0; compute_abs_deadline(&ts, ms); @@ -285,6 +303,7 @@ void qemu_sem_wait(QemuSemaphore *sem) { int rc; =20 + assert(sem->initialized); #if defined(__APPLE__) || defined(__NetBSD__) pthread_mutex_lock(&sem->lock); while (sem->count =3D=3D 0) { @@ -310,6 +329,7 @@ void qemu_sem_wait(QemuSemaphore *sem) #else static inline void qemu_futex_wake(QemuEvent *ev, int n) { + assert(ev->initialized); pthread_mutex_lock(&ev->lock); if (n =3D=3D 1) { pthread_cond_signal(&ev->cond); @@ -321,6 +341,7 @@ static inline void qemu_futex_wake(QemuEvent *ev, int n) =20 static inline void qemu_futex_wait(QemuEvent *ev, unsigned val) { + assert(ev->initialized); pthread_mutex_lock(&ev->lock); if (ev->value =3D=3D val) { pthread_cond_wait(&ev->cond, &ev->lock); @@ -355,10 +376,13 @@ void qemu_event_init(QemuEvent *ev, bool init) #endif =20 ev->value =3D (init ? EV_SET : EV_FREE); + ev->initialized =3D true; } =20 void qemu_event_destroy(QemuEvent *ev) { + assert(ev->initialized); + ev->initialized =3D false; #ifndef __linux__ pthread_mutex_destroy(&ev->lock); pthread_cond_destroy(&ev->cond); @@ -370,6 +394,7 @@ void qemu_event_set(QemuEvent *ev) /* qemu_event_set has release semantics, but because it *loads* * ev->value we need a full memory barrier here. */ + assert(ev->initialized); smp_mb(); if (atomic_read(&ev->value) !=3D EV_SET) { if (atomic_xchg(&ev->value, EV_SET) =3D=3D EV_BUSY) { @@ -383,6 +408,7 @@ void qemu_event_reset(QemuEvent *ev) { unsigned value; =20 + assert(ev->initialized); value =3D atomic_read(&ev->value); smp_mb_acquire(); if (value =3D=3D EV_SET) { @@ -398,6 +424,7 @@ void qemu_event_wait(QemuEvent *ev) { unsigned value; =20 + assert(ev->initialized); value =3D atomic_read(&ev->value); smp_mb_acquire(); if (value !=3D EV_SET) { diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 653f29f..94f3491 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -46,15 +46,19 @@ static void error_exit(int err, const char *msg) void qemu_mutex_init(QemuMutex *mutex) { InitializeSRWLock(&mutex->lock); + mutex->initialized =3D true; } =20 void qemu_mutex_destroy(QemuMutex *mutex) { + assert(mutex->initialized); + mutex->initialized =3D false; InitializeSRWLock(&mutex->lock); } =20 void qemu_mutex_lock(QemuMutex *mutex) { + assert(mutex->initialized); AcquireSRWLockExclusive(&mutex->lock); trace_qemu_mutex_locked(mutex); } @@ -63,6 +67,7 @@ int qemu_mutex_trylock(QemuMutex *mutex) { int owned; =20 + assert(mutex->initialized); owned =3D TryAcquireSRWLockExclusive(&mutex->lock); if (owned) { trace_qemu_mutex_locked(mutex); @@ -73,6 +78,7 @@ int qemu_mutex_trylock(QemuMutex *mutex) =20 void qemu_mutex_unlock(QemuMutex *mutex) { + assert(mutex->initialized); trace_qemu_mutex_unlocked(mutex); ReleaseSRWLockExclusive(&mutex->lock); } @@ -80,25 +86,31 @@ void qemu_mutex_unlock(QemuMutex *mutex) void qemu_rec_mutex_init(QemuRecMutex *mutex) { InitializeCriticalSection(&mutex->lock); + mutex->initialized =3D true; } =20 void qemu_rec_mutex_destroy(QemuRecMutex *mutex) { + assert(mutex->initialized); + mutex->initialized =3D false; DeleteCriticalSection(&mutex->lock); } =20 void qemu_rec_mutex_lock(QemuRecMutex *mutex) { + assert(mutex->initialized); EnterCriticalSection(&mutex->lock); } =20 int qemu_rec_mutex_trylock(QemuRecMutex *mutex) { + assert(mutex->initialized); return !TryEnterCriticalSection(&mutex->lock); } =20 void qemu_rec_mutex_unlock(QemuRecMutex *mutex) { + assert(mutex->initialized); LeaveCriticalSection(&mutex->lock); } =20 @@ -106,25 +118,31 @@ void qemu_cond_init(QemuCond *cond) { memset(cond, 0, sizeof(*cond)); InitializeConditionVariable(&cond->var); + cond->initialized =3D true; } =20 void qemu_cond_destroy(QemuCond *cond) { + assert(cond->initialized); + cond->initialized =3D false; InitializeConditionVariable(&cond->var); } =20 void qemu_cond_signal(QemuCond *cond) { + assert(cond->initialized); WakeConditionVariable(&cond->var); } =20 void qemu_cond_broadcast(QemuCond *cond) { + assert(cond->initialized); WakeAllConditionVariable(&cond->var); } =20 void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) { + assert(cond->initialized); trace_qemu_mutex_unlocked(mutex); SleepConditionVariableSRW(&cond->var, &mutex->lock, INFINITE, 0); trace_qemu_mutex_locked(mutex); @@ -134,21 +152,28 @@ void qemu_sem_init(QemuSemaphore *sem, int init) { /* Manual reset. */ sem->sema =3D CreateSemaphore(NULL, init, LONG_MAX, NULL); + sem->initialized =3D true; } =20 void qemu_sem_destroy(QemuSemaphore *sem) { + assert(sem->initialized); + sem->initialized =3D false; CloseHandle(sem->sema); } =20 void qemu_sem_post(QemuSemaphore *sem) { + assert(sem->initialized); ReleaseSemaphore(sem->sema, 1, NULL); } =20 int qemu_sem_timedwait(QemuSemaphore *sem, int ms) { - int rc =3D WaitForSingleObject(sem->sema, ms); + int rc; + + assert(sem->initialized); + rc =3D WaitForSingleObject(sem->sema, ms); if (rc =3D=3D WAIT_OBJECT_0) { return 0; } @@ -160,6 +185,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) =20 void qemu_sem_wait(QemuSemaphore *sem) { + assert(sem->initialized); if (WaitForSingleObject(sem->sema, INFINITE) !=3D WAIT_OBJECT_0) { error_exit(GetLastError(), __func__); } @@ -193,15 +219,19 @@ void qemu_event_init(QemuEvent *ev, bool init) /* Manual reset. */ ev->event =3D CreateEvent(NULL, TRUE, TRUE, NULL); ev->value =3D (init ? EV_SET : EV_FREE); + ev->initialized =3D true; } =20 void qemu_event_destroy(QemuEvent *ev) { + assert(ev->initialized); + ev->initialized =3D false; CloseHandle(ev->event); } =20 void qemu_event_set(QemuEvent *ev) { + assert(ev->initialized); /* qemu_event_set has release semantics, but because it *loads* * ev->value we need a full memory barrier here. */ @@ -218,6 +248,7 @@ void qemu_event_reset(QemuEvent *ev) { unsigned value; =20 + assert(ev->initialized); value =3D atomic_read(&ev->value); smp_mb_acquire(); if (value =3D=3D EV_SET) { @@ -232,6 +263,7 @@ void qemu_event_wait(QemuEvent *ev) { unsigned value; =20 + assert(ev->initialized); value =3D atomic_read(&ev->value); smp_mb_acquire(); if (value !=3D EV_SET) { --=20 1.8.3.1