[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Xen reliance on non-standard GCC features
Hi Jan,
On 09/06/2023 09: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.
I don't think there is a reason to keep the ";" after. So I would be
fine if this is removed.
Cheers,
--
Julien Grall
|
