Am 20.09.2023 um 20:31 hat Markus Armbruster geschrieben: > Variables declared in macros can shadow other variables. Much of the > time, this is harmless, e.g.: > > #define _FDT(exp) \ > do { \ > int ret = (exp); \ > if (ret < 0) { \ > error_report("error creating device tree: %s: %s", \ > #exp, fdt_strerror(ret)); \ > exit(1); \ > } \ > } while (0) > > Harmless shadowing in h_client_architecture_support(): > > target_ulong ret; > > [...] > > ret = do_client_architecture_support(cpu, spapr, vec, fdt_bufsize); > if (ret == H_SUCCESS) { > _FDT((fdt_pack(spapr->fdt_blob))); > [...] > } > > return ret; > > However, we can get in trouble when the shadowed variable is used in a > macro argument: > > #define QOBJECT(obj) ({ \ > typeof(obj) o = (obj); \ > o ? container_of(&(o)->base, QObject, base) : NULL; \ > }) > > QOBJECT(o) expands into > > ({ > ---> typeof(o) o = (o); > o ? container_of(&(o)->base, QObject, base) : NULL; > }) > > Unintended variable name capture at --->. We'd be saved by > -Winit-self. But I could certainly construct more elaborate death > traps that don't trigger it. > > To reduce the risk of trapping ourselves, we use variable names in > macros that no sane person would use elsewhere. Here's our actual > definition of QOBJECT(): > > #define QOBJECT(obj) ({ \ > typeof(obj) _obj = (obj); \ > _obj ? container_of(&(_obj)->base, QObject, base) : NULL; \ > }) > > Works well enough until we nest macro calls. For instance, with > > #define qobject_ref(obj) ({ \ > typeof(obj) _obj = (obj); \ > qobject_ref_impl(QOBJECT(_obj)); \ > _obj; \ > }) > > the expression qobject_ref(obj) expands into > > ({ > typeof(obj) _obj = (obj); > qobject_ref_impl( > ({ > ---> typeof(_obj) _obj = (_obj); > _obj ? container_of(&(_obj)->base, QObject, base) : NULL; > })); > _obj; > }) > > Unintended variable name capture at --->. > > The only reliable way to prevent unintended variable name capture is > -Wshadow. > > One blocker for enabling it is shadowing hiding in function-like > macros like > > qdict_put(dict, "name", qobject_ref(...)) > > qdict_put() wraps its last argument in QOBJECT(), and the last > argument here contains another QOBJECT(). > > Use dark preprocessor sorcery to make the macros that give us this > problem use different variable names on every call. > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > Reviewed-by: Eric Blake <ebl...@redhat.com> > --- > include/qapi/qmp/qobject.h | 11 +++++++++-- > include/qemu/atomic.h | 17 ++++++++++++----- > include/qemu/compiler.h | 3 +++ > include/qemu/osdep.h | 31 +++++++++++++++++++++++-------- > 4 files changed, 47 insertions(+), 15 deletions(-) > > diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h > index 9003b71fd3..d36cc97805 100644 > --- a/include/qapi/qmp/qobject.h > +++ b/include/qapi/qmp/qobject.h > @@ -45,10 +45,17 @@ struct QObject { > struct QObjectBase_ base; > }; > > -#define QOBJECT(obj) ({ \ > +/* > + * Preprocessory sorcery ahead: use a different identifier for the > + * local variable in each expansion, so we can nest macro calls > + * without shadowing variables. > + */ > +#define QOBJECT_INTERNAL(obj, _obj) ({ \ > typeof(obj) _obj = (obj); \ > - _obj ? container_of(&(_obj)->base, QObject, base) : NULL; \ > + _obj \ > + ? container_of(&(_obj)->base, QObject, base) : NULL; \
What happened here? The code in this line (or now two lines) seems to be unchanged apart from a strange looking newline. > }) > +#define QOBJECT(obj) QOBJECT_INTERNAL((obj), MAKE_IDENTFIER(_obj)) Kevin