| Message ID | 20230902151351.10325-1-javi.merino@cloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coverage: update gcov info for newer versions of gcc | expand |
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, \ > 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 ... > --- /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
On Thu, Sep 07, 2023 at 04:41:59PM +0200, Jan Beulich wrote: > 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, \ > > 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 ... Right! I tried to keep the current structure but I agree: it is simpler if they were all combined in gcc_4_7.c. This is what linux does. I'll simplify it as you suggest and send a v2. Cheers, Javi
Hi Jan, > On Sep 7, 2023, at 22:41, Jan Beulich <jbeulich@suse.com> wrote: > > Henry - afaict this was submitted after the feature submission deadline, > so you may want to consider giving it an exception. Yes I am ok to include this in 4.18. Kind regards, Henry > > Jan
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
On 11.09.2023 17:26, Alex Bennée wrote: > 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. That's the problem - we've been intending to raise the baseline for quite some time, but figuring at least a rule of thumb by which to go both now and in the future turns out problematic. Jan
Jan Beulich <jbeulich@suse.com> writes: > On 11.09.2023 17:26, Alex Bennée wrote: >> 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. > > That's the problem - we've been intending to raise the baseline for quite > some time, but figuring at least a rule of thumb by which to go both now > and in the future turns out problematic. FWIW QEMU states: The project aims to support the most recent major version at all times for up to five years after its initial release. Support for the previous major version will be dropped 2 years after the new major version is released or when the vendor itself drops support, whichever comes first. In this context, third-party efforts to extend the lifetime of a distro are not considered, even when they are endorsed by the vendor (eg. Debian LTS); the same is true of repositories that contain packages backported from later releases (e.g. Debian backports). Within each major release, only the most recent minor release is considered. For the purposes of identifying supported software versions available on Linux, the project will look at CentOS, Debian, Fedora, openSUSE, RHEL, SLES and Ubuntu LTS. Other distros will be assumed to ship similar software versions. We also make some specific statements about python runtimes and optional dependencies. Again this is perhaps easier for us as a user-space program but it's worthwhile writing something up as a policy even if you find you need to tweak it later. We also document our feature deprecation policy and what has been removed. If you want to crib anything from QEMU as an initial stab at it please see: https://qemu.readthedocs.io/en/master/about/index.html (this is docs/about/ in our source tree). > > Jan
diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile index 63f98c71d6..75d05909f8 100644 --- 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, \ 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)))))) else obj-y += llvm.o endif diff --git a/xen/common/coverage/gcc_10.c b/xen/common/coverage/gcc_10.c new file mode 100644 index 0000000000..f03f5c0f21 --- /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" + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/coverage/gcc_12.c b/xen/common/coverage/gcc_12.c new file mode 100644 index 0000000000..a551b6a51b --- /dev/null +++ b/xen/common/coverage/gcc_12.c @@ -0,0 +1,32 @@ +/* + * This code provides functions to handle gcc's profiling data format + * introduced with gcc 12. + * + * 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 < 120000 +#error "Wrong version of GCC used to compile gcov" +#endif + +#define GCOV_COUNTERS 8 +/* Since GCC 12.1, sizes are in BYTES and not in WORDS (4B). */ +#define GCOV_UNIT_SIZE 4 + +#include "gcc_4_7.c" + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c index 25b4a8bcdc..bf99062fcb 100644 --- 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 #define GCOV_TAG_FUNCTION_LENGTH 3 @@ -88,6 +89,10 @@ struct gcov_info { unsigned int version; struct gcov_info *next; unsigned int stamp; +#if (GCC_VERSION >= 120000) + /* GCC 12.1 introduced a checksum field */ + unsigned int checksum; +#endif const char *filename; void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int); unsigned int n_functions; @@ -160,13 +165,18 @@ size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info) pos += gcov_store_uint32(buffer, pos, info->version); pos += gcov_store_uint32(buffer, pos, info->stamp); +#if (GCC_VERSION >= 120000) + /* Use zero as checksum of the compilation unit. */ + pos += gcov_store_uint32(buffer, pos, 0); +#endif + for ( fi_idx = 0; fi_idx < info->n_functions; fi_idx++ ) { fi_ptr = info->functions[fi_idx]; /* Function record. */ pos += gcov_store_uint32(buffer, pos, GCOV_TAG_FUNCTION); - pos += gcov_store_uint32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH); + pos += gcov_store_uint32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH * GCOV_UNIT_SIZE); pos += gcov_store_uint32(buffer, pos, fi_ptr->ident); pos += gcov_store_uint32(buffer, pos, fi_ptr->lineno_checksum); pos += gcov_store_uint32(buffer, pos, fi_ptr->cfg_checksum); @@ -181,7 +191,7 @@ size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info) /* Counter record. */ pos += gcov_store_uint32(buffer, pos, GCOV_TAG_FOR_COUNTER(ct_idx)); - pos += gcov_store_uint32(buffer, pos, ci_ptr->num * 2); + pos += gcov_store_uint32(buffer, pos, ci_ptr->num * 2 * GCOV_UNIT_SIZE); for ( cv_idx = 0; cv_idx < ci_ptr->num; cv_idx++ ) pos += gcov_store_uint64(buffer, pos, ci_ptr->values[cv_idx]); diff --git a/xen/common/coverage/gcc_4_9.c b/xen/common/coverage/gcc_4_9.c index dcea961936..ded89ab9d2 100644 --- 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" diff --git a/xen/common/coverage/gcc_5.c b/xen/common/coverage/gcc_5.c index 6e0d276f3b..fe39053311 100644 --- 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" diff --git a/xen/common/coverage/gcc_7.c b/xen/common/coverage/gcc_7.c index 3962eb4c76..edfdc9fa44 100644 --- a/xen/common/coverage/gcc_7.c +++ b/xen/common/coverage/gcc_7.c @@ -11,11 +11,12 @@ #include "gcov.h" -#if GCC_VERSION < 70000 +#if GCC_VERSION < 70000 || GCC_VERSION >= 100000 #error "Wrong version of GCC used to compile gcov" #endif #define GCOV_COUNTERS 9 +#define GCOV_UNIT_SIZE 1 #include "gcc_4_7.c"
Shamelessly copy changes to gcov_info structures from linux so that we can capture coverage for xen built with newer compilers. Signed-off-by: Javi Merino <javi.merino@cloud.com> --- This doesn't solve all the problems with coverage as Xen still crashes when trying to reset/read coverage[0]. Still, it's a step forward. [0] https://gitlab.com/xen-project/xen/-/issues/168 xen/common/coverage/Makefile | 4 +++- xen/common/coverage/gcc_10.c | 31 +++++++++++++++++++++++++++++++ xen/common/coverage/gcc_12.c | 32 ++++++++++++++++++++++++++++++++ xen/common/coverage/gcc_4_7.c | 14 ++++++++++++-- xen/common/coverage/gcc_4_9.c | 1 + xen/common/coverage/gcc_5.c | 1 + xen/common/coverage/gcc_7.c | 3 ++- 7 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 xen/common/coverage/gcc_10.c create mode 100644 xen/common/coverage/gcc_12.c