Re: [PATCH for-4.15] xen: Bump the minimum version of GCC supported to 4.9 (5.1 on arm64)

[Jan Beulich <jbeulich@suse.com>]

On 06.03.2021 22:41, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Compilers older than 4.8 have known codegen issues which can lead to
> silent miscompilation:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
> 
> Furthermore, pre-4.9 GCC have known bugs (including things like
> internal compiler errors on Arm) which would require workaround (I
> haven't checked if we have any in Xen).
> 
> The minimum version of GCC to build the hypervisor is now raised to 4.9.
> 
> In addition to that, on arm64, GCC version >= 4.9 and < 5.1 have been
> shown to emit memory references beyond the stack pointer, resulting in
> memory corruption if an interrupt is taken after the stack pointer has
> been adjusted but before the reference has been executed.
> 
> Therefore, the minimum for arm64 is raised to 5.1.

I'm sure newer compiler versions also have bugs. Therefore I'm not
convinced using this as the primary reason for a bump is enough.
Plus what if critical to us bugs get found in, say, 5.x? Are we
going to bump to 6.x then (and so on, until we allow only the most
recent major version to be used)?

Additionally - partly related to your own reply regarding the CI
failures - imo there needs to be an analysis of what older distros
will no longer build (at all or by default). We've been discussing
to bump minimum tool chain versions for a long time. At least as
far as I'm concerned, I didn't take on this job precisely because
the code changes needed are relatively simple, but justification
can (and apparently will) be rather complicated.

Fundamentally, whatever kind of criteria we use to justify the
bump now ought to be usable down the road by people justifying
further bumps.

Also - what about clang? Linux requires 10.0.1 as a minimum.

> ---
> 
> I don't have a strong opinion on the minimum version for GCC on x86.
> So this is following Andrew's suggestion and the minimum from Linux.
> 
> This patch is candidate to 4.15 and backport.

I think such a change needs to be proposed much earlier in a release
cycle.

> --- a/README
> +++ b/README
> @@ -38,12 +38,15 @@ provided by your OS distributor:
>      * GNU Make v3.80 or later
>      * C compiler and linker:
>        - For x86:
> -        - GCC 4.1.2_20070115 or later
> +        - GCC 4.9 or later
>          - GNU Binutils 2.16.91.0.5 or later

I don't think it makes much sense to keep the binutils version this
low, the more that I don't think we can really build (correctly)
with this old a version anymore. Whatever the gcc version chosen, I
think we want to pick a binutils version from about the same time
frame.

> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -5,6 +5,19 @@
>  #error Sorry, your compiler is too old/not recognized.
>  #endif
>  
> +#if CONFIG_CC_IS_GCC
> +# if CONFIG_GCC_VERSION < 40900
> +/* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 */

As per the bug report, the issue was determined to not be present
in e.g. 4.3. Hence while such a bug may influence our choice of
minimum version, I don't think it can reasonably be named here as
the apparent only reason for the choice. Personally I don't think
any justification should be put here.

> +#  error Sorry, your version of GCC is too old - please use 4.9 or newer.
> +# elif defined(CONFIG_ARM_64) && CONFIG_GCC_VERSION < 50100
> +/*
> + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> + * https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> + */
> +#  error Sorry, your version of GCC is too old - please use 5.1 or newer.

From the bug entry the fix looks to have been backported to 4.9,
or at least some (important?) branches thereof.

> +# endif
> +#endif

Instead of a completely new conditional, I think this wants to be
combined with the existing one (the tail of which is visible in
context above).

Jan