[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] x86/altcall: further refine clang workaround
On Mon Jul 29, 2024 at 11:47 AM BST, Jan Beulich wrote: > On 29.07.2024 12:30, Roger Pau Monne wrote: > > --- a/xen/arch/x86/include/asm/alternative.h > > +++ b/xen/arch/x86/include/asm/alternative.h > > @@ -183,13 +183,13 @@ extern void alternative_branches(void); > > * https://github.com/llvm/llvm-project/issues/12579 > > * https://github.com/llvm/llvm-project/issues/82598 > > */ > > -#define ALT_CALL_ARG(arg, n) \ > > - register union { \ > > - typeof(arg) e[sizeof(long) / sizeof(arg)]; \ > > - unsigned long r; \ > > - } a ## n ## _ asm ( ALT_CALL_arg ## n ) = { \ > > - .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\ > > - } > > +#define ALT_CALL_ARG(arg, n) \ > > + register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) = ({ \ > > + unsigned long tmp = 0; \ > > + *(typeof(arg) *)&tmp = (arg); \ > > + BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); \ > > With this, even more so than before, I think the type of tmp would better > be void * (or the BUILD_BUG_ON() be made use unsigned long, yet I consider > that less desirable). As a nit, I also don't think the backslashes need > moving out by one position. Finally I'm afraid you're leaving stale the > comment ahead of this construct. > > Jan I wouldn't be thrilled to create a temp variable of yet another type that is potentially neither "typeof(arg)" nor "unsigned long". No need to create a 3 body problem, where 2 is enough. Adjusting BUILD_BUG_ON() to use the same temp type seems sensible, but I don't mind very much. With the stale comment adjusted: Reviewed-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx> Cheers, Alejandro
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |