[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: Xen reliance on non-standard GCC features
On 08.06.2023 14:18, Roberto Bagnara wrote: > On 07/06/23 09:39, Jan Beulich wrote: >> On 05.06.2023 15:26, Roberto Bagnara wrote: >>> On 05/06/23 11:28, Jan Beulich wrote: >>>> On 05.06.2023 07:28, Roberto Bagnara wrote: >>> You are right: here are a few examples for U2: >>> >>> xen/arch/arm/cpuerrata.c:92.12-92.35: >>> empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used >>> is `/usr/bin/aarch64-linux-gnu-gcc-12' >>> xen/include/xen/spinlock.h:31.21-31.23: expanded from macro `_LOCK_DEBUG' >>> xen/include/xen/spinlock.h:143.57-143.67: expanded from macro >>> `SPIN_LOCK_UNLOCKED' >>> xen/include/xen/spinlock.h:144.43-144.60: expanded from macro >>> `DEFINE_SPINLOCK' >> >> I'm afraid this is a bad example, as it goes hand-in-hand with using >> another extension. I don't think using a non-empty initialization list >> is going to work with >> >> union lock_debug { }; > > Yes, this is C99 undefined behavior 58: > "A structure or union is defined as containing no named members (6.7.2.1)." > > Here is another example: > > lpae_t pte = {}; > > whereas we have > > typedef union { > uint64_t bits; > lpae_pt_t pt; > lpae_p2m_t p2m; > lpae_walk_t walk; > } lpae_t; > > >>> xen/arch/arm/cpuerrata.c:678.5-678.6: >>> empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used >>> is `/usr/bin/aarch64-linux-gnu-gcc-12' >>> >>> xen/arch/arm/cpufeature.c:33.5-33.6: >>> empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used >>> is `/usr/bin/aarch64-linux-gnu-gcc-12' >> >> Both of these are a common idiom we use: The "sentinel" of an array >> of compound type initializer. > > Wouldn't it be possible writing such sentinels in a standard-compliant > way, like {0} or similar, instead of {}? I would be possible, sure, but the question is whether we want that. Iirc in review comments we've been asking to preferably use {}, for being shorter / less clutter without resulting in any ambiguity. >>>>> U6) Empty declarations. >>> >>> Examples: >>> >>> xen/arch/arm/arm64/lib/find_next_bit.c:57.29: >>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is >>> `/usr/bin/aarch64-linux-gnu-gcc-12' >>> >>> xen/arch/arm/arm64/lib/find_next_bit.c:103.34: >>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is >>> `/usr/bin/aarch64-linux-gnu-gcc-12' >> >> Looks like these could be taken care of by finally purging our >> EXPORT_SYMBOL() stub. >> >>> xen/arch/arm/include/asm/vreg.h:143.26: >>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is >>> `/usr/bin/aarch64-linux-gnu-gcc-12' >>> >>> xen/arch/arm/include/asm/vreg.h:144.26: >>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 >>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is >>> `/usr/bin/aarch64-linux-gnu-gcc-12' >> >> I'm having trouble spotting anything suspicious there. > > The macro expands to definitions of inline functions > and after the macro invocation there is a ";". > > The preprocessed code is then: > > static inline void foo() { ... } > ; > > where the final ";" is an empty declaration not allowed by > the C99 language standard. Oh, I see. > Removing the ";" after the macro invocation is a possible solution, > but other possibilities exist if this is strongly unwanted. We have other macros to instantiate functions, and there no stray semicolons are used. I think this wants doing the same way here, but it being Arm code the ultimate say is with the Arm maintainers. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |