diff mbox series

[1/3] util/qemu-thread-posix: use TSA_NO_TSA to suppress clang TSA warnings

Message ID 20230117135203.3049709-2-eesposit@redhat.com (mailing list archive)
State New, archived
Headers show
Series TSA: make sure QEMU compiles when using clang TSA | expand

Commit Message

Emanuele Giuseppe Esposito Jan. 17, 2023, 1:52 p.m. UTC
QEMU does not compile when enabling clang's thread safety analysis
(TSA),
because some functions create wrappers for pthread mutexes but do
not use any TSA macro. Therefore the compiler fails.

In order to make the compiler happy and avoid adding all the
necessary macros to all callers (lock functions should use
TSA_ACQUIRE, while unlock TSA_RELEASE, and this applies to all
users of pthread_mutex_lock/pthread_mutex_unlock),
simply use TSA_NO_TSA to supppress such warnings.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 include/qemu/thread.h    | 14 +++++++++-----
 util/qemu-thread-posix.c |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

Comments

Philippe Mathieu-Daudé Jan. 17, 2023, 2:33 p.m. UTC | #1
On 17/1/23 14:52, Emanuele Giuseppe Esposito wrote:
> QEMU does not compile when enabling clang's thread safety analysis
> (TSA),
> because some functions create wrappers for pthread mutexes but do
> not use any TSA macro. Therefore the compiler fails.
> 
> In order to make the compiler happy and avoid adding all the
> necessary macros to all callers (lock functions should use
> TSA_ACQUIRE, while unlock TSA_RELEASE, and this applies to all
> users of pthread_mutex_lock/pthread_mutex_unlock),
> simply use TSA_NO_TSA to supppress such warnings.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   include/qemu/thread.h    | 14 +++++++++-----
>   util/qemu-thread-posix.c |  2 +-
>   2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 7c6703bce3..81ec9fc144 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -3,6 +3,7 @@
>   
>   #include "qemu/processor.h"
>   #include "qemu/atomic.h"
> +#include "qemu/clang-tsa.h"

Missing file?     ^^^^^^^^^^
Emanuele Giuseppe Esposito Jan. 17, 2023, 2:43 p.m. UTC | #2
Am 17/01/2023 um 15:33 schrieb Philippe Mathieu-Daudé:
> On 17/1/23 14:52, Emanuele Giuseppe Esposito wrote:
>> QEMU does not compile when enabling clang's thread safety analysis
>> (TSA),
>> because some functions create wrappers for pthread mutexes but do
>> not use any TSA macro. Therefore the compiler fails.
>>
>> In order to make the compiler happy and avoid adding all the
>> necessary macros to all callers (lock functions should use
>> TSA_ACQUIRE, while unlock TSA_RELEASE, and this applies to all
>> users of pthread_mutex_lock/pthread_mutex_unlock),
>> simply use TSA_NO_TSA to supppress such warnings.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>>   include/qemu/thread.h    | 14 +++++++++-----
>>   util/qemu-thread-posix.c |  2 +-
>>   2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
>> index 7c6703bce3..81ec9fc144 100644
>> --- a/include/qemu/thread.h
>> +++ b/include/qemu/thread.h
>> @@ -3,6 +3,7 @@
>>     #include "qemu/processor.h"
>>   #include "qemu/atomic.h"
>> +#include "qemu/clang-tsa.h"
> 
> Missing file?     ^^^^^^^^^^
> 
? Forgot to pull latest changes? I see clang-tsa.h in master
Philippe Mathieu-Daudé Jan. 17, 2023, 3:49 p.m. UTC | #3
On 17/1/23 15:43, Emanuele Giuseppe Esposito wrote:
> 
> 
> Am 17/01/2023 um 15:33 schrieb Philippe Mathieu-Daudé:
>> On 17/1/23 14:52, Emanuele Giuseppe Esposito wrote:
>>> QEMU does not compile when enabling clang's thread safety analysis
>>> (TSA),
>>> because some functions create wrappers for pthread mutexes but do
>>> not use any TSA macro. Therefore the compiler fails.
>>>
>>> In order to make the compiler happy and avoid adding all the
>>> necessary macros to all callers (lock functions should use
>>> TSA_ACQUIRE, while unlock TSA_RELEASE, and this applies to all
>>> users of pthread_mutex_lock/pthread_mutex_unlock),
>>> simply use TSA_NO_TSA to supppress such warnings.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>> ---
>>>    include/qemu/thread.h    | 14 +++++++++-----
>>>    util/qemu-thread-posix.c |  2 +-
>>>    2 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
>>> index 7c6703bce3..81ec9fc144 100644
>>> --- a/include/qemu/thread.h
>>> +++ b/include/qemu/thread.h
>>> @@ -3,6 +3,7 @@
>>>      #include "qemu/processor.h"
>>>    #include "qemu/atomic.h"
>>> +#include "qemu/clang-tsa.h"
>>
>> Missing file?     ^^^^^^^^^^
>>
> ? Forgot to pull latest changes? I see clang-tsa.h in master

Oops sorry I forgot to reset a git-bisection, and indeed was
based on a older base :\
diff mbox series

Patch

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 7c6703bce3..81ec9fc144 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -3,6 +3,7 @@ 
 
 #include "qemu/processor.h"
 #include "qemu/atomic.h"
+#include "qemu/clang-tsa.h"
 
 typedef struct QemuCond QemuCond;
 typedef struct QemuSemaphore QemuSemaphore;
@@ -24,9 +25,12 @@  typedef struct QemuThread QemuThread;
 
 void qemu_mutex_init(QemuMutex *mutex);
 void qemu_mutex_destroy(QemuMutex *mutex);
-int qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, const int line);
-void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, const int line);
-void qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, const int line);
+int TSA_NO_TSA qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file,
+                                       const int line);
+void TSA_NO_TSA qemu_mutex_lock_impl(QemuMutex *mutex, const char *file,
+                                     const int line);
+void TSA_NO_TSA qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file,
+                                       const int line);
 
 void qemu_rec_mutex_init(QemuRecMutex *mutex);
 void qemu_rec_mutex_destroy(QemuRecMutex *mutex);
@@ -153,8 +157,8 @@  void qemu_cond_destroy(QemuCond *cond);
  */
 void qemu_cond_signal(QemuCond *cond);
 void qemu_cond_broadcast(QemuCond *cond);
-void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex,
-                         const char *file, const int line);
+void TSA_NO_TSA qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex,
+                                    const char *file, const int line);
 bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms,
                               const char *file, const int line);
 
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index bae938c670..2dd1069cd3 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -223,7 +223,7 @@  void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const char *file, con
         error_exit(err, __func__);
 }
 
-static bool
+static bool TSA_NO_TSA
 qemu_cond_timedwait_ts(QemuCond *cond, QemuMutex *mutex, struct timespec *ts,
                        const char *file, const int line)
 {