Re: [PATCH] coverage: update gcov info for newer versions of gcc

["Alex Bennée" <alex.bennee@linaro.org>]

Jan Beulich <jbeulich@suse.com> writes:

> On 02.09.2023 17:11, Javi Merino wrote:
>> --- a/xen/common/coverage/Makefile
>> +++ b/xen/common/coverage/Makefile
>> @@ -5,7 +5,9 @@ obj-y += $(call cc-ifversion,-lt,0407, \
>>  		gcc_3_4.o, $(call cc-ifversion,-lt,0409, \

This isn't even supported, see below:

>>  		gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
>>  		gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
>> -		gcc_5.o, gcc_7.o))))
>> +		gcc_5.o, $(call cc-ifversion,-lt,1000, \
>> +		gcc_7.o,  $(call cc-ifversion,-lt,1200, \
>> +		gcc_10.o, gcc_12.o))))))
>
> This is getting unwieldy, so I think we ought to try to limit the number
> of different files we have. Already gcc_4_9.c and gcc_7.c specify the
> same GCOV_COUNTERS and differ only in the version checks (which could be
> combined). Therefore ...

You may want to consider your policy on supported GCC versions. I see
you still support:

    * C compiler and linker:
      - For x86:
        - GCC 4.1.2_20070115 or later
        - GNU Binutils 2.16.91.0.5 or later
        or
        - Clang/LLVM 3.5 or later
      - For ARM 32-bit:
        - GCC 4.9 or later
        - GNU Binutils 2.24 or later
      - For ARM 64-bit:
        - GCC 5.1 or later
        - GNU Binutils 2.24 or later

While also having some commented out warnings because the base GCC is so old:

  CFLAGS   += -Wmissing-prototypes
  # (gcc 4.3x and later)   -Wconversion -Wno-sign-conversion

For reference QEMU's minimum GCC is 7.4

  if compiler.get_id() == 'gcc' and compiler.version().version_compare('>=7.4')

and while our support cycle is based off distro LTS releases I have to
wonder do you actually need to support users building the
latest/greatest version of the hypervisor on ancient user-spaces with
old compilers?

I think the oldest distro you have in your CI is jessie (still hanging
on with extended LTS support) and that uses GCC 4.9.

I see there are various scripts to gather support status into the
documentation but I couldn't find a general statement on the projects
overall approach to supported versions of components.

>
>> --- /dev/null
>> +++ b/xen/common/coverage/gcc_10.c
>> @@ -0,0 +1,31 @@
>> +/*
>> + *  This code provides functions to handle gcc's profiling data format
>> + *  introduced with gcc 10.
>> + *
>> + *  For a better understanding, refer to gcc source:
>> + *  gcc/gcov-io.h
>> + *  libgcc/libgcov.c
>> + *
>> + *  Uses gcc-internal data definitions.
>> + */
>> +
>> +#include "gcov.h"
>> +
>> +#if GCC_VERSION < 100000 || GCC_VERSION > 120000
>> +#error "Wrong version of GCC used to compile gcov"
>> +#endif
>> +
>> +#define GCOV_COUNTERS 8
>> +#define GCOV_UNIT_SIZE 1
>> +
>> +#include "gcc_4_7.c"
>
> ... this could simply re-use gcc_4_7.c directly, with just the version
> check there suitably tweaked.
>
>> --- a/xen/common/coverage/gcc_4_7.c
>> +++ b/xen/common/coverage/gcc_4_7.c
>> @@ -27,6 +27,7 @@
>>  #  error "Wrong version of GCC used to compile gcov"
>>  # endif
>>  #define GCOV_COUNTERS 8
>> +#define GCOV_UNIT_SIZE 1
>>  #endif
>
> If further this became a separate #ifdef, ...
>
>> --- a/xen/common/coverage/gcc_4_9.c
>> +++ b/xen/common/coverage/gcc_4_9.c
>> @@ -19,6 +19,7 @@
>>  #endif
>>  
>>  #define GCOV_COUNTERS 9
>> +#define GCOV_UNIT_SIZE 1
>>  
>>  #include "gcc_4_7.c"
>>  
>> --- a/xen/common/coverage/gcc_5.c
>> +++ b/xen/common/coverage/gcc_5.c
>> @@ -19,6 +19,7 @@
>>  #endif
>>  
>>  #define GCOV_COUNTERS 10
>> +#define GCOV_UNIT_SIZE 1
>>  
>>  #include "gcc_4_7.c"
>>  
>
> ... touching these two files could be avoided altogether.
>
> Henry - afaict this was submitted after the feature submission deadline,
> so you may want to consider giving it an exception.
>
> Jan


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro