Re: Xen reliance on non-standard GCC features

[Jan Beulich <jbeulich@xxxxxxxx>]

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