[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xsm/flask: Fix build with Clang 13
On 25.04.2022 20:07, Andrew Cooper wrote: > Clang 13 chokes with: > > In file included from xsm/flask/flask_op.c:780: > xsm/flask/flask_op.c:698:33: error: passing 4-byte aligned argument to > 8-byte aligned parameter 1 of 'flask_ocontext_add' may result in an > unaligned pointer access [-Werror,-Walign-mismatch] > rv = flask_ocontext_add(&op.u.ocontext); > ^ > > and the same for flask_ocontext_del(). It isn't a problem in practice, > because the union always starts 8 bytes into {xen,compat}_flask_op_t, but the > diagnostic is based on type alignment alone. > > struct xen_flask_ocontext has the same layout between native and compat, but > does change alignment because of uint64_t, and there is only a native > implementation of flask_ocontext_add(). > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > --- > CC: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> > CC: Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx> > CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx> > CC: Jan Beulich <JBeulich@xxxxxxxx> > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx> > CC: Wei Liu <wl@xxxxxxx> > CC: Julien Grall <julien@xxxxxxx> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > > Slightly RFC because there don't appear to be any good options here. We cannot address this by altering the public header. Besides us having previously agreed to avoid the use of extensions outside of tools-only parts of these headers (or else you could simply use uint64_aligned_t), you're also altering the ABI for compat guests by changing the alignment. On irc yesterday we had 18:12:06 - jbeulich: With alignof() == 4 the compiler could put the variable on the stack, but not 8-byte aligned (if something else occupies another 32-bit slot). 18:13:34 - andyhhp: well - it can't in this case 18:13:37 - andyhhp: I agree it can in principle which I don't understand, but which I'd like to understand in this context: Why would the compiler not be allowed to place compat_flask_op()'s op variable 4-but-not-8-byte aligned on the stack? Then, if forcing 8-byte alignment of op, the compiler still issuing a diagnostic would be a (minor) bug imo, as taking into account variable alignment and offset into the structure would be enough to know that _this instance_ of the struct cannot be misaligned. (Of course this wouldn't help us, as we'd still need to work around the deficiency.) One possible way to deal with the problem that I can see (without having actually tried it) is to make the two functions take a parameter of compat_flask_ocontext_t *. That's type-compatible with struct xen_flask_ocontext *, but has reduced alignment. Of course this will require ugly #ifdef-ary because the type won't be available without CONFIG_COMPAT. Otoh any approach avoiding #ifdef-ary (like introducing yet another typedef matching that of compat_flask_ocontext_t, just without reference to struct compat_flask_ocontext) would needlessly impact !COMPAT builds. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |