Re: [Qemu-devel] [PATCH v3 2/7] qapi: Add qobject_to()

[Eric Blake]

On 02/26/2018 05:58 AM, Max Reitz wrote:
> On 2018-02-24 21:57, Eric Blake wrote:
> > On 02/24/2018 09:40 AM, Max Reitz wrote:
> > > This is a dynamic casting macro that, given a QObject type, returns an
> > > object as that type or NULL if the object is of a different type (or
> > > NULL itself).

> > > +#define qobject_to(obj, type) \
> > > +    container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type))
> > > ?: \
> > > +                     QOBJECT((type *)NULL), \
> >
> > I guess the third (second?) branch of the ternary is written this way,
> > rather than the simpler 'NULL', to ensure that 'type' is still something
> > that can have the QOBJECT() macro applied to it?  Should be okay.
>
> It's written this way because of the container_of() around it.  We want
> the whole expression to return NULL then, and without the QOBJECT()
> around it, it would only return NULL if offsetof(type, base) == 0 (which
> it is not necessarily).
>
> OTOH, container_of(&((type *)NULL)->base, type, base) is by definition NULL.
>
> (QOBJECT(x) is &(x)->base)

Well, clang's ubsan griped:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05143.html

Practically, all of our qtypes have 'base' at offset 0, which means 
(QObject*)addr and (QString*)addr are the same address, even when addr 
is NULL.  But neither QOBJECT() nor container_of() are currently fit to 
run on a NULL pointer, since the 'base' member need not be at offset 0, 
at which point, we'd be converting away from the NULL pointer on the 
&(x)->base conversion, and then back to the NULL pointer on the 
container_of() conversion.  So at the end of the day, we get the right 
results, but we relied on undefined behavior in the interim.

So here's what I'm squashing in, if you like it (and remembering that I 
already swapped argument order to be qobject_to(type, obj) in my pending 
pull requests):

diff --git i/include/qapi/qmp/qobject.h w/include/qapi/qmp/qobject.h
index ea9702270e7..e6ce9347ab8 100644
--- i/include/qapi/qmp/qobject.h
+++ w/include/qapi/qmp/qobject.h
@@ -62,9 +62,8 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7,
                    "The QTYPE_CAST_TO_* list needs to be extended");

 #define qobject_to(type, obj) \
-    container_of(qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)) ?: \
-                     QOBJECT((type *)NULL), \
-                 type, base)
+    ({ QObject *_tmp = qobject_check_type(obj, glue(QTYPE_CAST_TO_, 
type)); \
+        _tmp ? container_of(_tmp, type, base) : (type *)NULL; })

 /* Initialize an object to default values */
 static inline void qobject_init(QObject *obj, QType type)


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