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
