On 06/07/2017 14:16, Eric Blake wrote: > 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 <f...@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?
It depends on whether the compiler handles a non-1, nonzero value as true or false. If it counts as true, using an uint32_t with a magic value (0xacce55ed? ;)) would not be fooled by MALLOC_PERTURB_. This would be nice. >> @@ -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? It should be well predicted, but if it comes up in profiles we can make it conditional on release versions (or maybe you should use a spinlock instead). Paolo
signature.asc
Description: OpenPGP digital signature