Re: [Qemu-devel] [PATCH] qemu-thread: Assert locks are initialized before using

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]

On 07/04/2017 07:23 AM, Fam Zheng wrote:
> 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 <famz@redhat.com>
> ---
>  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;
>  
>  struct QemuMutex {
>      pthread_mutex_t lock;
> +    bool initialized;
>  };

Are we worried about an object living on the stack and inheriting bit
values that make the object already appear initialized?  Would a magic
number a little less likely than '1' reduce the risk of inherited stack
garbage throwing us off?

Then again, several years ago, the Cygwin project quit using a magic
number cookie to track if synchronization objects were initialized, as
it ran into issues where repeated calls to a function that allocates an
object would cause the second allocation to fail because it saw leftover
stack contents from the first time through, so even with it's use of
something a little less likely than a bool '1', it still became a problem.


> @@ -58,6 +61,7 @@ void qemu_mutex_lock(QemuMutex *mutex)
>  {
>      int err;
>  
> +    assert(mutex->initialized);
>      err = pthread_mutex_lock(&mutex->lock);
>      if (err)
>          error_exit(err, __func__);

Are we sure this isn't going to penalize our code speed, by adding a
conditional on every lock/unlock?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]