[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: Xen reliance on non-standard GCC features
On 09/06/2023 10:54, Jan Beulich wrote: > > > 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. Apart from vreg.h the same applies to TLB_HELPER of arm32/arm64. I think also TYPE_SAFE would want to be fixed. ~Michal
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |