On 17/03/20 15:26, Stefan Hajnoczi wrote:
> Yes, looks like the compiler can't figure out the control flow on
> NetBSD.
>
> We could drop the WITH_QEMU_LOCK_GUARD() macro and use this idiom
> instead:
>
> {
> QEMU_LOCK_GUARD(&mutex);
> ...
> }
>
> But it's unusual for C code to create scopes without a statement (for,
> if, while).
After staring at compiler dumps for a while I have just concluded that
this could actually be considered a bug in WITH_QEMU_LOCK_GUARD.
QEMU_MAKE_LOCKABLE returns NULL if passed a NULL argument. This is the
root cause of the NetBSD failure, as the compiler doesn't figure out
that &timer_list->active_timers_lock is non-NULL and therefore doesn't
simplify the qemu_make_lockable function.
But why does that cause an uninitialized variable warning? Because if
WITH_QEMU_LOCK_GUARD were passed NULL, it would not execute its body!
So I'm going to squash the following in the series, mostly through a new
patch "lockable: introduce QEMU_MAKE_LOCKABLE_NONNULL":
diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 44b3f4b..1aeb2cb 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -67,7 +67,7 @@ qemu_make_lockable(void *x, QemuLockable *lockable)
* In C++ it would be different, but then C++ wouldn't need QemuLockable
* either...
*/
-#define QEMU_MAKE_LOCKABLE_(x) qemu_make_lockable((x), &(QemuLockable) { \
+#define QEMU_MAKE_LOCKABLE_(x) (&(QemuLockable) { \
.object = (x), \
.lock = QEMU_LOCK_FUNC(x), \
.unlock = QEMU_UNLOCK_FUNC(x), \
@@ -75,14 +75,27 @@ qemu_make_lockable(void *x, QemuLockable *lockable)
/* QEMU_MAKE_LOCKABLE - Make a polymorphic QemuLockable
*
- * @x: a lock object (currently one of QemuMutex, CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex, CoMutex,
QemuSpin).
*
* Returns a QemuLockable object that can be passed around
- * to a function that can operate with locks of any kind.
+ * to a function that can operate with locks of any kind, or
+ * NULL if @x is %NULL.
*/
#define QEMU_MAKE_LOCKABLE(x) \
QEMU_GENERIC(x, \
(QemuLockable *, (x)), \
+ qemu_make_lockable((x), QEMU_MAKE_LOCKABLE_(x)))
+
+/* QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
+ *
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex, CoMutex,
QemuSpin).
+ *
+ * Returns a QemuLockable object that can be passed around
+ * to a function that can operate with locks of any kind.
+ */
+#define QEMU_MAKE_LOCKABLE_NONNULL(x) \
+ QEMU_GENERIC(x, \
+ (QemuLockable *, (x)), \
QEMU_MAKE_LOCKABLE_(x))
static inline void qemu_lockable_lock(QemuLockable *x)
@@ -112,7 +125,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable,
qemu_lockable_auto_unlock)
#define WITH_QEMU_LOCK_GUARD_(x, var) \
for (g_autoptr(QemuLockable) var = \
- qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x))); \
+ qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE_NONNULL((x))); \
var; \
qemu_lockable_auto_unlock(var), var = NULL)
So thank you NetBSD compiler, I guess. :P
Paolo
signature.asc
Description: OpenPGP digital signature
