Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Thu, Oct 13, 2016 at 03:15:09AM -0600, Jan Beulich wrote:
> >>> On 13.10.16 at 10:49,  wrote:
> > On Thu, Oct 13, 2016 at 02:29:08AM -0600, Jan Beulich wrote:
> > [...]
> >> >> >> ... this structure's trailing fields actually getting used by the 
> >> >> >> code
> >> >> >> won't work well when changing compiler versions without cleaning
> >> >> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> >> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> >> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> >> >> development variant of 5.x) would use anything different from
> >> >> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> >> >> normally be necessary anymore with gcc 5+.
> >> >> >> 
> >> >> > 
> >> >> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> >> >> > "y" part is __GNUC_PATCHLEVEL__.
> >> >> 
> >> >> No, I didn't. From 5.x onwards the information previously carried in
> >> >> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
> >> >> as previously you would not normally need to look at the former,
> >> >> with newer gcc you shouldn't need to look at the latter.
> >> >> 
> >> > 
> >> > I can't find relevant information in GCC cpp manual.
> >> > 
> >> > Specifically, I look at 4.9.4 and 5.4.0 doc:
> >> > 
> >> > 
> > https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
> >  
> > 
> >> > on-Predefined-Macros
> >> > 
> > https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
> >  
> > 
> >> > on-Predefined-Macros
> >> > 
> >> > The sections about __GNUC_* macros are identical, their semantics stay
> >> > the same.
> >> > 
> >> > What did I miss?
> >> 
> >> Their change in how version numbers get used. I'm sure you've noticed
> >> there never was a released 5.0.0 or 6.0.0, and that the stable updates
> >> following 5.1.0 were 5.2.0, 5.3.0, etc.
> >> 
> > 
> > OK. I found the bits at https://gcc.gnu.org/develop.html. I see what you
> > meant previously.
> > 
> > It doesn't seem to be a problem to me to compare to 5.1 though -- that's
> > the first release of gcc 5, which should be what people use anyway.
> 
> But your check should cover the introduction point of the feature,
> which is 5.0.0 imo.
> 
> > If it is the complexity of the macro that concerns you, now it has been
> > changed to use GCC_VERSION macro in gcov.h, which is a lot simpler to
> > reason about. Are you happy with such arrangement?
> 
> If you mean this to be an adjustment newer than v3, then I think
> I'd be fine with that, provided you cover the full range (as indicated
> above), i.e. starting at 5.0.0.
> 

OK. The check is now like:

#if GCC_VERSION < 5
#error "Wrong version of GCC used to compile gcov"
#endif

GCC_VERSION is a convenient marco that you can find in this patch.

Then all reference to gcc 5.1 will be changed to gcc 5.

Wei.

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 13.10.16 at 10:49,  wrote:
> On Thu, Oct 13, 2016 at 02:29:08AM -0600, Jan Beulich wrote:
> [...]
>> >> >> ... this structure's trailing fields actually getting used by the code
>> >> >> won't work well when changing compiler versions without cleaning
>> >> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> >> >> #define-ing their GCOV_COUNTERS and then #include-ing this
>> >> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> >> >> development variant of 5.x) would use anything different from
>> >> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> >> >> normally be necessary anymore with gcc 5+.
>> >> >> 
>> >> > 
>> >> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
>> >> > "y" part is __GNUC_PATCHLEVEL__.
>> >> 
>> >> No, I didn't. From 5.x onwards the information previously carried in
>> >> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
>> >> as previously you would not normally need to look at the former,
>> >> with newer gcc you shouldn't need to look at the latter.
>> >> 
>> > 
>> > I can't find relevant information in GCC cpp manual.
>> > 
>> > Specifically, I look at 4.9.4 and 5.4.0 doc:
>> > 
>> > 
> https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
>  
> 
>> > on-Predefined-Macros
>> > 
> https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
>  
> 
>> > on-Predefined-Macros
>> > 
>> > The sections about __GNUC_* macros are identical, their semantics stay
>> > the same.
>> > 
>> > What did I miss?
>> 
>> Their change in how version numbers get used. I'm sure you've noticed
>> there never was a released 5.0.0 or 6.0.0, and that the stable updates
>> following 5.1.0 were 5.2.0, 5.3.0, etc.
>> 
> 
> OK. I found the bits at https://gcc.gnu.org/develop.html. I see what you
> meant previously.
> 
> It doesn't seem to be a problem to me to compare to 5.1 though -- that's
> the first release of gcc 5, which should be what people use anyway.

But your check should cover the introduction point of the feature,
which is 5.0.0 imo.

> If it is the complexity of the macro that concerns you, now it has been
> changed to use GCC_VERSION macro in gcov.h, which is a lot simpler to
> reason about. Are you happy with such arrangement?

If you mean this to be an adjustment newer than v3, then I think
I'd be fine with that, provided you cover the full range (as indicated
above), i.e. starting at 5.0.0.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Thu, Oct 13, 2016 at 02:29:08AM -0600, Jan Beulich wrote:
[...]
> >> >> ... this structure's trailing fields actually getting used by the code
> >> >> won't work well when changing compiler versions without cleaning
> >> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> >> development variant of 5.x) would use anything different from
> >> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> >> normally be necessary anymore with gcc 5+.
> >> >> 
> >> > 
> >> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> >> > "y" part is __GNUC_PATCHLEVEL__.
> >> 
> >> No, I didn't. From 5.x onwards the information previously carried in
> >> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
> >> as previously you would not normally need to look at the former,
> >> with newer gcc you shouldn't need to look at the latter.
> >> 
> > 
> > I can't find relevant information in GCC cpp manual.
> > 
> > Specifically, I look at 4.9.4 and 5.4.0 doc:
> > 
> > https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
> >  
> > on-Predefined-Macros
> > https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
> >  
> > on-Predefined-Macros
> > 
> > The sections about __GNUC_* macros are identical, their semantics stay
> > the same.
> > 
> > What did I miss?
> 
> Their change in how version numbers get used. I'm sure you've noticed
> there never was a released 5.0.0 or 6.0.0, and that the stable updates
> following 5.1.0 were 5.2.0, 5.3.0, etc.
> 

OK. I found the bits at https://gcc.gnu.org/develop.html. I see what you
meant previously.

It doesn't seem to be a problem to me to compare to 5.1 though -- that's
the first release of gcc 5, which should be what people use anyway.

If it is the complexity of the macro that concerns you, now it has been
changed to use GCC_VERSION macro in gcov.h, which is a lot simpler to
reason about. Are you happy with such arrangement?

If you feel strongly about this version comparison thing, I'm fine with
just comparing it to the major number, too.

Wei.

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 12.10.16 at 19:07,  wrote:
> On Wed, Oct 12, 2016 at 09:42:17AM -0600, Jan Beulich wrote:
>> >>> On 12.10.16 at 17:33,  wrote:
>> > On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>> >> >>> On 11.10.16 at 12:31,  wrote:
>> >> > --- /dev/null
>> >> > +++ b/xen/common/gcov/gcc_4_7.c
>> >> > @@ -0,0 +1,205 @@
>> >> > +/*
>> >> > + *  This code provides functions to handle gcc's profiling data format
>> >> > + *  introduced with gcc 4.7.
>> >> > + *
>> >> > + *  This file is based heavily on gcc_3_4.c file.
>> >> > + *
>> >> > + *  For a better understanding, refer to gcc source:
>> >> > + *  gcc/gcov-io.h
>> >> > + *  libgcc/libgcov.c
>> >> > + *
>> >> > + *  Uses gcc-internal data definitions.
>> >> > + *
>> >> > + *  Imported from Linux and modified for Xen by
>> >> > + *Wei Liu 
>> >> > + */
>> >> > +
>> >> > +#include 
>> >> > +
>> >> > +#include "gcov.h"
>> >> > +
>> >> > +#if GCC_VERSION < 40700
>> >> > +#error "Wrong version of GCC used to compile gcov"
>> >> > +#endif
>> >> > +
>> >> > +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
>> >> > +#define GCOV_COUNTERS   10
>> >> > +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
>> >> > +#define GCOV_COUNTERS   9
>> >> > +#else
>> >> > +#define GCOV_COUNTERS   8
>> >> > +#endif
>> >> 
>> >> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> >> but then didn't finish analyzing the situation), but I'm afraid this
>> >> together with ...
>> >> 
>> >> > +struct gcov_info {
>> >> > +unsigned int version;
>> >> > +struct gcov_info *next;
>> >> > +unsigned int stamp;
>> >> > +const char *filename;
>> >> > +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>> >> > +unsigned int n_functions;
>> >> > +struct gcov_fn_info **functions;
>> >> > +};
>> >> 
>> >> ... this structure's trailing fields actually getting used by the code
>> >> won't work well when changing compiler versions without cleaning
>> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> >> #define-ing their GCOV_COUNTERS and then #include-ing this
>> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> >> development variant of 5.x) would use anything different from
>> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> >> normally be necessary anymore with gcc 5+.
>> >> 
>> > 
>> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
>> > "y" part is __GNUC_PATCHLEVEL__.
>> 
>> No, I didn't. From 5.x onwards the information previously carried in
>> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
>> as previously you would not normally need to look at the former,
>> with newer gcc you shouldn't need to look at the latter.
>> 
> 
> I can't find relevant information in GCC cpp manual.
> 
> Specifically, I look at 4.9.4 and 5.4.0 doc:
> 
> https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
>  
> on-Predefined-Macros
> https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
>  
> on-Predefined-Macros
> 
> The sections about __GNUC_* macros are identical, their semantics stay
> the same.
> 
> What did I miss?

Their change in how version numbers get used. I'm sure you've noticed
there never was a released 5.0.0 or 6.0.0, and that the stable updates
following 5.1.0 were 5.2.0, 5.3.0, etc.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Martin Pohlack]

On 12.10.2016 18:21, Konrad Rzeszutek Wilk wrote:

On Wed, Oct 12, 2016 at 04:17:57PM +0200, Martin Pohlack wrote:

On 12.10.2016 15:44, Konrad Rzeszutek Wilk wrote:

On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:

On 12.10.16 at 15:23,  wrote:

 And then - how is all of this supposed to be working in conjucntion
with live patching, where the patch may have been created by yet
another compiler version?


Uh, I hope one does not create a livepatch patch with another compiler
version!

Let me put on the TODO to make livepatch-build-tools check gcc against
compile.h so that one does not try this.


What's wrong with mixing compiler versions in general?


Besides scaring me?

The one issue we had encountered was with compilers generating random named
symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
where the XYZ depends on the position of the moon along with how many
goats you have sacrificied to the altar of GCC gods.

Older compilers don't neccessarily do it, newer ones do, and every time
you build an livepatch the naming is different. Frustrating.

It maybe that newer versions of GCC are more predictable about this
naming.

Maybe Martin can share some of his experience? CC-ing him.


There are a couple of naming conventions for internal symbols and also
static symbols where you basically have to pray that gcc implementation does
not change.  Interestingly, icc has some conventions that make those symbol
names a bit more stable.

The tricky thing is matchmaking between the existing build and the new build
to construct the binary diff and to match up symbols for which you want to
provide replacement code.

We use a reproducible build environment to construct hotpatches for an
existing build in the absolutely same environment (gcc version, lib
versions, gas version, binutils etc.).  This sidesteps most of the problems.


I think the matchmaking process does not solve per say some tricky CSWTCH 
issues.

If a patch mucks with a switch statement (e.g. add a new case) we are pretty 
much
guaranteed to get in trouble. And really a change in any control structure may 
cause
gcc to take different code path, causing it to renumber CSWTCH. Or worst, 
renumber
it to the one that the hypervisor is using for some other switch statements.
I think the size of the symbol vs the one in the hypervisor is different
so one can check for this. Bad things happen if it is the same size, but bcmp
can come in handy there.


If you change a switch statement, the containing function's binary 
representation will change (+ inlining effects).  This means you will 
have to ship a new version of this function with the hotpatch.  I have 
seen gcc put jump tables for switch statements into dedicated .rodata* 
sections, at least with -ffunction-sections and -fdata-sections.  You 
need to treat those as belonging to the corresponding function and also 
ship them with the hotpatch.  If you restrict the match-making to 
function-level symbols and retain references between such a function and 
its rodata section, you should be fine.



Are there any ways to make GCC be predictable or some patches
to make GCC be less random. Perhaps instead of XYZ it would use the function 
name..


You sidestep some issues by making source-code patches as line-neutral 
as possible and introducing new symbols and definitions close to usage 
instead of in header files for hotpatches.  This reduces cascading 
effects for such renames.  Some luck, tweaking, and inlining of 
definitions is sometimes required.


Martin


P.S.
GCC scares me because the code comes in these big patches with not much 
explanation
on how it suppose to work. It probably is a piece of cake for folks who
have been marinating in compilers but for a newbie like me it is hardcore
black magic.


Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Wed, Oct 12, 2016 at 09:42:17AM -0600, Jan Beulich wrote:
> >>> On 12.10.16 at 17:33,  wrote:
> > On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >> >>> On 11.10.16 at 12:31,  wrote:
> >> > --- /dev/null
> >> > +++ b/xen/common/gcov/gcc_4_7.c
> >> > @@ -0,0 +1,205 @@
> >> > +/*
> >> > + *  This code provides functions to handle gcc's profiling data format
> >> > + *  introduced with gcc 4.7.
> >> > + *
> >> > + *  This file is based heavily on gcc_3_4.c file.
> >> > + *
> >> > + *  For a better understanding, refer to gcc source:
> >> > + *  gcc/gcov-io.h
> >> > + *  libgcc/libgcov.c
> >> > + *
> >> > + *  Uses gcc-internal data definitions.
> >> > + *
> >> > + *  Imported from Linux and modified for Xen by
> >> > + *Wei Liu 
> >> > + */
> >> > +
> >> > +#include 
> >> > +
> >> > +#include "gcov.h"
> >> > +
> >> > +#if GCC_VERSION < 40700
> >> > +#error "Wrong version of GCC used to compile gcov"
> >> > +#endif
> >> > +
> >> > +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> >> > +#define GCOV_COUNTERS   10
> >> > +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> >> > +#define GCOV_COUNTERS   9
> >> > +#else
> >> > +#define GCOV_COUNTERS   8
> >> > +#endif
> >> 
> >> I'm sorry for not having pointed this out on v2 (I had noticed it,
> >> but then didn't finish analyzing the situation), but I'm afraid this
> >> together with ...
> >> 
> >> > +struct gcov_info {
> >> > +unsigned int version;
> >> > +struct gcov_info *next;
> >> > +unsigned int stamp;
> >> > +const char *filename;
> >> > +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> >> > +unsigned int n_functions;
> >> > +struct gcov_fn_info **functions;
> >> > +};
> >> 
> >> ... this structure's trailing fields actually getting used by the code
> >> won't work well when changing compiler versions without cleaning
> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> development variant of 5.x) would use anything different from
> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> normally be necessary anymore with gcc 5+.
> >> 
> > 
> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> > "y" part is __GNUC_PATCHLEVEL__.
> 
> No, I didn't. From 5.x onwards the information previously carried in
> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
> as previously you would not normally need to look at the former,
> with newer gcc you shouldn't need to look at the latter.
> 

I can't find relevant information in GCC cpp manual.

Specifically, I look at 4.9.4 and 5.4.0 doc:

https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Common-Predefined-Macros
https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Common-Predefined-Macros

The sections about __GNUC_* macros are identical, their semantics stay
the same.

What did I miss?

> > I've broken down things into several files as well as provided
> > corresponding Kconfig options:
> > 
> >  gcc_4_7_base.c: the body of what is now gcc_4_7.c, better name is
> >  welcome
> 
> Why don't you keep it gcc_4_7.c, with its counter definition being
> conditional upon the symbol not already being defined?
> 

Fixed as discussed on IRC.

Wei.

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Konrad Rzeszutek Wilk]

On Wed, Oct 12, 2016 at 04:17:57PM +0200, Martin Pohlack wrote:
> On 12.10.2016 15:44, Konrad Rzeszutek Wilk wrote:
> > On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
> > > > > > On 12.10.16 at 15:23,  wrote:
> > > > >  And then - how is all of this supposed to be working in conjucntion
> > > > > with live patching, where the patch may have been created by yet
> > > > > another compiler version?
> > > > 
> > > > Uh, I hope one does not create a livepatch patch with another compiler
> > > > version!
> > > > 
> > > > Let me put on the TODO to make livepatch-build-tools check gcc against
> > > > compile.h so that one does not try this.
> > > 
> > > What's wrong with mixing compiler versions in general?
> > 
> > Besides scaring me?
> > 
> > The one issue we had encountered was with compilers generating random named
> > symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
> > where the XYZ depends on the position of the moon along with how many
> > goats you have sacrificied to the altar of GCC gods.
> > 
> > Older compilers don't neccessarily do it, newer ones do, and every time
> > you build an livepatch the naming is different. Frustrating.
> > 
> > It maybe that newer versions of GCC are more predictable about this
> > naming.
> > 
> > Maybe Martin can share some of his experience? CC-ing him.
> 
> There are a couple of naming conventions for internal symbols and also
> static symbols where you basically have to pray that gcc implementation does
> not change.  Interestingly, icc has some conventions that make those symbol
> names a bit more stable.
> 
> The tricky thing is matchmaking between the existing build and the new build
> to construct the binary diff and to match up symbols for which you want to
> provide replacement code.
> 
> We use a reproducible build environment to construct hotpatches for an
> existing build in the absolutely same environment (gcc version, lib
> versions, gas version, binutils etc.).  This sidesteps most of the problems.

I think the matchmaking process does not solve per say some tricky CSWTCH 
issues.

If a patch mucks with a switch statement (e.g. add a new case) we are pretty 
much
guaranteed to get in trouble. And really a change in any control structure may 
cause
gcc to take different code path, causing it to renumber CSWTCH. Or worst, 
renumber
it to the one that the hypervisor is using for some other switch statements.
I think the size of the symbol vs the one in the hypervisor is different
so one can check for this. Bad things happen if it is the same size, but bcmp
can come in handy there.

Are there any ways to make GCC be predictable or some patches
to make GCC be less random. Perhaps instead of XYZ it would use the function 
name..

P.S.
GCC scares me because the code comes in these big patches with not much 
explanation
on how it suppose to work. It probably is a piece of cake for folks who
have been marinating in compilers but for a newbie like me it is hardcore
black magic.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 12.10.16 at 17:33,  wrote:
> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>> >>> On 11.10.16 at 12:31,  wrote:
>> > --- /dev/null
>> > +++ b/xen/common/gcov/gcc_4_7.c
>> > @@ -0,0 +1,205 @@
>> > +/*
>> > + *  This code provides functions to handle gcc's profiling data format
>> > + *  introduced with gcc 4.7.
>> > + *
>> > + *  This file is based heavily on gcc_3_4.c file.
>> > + *
>> > + *  For a better understanding, refer to gcc source:
>> > + *  gcc/gcov-io.h
>> > + *  libgcc/libgcov.c
>> > + *
>> > + *  Uses gcc-internal data definitions.
>> > + *
>> > + *  Imported from Linux and modified for Xen by
>> > + *Wei Liu 
>> > + */
>> > +
>> > +#include 
>> > +
>> > +#include "gcov.h"
>> > +
>> > +#if GCC_VERSION < 40700
>> > +#error "Wrong version of GCC used to compile gcov"
>> > +#endif
>> > +
>> > +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
>> > +#define GCOV_COUNTERS   10
>> > +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
>> > +#define GCOV_COUNTERS   9
>> > +#else
>> > +#define GCOV_COUNTERS   8
>> > +#endif
>> 
>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> but then didn't finish analyzing the situation), but I'm afraid this
>> together with ...
>> 
>> > +struct gcov_info {
>> > +unsigned int version;
>> > +struct gcov_info *next;
>> > +unsigned int stamp;
>> > +const char *filename;
>> > +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>> > +unsigned int n_functions;
>> > +struct gcov_fn_info **functions;
>> > +};
>> 
>> ... this structure's trailing fields actually getting used by the code
>> won't work well when changing compiler versions without cleaning
>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> #define-ing their GCOV_COUNTERS and then #include-ing this
>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> development variant of 5.x) would use anything different from
>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> normally be necessary anymore with gcc 5+.
>> 
> 
> I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> "y" part is __GNUC_PATCHLEVEL__.

No, I didn't. From 5.x onwards the information previously carried in
__GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
as previously you would not normally need to look at the former,
with newer gcc you shouldn't need to look at the latter.

> I've broken down things into several files as well as provided
> corresponding Kconfig options:
> 
>  gcc_4_7_base.c: the body of what is now gcc_4_7.c, better name is
>  welcome

Why don't you keep it gcc_4_7.c, with its counter definition being
conditional upon the symbol not already being defined?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >>> On 11.10.16 at 12:31,  wrote:
> > --- /dev/null
> > +++ b/xen/common/gcov/gcc_4_7.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + *  This code provides functions to handle gcc's profiling data format
> > + *  introduced with gcc 4.7.
> > + *
> > + *  This file is based heavily on gcc_3_4.c file.
> > + *
> > + *  For a better understanding, refer to gcc source:
> > + *  gcc/gcov-io.h
> > + *  libgcc/libgcov.c
> > + *
> > + *  Uses gcc-internal data definitions.
> > + *
> > + *  Imported from Linux and modified for Xen by
> > + *Wei Liu 
> > + */
> > +
> > +#include 
> > +
> > +#include "gcov.h"
> > +
> > +#if GCC_VERSION < 40700
> > +#error "Wrong version of GCC used to compile gcov"
> > +#endif
> > +
> > +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> > +#define GCOV_COUNTERS   10
> > +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> > +#define GCOV_COUNTERS   9
> > +#else
> > +#define GCOV_COUNTERS   8
> > +#endif
> 
> I'm sorry for not having pointed this out on v2 (I had noticed it,
> but then didn't finish analyzing the situation), but I'm afraid this
> together with ...
> 
> > +struct gcov_info {
> > +unsigned int version;
> > +struct gcov_info *next;
> > +unsigned int stamp;
> > +const char *filename;
> > +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> > +unsigned int n_functions;
> > +struct gcov_fn_info **functions;
> > +};
> 
> ... this structure's trailing fields actually getting used by the code
> won't work well when changing compiler versions without cleaning
> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> #define-ing their GCOV_COUNTERS and then #include-ing this
> shared source file. Plus btw, I don't think gcc 5.0.x (the
> development variant of 5.x) would use anything different from
> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> normally be necessary anymore with gcc 5+.
> 

I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
"y" part is __GNUC_PATCHLEVEL__.

I've broken down things into several files as well as provided
corresponding Kconfig options:

 gcc_4_7_base.c: the body of what is now gcc_4_7.c, better name is
 welcome
 gcc_4_7.c:  gcov format in gcc version [4.7, 4,9), nr counters 8
 gcc_4_9.c:  gcov format in gcc version [4.9, 5.1), nr counters 9
 gcc_5_1.c:  gcov format in gcc version [5.1, ...), nr counters 10

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Martin Pohlack]

On 12.10.2016 15:44, Konrad Rzeszutek Wilk wrote:

On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:

On 12.10.16 at 15:23,  wrote:

 And then - how is all of this supposed to be working in conjucntion
with live patching, where the patch may have been created by yet
another compiler version?


Uh, I hope one does not create a livepatch patch with another compiler
version!

Let me put on the TODO to make livepatch-build-tools check gcc against
compile.h so that one does not try this.


What's wrong with mixing compiler versions in general?


Besides scaring me?

The one issue we had encountered was with compilers generating random named
symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
where the XYZ depends on the position of the moon along with how many
goats you have sacrificied to the altar of GCC gods.

Older compilers don't neccessarily do it, newer ones do, and every time
you build an livepatch the naming is different. Frustrating.

It maybe that newer versions of GCC are more predictable about this
naming.

Maybe Martin can share some of his experience? CC-ing him.


There are a couple of naming conventions for internal symbols and also 
static symbols where you basically have to pray that gcc implementation 
does not change.  Interestingly, icc has some conventions that make 
those symbol names a bit more stable.


The tricky thing is matchmaking between the existing build and the new 
build to construct the binary diff and to match up symbols for which you 
want to provide replacement code.


We use a reproducible build environment to construct hotpatches for an 
existing build in the absolutely same environment (gcc version, lib 
versions, gas version, binutils etc.).  This sidesteps most of the problems.


Martin
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 12.10.16 at 15:44,  wrote:
> On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
>> >>> On 12.10.16 at 15:23,  wrote:
>> >>  And then - how is all of this supposed to be working in conjucntion
>> >> with live patching, where the patch may have been created by yet
>> >> another compiler version?
>> > 
>> > Uh, I hope one does not create a livepatch patch with another compiler 
>> > version!
>> > 
>> > Let me put on the TODO to make livepatch-build-tools check gcc against
>> > compile.h so that one does not try this.
>> 
>> What's wrong with mixing compiler versions in general?
> 
> Besides scaring me?

What is it that scares you?

> The one issue we had encountered was with compilers generating random named
> symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
> where the XYZ depends on the position of the moon along with how many
> goats you have sacrificied to the altar of GCC gods.
> 
> Older compilers don't neccessarily do it, newer ones do, and every time
> you build an livepatch the naming is different. Frustrating.

But this would mean you don't just depend on gcc version, but even
on the specific build (as the numbering you refer to may change with
whatever patches a distro applies on top of the upstream tarball, as
well as perhaps with configure and build options).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[George Dunlap]

On 12/10/16 14:34, Andrew Cooper wrote:
> On 12/10/16 14:26, George Dunlap wrote:
>> On 12/10/16 14:24, George Dunlap wrote:
>>> On 12/10/16 14:06, Wei Liu wrote:
 On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
 On 11.10.16 at 12:31,  wrote:
>> --- /dev/null
>> +++ b/xen/common/gcov/gcc_4_7.c
>> @@ -0,0 +1,205 @@
>> +/*
>> + *  This code provides functions to handle gcc's profiling data format
>> + *  introduced with gcc 4.7.
>> + *
>> + *  This file is based heavily on gcc_3_4.c file.
>> + *
>> + *  For a better understanding, refer to gcc source:
>> + *  gcc/gcov-io.h
>> + *  libgcc/libgcov.c
>> + *
>> + *  Uses gcc-internal data definitions.
>> + *
>> + *  Imported from Linux and modified for Xen by
>> + *Wei Liu 
>> + */
>> +
>> +#include 
>> +
>> +#include "gcov.h"
>> +
>> +#if GCC_VERSION < 40700
>> +#error "Wrong version of GCC used to compile gcov"
>> +#endif
>> +
>> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
>> +#define GCOV_COUNTERS   10
>> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
>> +#define GCOV_COUNTERS   9
>> +#else
>> +#define GCOV_COUNTERS   8
>> +#endif
> I'm sorry for not having pointed this out on v2 (I had noticed it,
> but then didn't finish analyzing the situation), but I'm afraid this
> together with ...
>
>> +struct gcov_info {
>> +unsigned int version;
>> +struct gcov_info *next;
>> +unsigned int stamp;
>> +const char *filename;
>> +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>> +unsigned int n_functions;
>> +struct gcov_fn_info **functions;
>> +};
> ... this structure's trailing fields actually getting used by the code
> won't work well when changing compiler versions without cleaning
> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
> #define-ing their GCOV_COUNTERS and then #include-ing this
> shared source file. Plus btw, I don't think gcc 5.0.x (the
> development variant of 5.x) would use anything different from
> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> normally be necessary anymore with gcc 5+.
>
 Right. I will do something about this. Thanks for catching this.

> And then - how is all of this supposed to be working in conjucntion
> with live patching, where the patch may have been created by yet
> another compiler version?
>
 There is a version field in gcov_info, so we can compare that and reject
 incompatible version.

 We need to use hooks in livepatching to call the constructor /
 destructor when applying / reverting a live-patch.  We might need to be
 cautious about locks or whatever, but I'm sure it can be figured out.

 But I have no idea how useful it would be to use gcov and livepatching
 together.  For now the easiest thing to do is to

depends on !LIVEPATCH

 in Kconfig.
>>> Wouldn't it be just as easy, and more useful, to set a "has been
>>> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>>>
>>> I would expect most users to want to build a single hypervisor that can
>>> be used for both gcov testing and live patching (under different
>>> circumstances).
>> I mean software provider, not user, of course.  That's what I would want
>> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
>> guys want as well.
> 
> GCOV is majority invasive, both in terms of performance (every basic
> block needs to do a locked increment of a counter) and data size
> (metadata for all basic blocks).
> 
> No software vendor is ever going to have it enabled in a production
> scenario.

You're right, I wasn't thinking.

But also presumably you'd like to be able to minimize the difference
between the thing you're testing and the thing you ship; having to
disable LivePatch increases the delta slightly.

Anyway, I still think having them both Kconfig-ured is a good idea, but
not so much that I'd spend any more time arguing for it.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Wed, Oct 12, 2016 at 09:46:51AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 12, 2016 at 02:40:08PM +0100, Wei Liu wrote:
> > On Wed, Oct 12, 2016 at 09:29:04AM -0400, Konrad Rzeszutek Wilk wrote:
> > [...]
> > > > 
> > > > Wouldn't it be just as easy, and more useful, to set a "has been
> > > > livepatched" flag, and return errors for all gcov hypercalls if its' 
> > > > set?
> > > > 
> > > > I would expect most users to want to build a single hypervisor that can
> > > > be used for both gcov testing and live patching (under different
> > > > circumstances).
> > > 
> > > I actually would welcome livepatching and gcov to see if the test-cases I 
> > > wrote
> > > cover most of the code.
> > > 
> > 
> > I don't follow. Gcov doesn't give you a call graph -- if that's what
> > you're after.
> 
> It gives me an idea which functions/branches have run (not the livepatch
> itself - just the infrastructure around adding a livepatch, doing ELF checks, 
> etc).
> 
> And more importantly - which ones haven't run and need some more test-cases.
> 

Right. Thanks for the explanation. That would indeed be useful.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Konrad Rzeszutek Wilk]

On Wed, Oct 12, 2016 at 02:40:08PM +0100, Wei Liu wrote:
> On Wed, Oct 12, 2016 at 09:29:04AM -0400, Konrad Rzeszutek Wilk wrote:
> [...]
> > > 
> > > Wouldn't it be just as easy, and more useful, to set a "has been
> > > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> > > 
> > > I would expect most users to want to build a single hypervisor that can
> > > be used for both gcov testing and live patching (under different
> > > circumstances).
> > 
> > I actually would welcome livepatching and gcov to see if the test-cases I 
> > wrote
> > cover most of the code.
> > 
> 
> I don't follow. Gcov doesn't give you a call graph -- if that's what
> you're after.

It gives me an idea which functions/branches have run (not the livepatch
itself - just the infrastructure around adding a livepatch, doing ELF checks, 
etc).

And more importantly - which ones haven't run and need some more test-cases.

> 
> > Adding in checking livepatch (common/livepatch.c: prepare_payload) to 
> > examine
> > the .gcov_info and see if it matches the hypervisor one, is fine too.
> > 
> 
> This then involves a non-trivial amount of work to figure out all the
> corner cases. It's better to defer that to a later stage.

Sure thing. And the !LIVEPATCH is fine for now. I just need to get an idea
of what this would entail so it can get done.
> 
> Wei.
> 
> > > 
> > >  -George
> > > 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Konrad Rzeszutek Wilk]

On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
> >>> On 12.10.16 at 15:23,  wrote:
> >>  And then - how is all of this supposed to be working in conjucntion
> >> with live patching, where the patch may have been created by yet
> >> another compiler version?
> > 
> > Uh, I hope one does not create a livepatch patch with another compiler 
> > version!
> > 
> > Let me put on the TODO to make livepatch-build-tools check gcc against
> > compile.h so that one does not try this.
> 
> What's wrong with mixing compiler versions in general?

Besides scaring me?

The one issue we had encountered was with compilers generating random named
symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
where the XYZ depends on the position of the moon along with how many
goats you have sacrificied to the altar of GCC gods.

Older compilers don't neccessarily do it, newer ones do, and every time
you build an livepatch the naming is different. Frustrating.

It maybe that newer versions of GCC are more predictable about this
naming.

Maybe Martin can share some of his experience? CC-ing him.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Andrew Cooper]

On 12/10/16 14:41, George Dunlap wrote:
> On 12/10/16 14:34, Andrew Cooper wrote:
>> On 12/10/16 14:26, George Dunlap wrote:
>>> On 12/10/16 14:24, George Dunlap wrote:
 On 12/10/16 14:06, Wei Liu wrote:
> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> On 11.10.16 at 12:31,  wrote:
>>> --- /dev/null
>>> +++ b/xen/common/gcov/gcc_4_7.c
>>> @@ -0,0 +1,205 @@
>>> +/*
>>> + *  This code provides functions to handle gcc's profiling data format
>>> + *  introduced with gcc 4.7.
>>> + *
>>> + *  This file is based heavily on gcc_3_4.c file.
>>> + *
>>> + *  For a better understanding, refer to gcc source:
>>> + *  gcc/gcov-io.h
>>> + *  libgcc/libgcov.c
>>> + *
>>> + *  Uses gcc-internal data definitions.
>>> + *
>>> + *  Imported from Linux and modified for Xen by
>>> + *Wei Liu 
>>> + */
>>> +
>>> +#include 
>>> +
>>> +#include "gcov.h"
>>> +
>>> +#if GCC_VERSION < 40700
>>> +#error "Wrong version of GCC used to compile gcov"
>>> +#endif
>>> +
>>> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
>>> +#define GCOV_COUNTERS   10
>>> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
>>> +#define GCOV_COUNTERS   9
>>> +#else
>>> +#define GCOV_COUNTERS   8
>>> +#endif
>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> but then didn't finish analyzing the situation), but I'm afraid this
>> together with ...
>>
>>> +struct gcov_info {
>>> +unsigned int version;
>>> +struct gcov_info *next;
>>> +unsigned int stamp;
>>> +const char *filename;
>>> +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>> +unsigned int n_functions;
>>> +struct gcov_fn_info **functions;
>>> +};
>> ... this structure's trailing fields actually getting used by the code
>> won't work well when changing compiler versions without cleaning
>> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
>> #define-ing their GCOV_COUNTERS and then #include-ing this
>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> development variant of 5.x) would use anything different from
>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> normally be necessary anymore with gcc 5+.
>>
> Right. I will do something about this. Thanks for catching this.
>
>> And then - how is all of this supposed to be working in conjucntion
>> with live patching, where the patch may have been created by yet
>> another compiler version?
>>
> There is a version field in gcov_info, so we can compare that and reject
> incompatible version.
>
> We need to use hooks in livepatching to call the constructor /
> destructor when applying / reverting a live-patch.  We might need to be
> cautious about locks or whatever, but I'm sure it can be figured out.
>
> But I have no idea how useful it would be to use gcov and livepatching
> together.  For now the easiest thing to do is to
>
>depends on !LIVEPATCH
>
> in Kconfig.
 Wouldn't it be just as easy, and more useful, to set a "has been
 livepatched" flag, and return errors for all gcov hypercalls if its' set?

 I would expect most users to want to build a single hypervisor that can
 be used for both gcov testing and live patching (under different
 circumstances).
>>> I mean software provider, not user, of course.  That's what I would want
>>> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
>>> guys want as well.
>> GCOV is majority invasive, both in terms of performance (every basic
>> block needs to do a locked increment of a counter) and data size
>> (metadata for all basic blocks).
>>
>> No software vendor is ever going to have it enabled in a production
>> scenario.
> You're right, I wasn't thinking.
>
> But also presumably you'd like to be able to minimize the difference
> between the thing you're testing and the thing you ship; having to
> disable LivePatch increases the delta slightly.
>
> Anyway, I still think having them both Kconfig-ured is a good idea, but
> not so much that I'd spend any more time arguing for it.

It is a testing feature.  It would certainly be nice to get code
coverage of the livepatching parts, but that absolutely shouldn't block
making gcov usable in the first place.

It might be worth sticking a #TODO make GCOV worth with Livepatching in
the kconfig file (for anyone who stumbles across this dependency).

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Wed, Oct 12, 2016 at 09:29:04AM -0400, Konrad Rzeszutek Wilk wrote:
[...]
> > 
> > Wouldn't it be just as easy, and more useful, to set a "has been
> > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> > 
> > I would expect most users to want to build a single hypervisor that can
> > be used for both gcov testing and live patching (under different
> > circumstances).
> 
> I actually would welcome livepatching and gcov to see if the test-cases I 
> wrote
> cover most of the code.
> 

I don't follow. Gcov doesn't give you a call graph -- if that's what
you're after.

> Adding in checking livepatch (common/livepatch.c: prepare_payload) to examine
> the .gcov_info and see if it matches the hypervisor one, is fine too.
> 

This then involves a non-trivial amount of work to figure out all the
corner cases. It's better to defer that to a later stage.

Wei.

> > 
> >  -George
> > 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Andrew Cooper]

On 12/10/16 14:26, George Dunlap wrote:
> On 12/10/16 14:24, George Dunlap wrote:
>> On 12/10/16 14:06, Wei Liu wrote:
>>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>> On 11.10.16 at 12:31,  wrote:
> --- /dev/null
> +++ b/xen/common/gcov/gcc_4_7.c
> @@ -0,0 +1,205 @@
> +/*
> + *  This code provides functions to handle gcc's profiling data format
> + *  introduced with gcc 4.7.
> + *
> + *  This file is based heavily on gcc_3_4.c file.
> + *
> + *  For a better understanding, refer to gcc source:
> + *  gcc/gcov-io.h
> + *  libgcc/libgcov.c
> + *
> + *  Uses gcc-internal data definitions.
> + *
> + *  Imported from Linux and modified for Xen by
> + *Wei Liu 
> + */
> +
> +#include 
> +
> +#include "gcov.h"
> +
> +#if GCC_VERSION < 40700
> +#error "Wrong version of GCC used to compile gcov"
> +#endif
> +
> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> +#define GCOV_COUNTERS   10
> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> +#define GCOV_COUNTERS   9
> +#else
> +#define GCOV_COUNTERS   8
> +#endif
 I'm sorry for not having pointed this out on v2 (I had noticed it,
 but then didn't finish analyzing the situation), but I'm afraid this
 together with ...

> +struct gcov_info {
> +unsigned int version;
> +struct gcov_info *next;
> +unsigned int stamp;
> +const char *filename;
> +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> +unsigned int n_functions;
> +struct gcov_fn_info **functions;
> +};
 ... this structure's trailing fields actually getting used by the code
 won't work well when changing compiler versions without cleaning
 the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
 #define-ing their GCOV_COUNTERS and then #include-ing this
 shared source file. Plus btw, I don't think gcc 5.0.x (the
 development variant of 5.x) would use anything different from
 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
 normally be necessary anymore with gcc 5+.

>>> Right. I will do something about this. Thanks for catching this.
>>>
 And then - how is all of this supposed to be working in conjucntion
 with live patching, where the patch may have been created by yet
 another compiler version?

>>> There is a version field in gcov_info, so we can compare that and reject
>>> incompatible version.
>>>
>>> We need to use hooks in livepatching to call the constructor /
>>> destructor when applying / reverting a live-patch.  We might need to be
>>> cautious about locks or whatever, but I'm sure it can be figured out.
>>>
>>> But I have no idea how useful it would be to use gcov and livepatching
>>> together.  For now the easiest thing to do is to
>>>
>>>depends on !LIVEPATCH
>>>
>>> in Kconfig.
>> Wouldn't it be just as easy, and more useful, to set a "has been
>> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>>
>> I would expect most users to want to build a single hypervisor that can
>> be used for both gcov testing and live patching (under different
>> circumstances).
> I mean software provider, not user, of course.  That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.

GCOV is majority invasive, both in terms of performance (every basic
block needs to do a locked increment of a counter) and data size
(metadata for all basic blocks).

No software vendor is ever going to have it enabled in a production
scenario.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 12.10.16 at 15:26,  wrote:
> On 12/10/16 14:24, George Dunlap wrote:
>> I would expect most users to want to build a single hypervisor that can
>> be used for both gcov testing and live patching (under different
>> circumstances).
> 
> I mean software provider, not user, of course.  That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.

But gcov is explicitly a non-production feature, iirc.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Wed, Oct 12, 2016 at 02:26:26PM +0100, George Dunlap wrote:
[...]
> >>
> >> There is a version field in gcov_info, so we can compare that and reject
> >> incompatible version.
> >>
> >> We need to use hooks in livepatching to call the constructor /
> >> destructor when applying / reverting a live-patch.  We might need to be
> >> cautious about locks or whatever, but I'm sure it can be figured out.
> >>
> >> But I have no idea how useful it would be to use gcov and livepatching
> >> together.  For now the easiest thing to do is to
> >>
> >>depends on !LIVEPATCH
> >>
> >> in Kconfig.
> > 
> > Wouldn't it be just as easy, and more useful, to set a "has been
> > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> > 
> > I would expect most users to want to build a single hypervisor that can
> > be used for both gcov testing and live patching (under different
> > circumstances).
> 
> I mean software provider, not user, of course.  That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.
> 

The usefulness of such build is not as big as you might think I'm
afraid -- it wouldn't be useful until we figure out how to apply a
livepatch generated with gcov support built in.

Wei.

>  -George
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 12.10.16 at 15:23,  wrote:
>>  And then - how is all of this supposed to be working in conjucntion
>> with live patching, where the patch may have been created by yet
>> another compiler version?
> 
> Uh, I hope one does not create a livepatch patch with another compiler 
> version!
> 
> Let me put on the TODO to make livepatch-build-tools check gcc against
> compile.h so that one does not try this.

What's wrong with mixing compiler versions in general?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Konrad Rzeszutek Wilk]

On Wed, Oct 12, 2016 at 02:24:51PM +0100, George Dunlap wrote:
> On 12/10/16 14:06, Wei Liu wrote:
> > On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> > On 11.10.16 at 12:31,  wrote:
> >>> --- /dev/null
> >>> +++ b/xen/common/gcov/gcc_4_7.c
> >>> @@ -0,0 +1,205 @@
> >>> +/*
> >>> + *  This code provides functions to handle gcc's profiling data format
> >>> + *  introduced with gcc 4.7.
> >>> + *
> >>> + *  This file is based heavily on gcc_3_4.c file.
> >>> + *
> >>> + *  For a better understanding, refer to gcc source:
> >>> + *  gcc/gcov-io.h
> >>> + *  libgcc/libgcov.c
> >>> + *
> >>> + *  Uses gcc-internal data definitions.
> >>> + *
> >>> + *  Imported from Linux and modified for Xen by
> >>> + *Wei Liu 
> >>> + */
> >>> +
> >>> +#include 
> >>> +
> >>> +#include "gcov.h"
> >>> +
> >>> +#if GCC_VERSION < 40700
> >>> +#error "Wrong version of GCC used to compile gcov"
> >>> +#endif
> >>> +
> >>> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> >>> +#define GCOV_COUNTERS   10
> >>> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> >>> +#define GCOV_COUNTERS   9
> >>> +#else
> >>> +#define GCOV_COUNTERS   8
> >>> +#endif
> >>
> >> I'm sorry for not having pointed this out on v2 (I had noticed it,
> >> but then didn't finish analyzing the situation), but I'm afraid this
> >> together with ...
> >>
> >>> +struct gcov_info {
> >>> +unsigned int version;
> >>> +struct gcov_info *next;
> >>> +unsigned int stamp;
> >>> +const char *filename;
> >>> +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> >>> +unsigned int n_functions;
> >>> +struct gcov_fn_info **functions;
> >>> +};
> >>
> >> ... this structure's trailing fields actually getting used by the code
> >> won't work well when changing compiler versions without cleaning
> >> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> development variant of 5.x) would use anything different from
> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> normally be necessary anymore with gcc 5+.
> >>
> > 
> > Right. I will do something about this. Thanks for catching this.
> > 
> >> And then - how is all of this supposed to be working in conjucntion
> >> with live patching, where the patch may have been created by yet
> >> another compiler version?
> >>
> > 
> > There is a version field in gcov_info, so we can compare that and reject
> > incompatible version.
> > 
> > We need to use hooks in livepatching to call the constructor /
> > destructor when applying / reverting a live-patch.  We might need to be
> > cautious about locks or whatever, but I'm sure it can be figured out.
> > 
> > But I have no idea how useful it would be to use gcov and livepatching
> > together.  For now the easiest thing to do is to
> > 
> >depends on !LIVEPATCH
> > 
> > in Kconfig.
> 
> Wouldn't it be just as easy, and more useful, to set a "has been
> livepatched" flag, and return errors for all gcov hypercalls if its' set?
> 
> I would expect most users to want to build a single hypervisor that can
> be used for both gcov testing and live patching (under different
> circumstances).

I actually would welcome livepatching and gcov to see if the test-cases I wrote
cover most of the code.

Adding in checking livepatch (common/livepatch.c: prepare_payload) to examine
the .gcov_info and see if it matches the hypervisor one, is fine too.

> 
>  -George
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[George Dunlap]

On 12/10/16 14:24, George Dunlap wrote:
> On 12/10/16 14:06, Wei Liu wrote:
>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>> On 11.10.16 at 12:31,  wrote:
 --- /dev/null
 +++ b/xen/common/gcov/gcc_4_7.c
 @@ -0,0 +1,205 @@
 +/*
 + *  This code provides functions to handle gcc's profiling data format
 + *  introduced with gcc 4.7.
 + *
 + *  This file is based heavily on gcc_3_4.c file.
 + *
 + *  For a better understanding, refer to gcc source:
 + *  gcc/gcov-io.h
 + *  libgcc/libgcov.c
 + *
 + *  Uses gcc-internal data definitions.
 + *
 + *  Imported from Linux and modified for Xen by
 + *Wei Liu 
 + */
 +
 +#include 
 +
 +#include "gcov.h"
 +
 +#if GCC_VERSION < 40700
 +#error "Wrong version of GCC used to compile gcov"
 +#endif
 +
 +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
 +#define GCOV_COUNTERS   10
 +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
 +#define GCOV_COUNTERS   9
 +#else
 +#define GCOV_COUNTERS   8
 +#endif
>>>
>>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>>> but then didn't finish analyzing the situation), but I'm afraid this
>>> together with ...
>>>
 +struct gcov_info {
 +unsigned int version;
 +struct gcov_info *next;
 +unsigned int stamp;
 +const char *filename;
 +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
 +unsigned int n_functions;
 +struct gcov_fn_info **functions;
 +};
>>>
>>> ... this structure's trailing fields actually getting used by the code
>>> won't work well when changing compiler versions without cleaning
>>> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
>>> #define-ing their GCOV_COUNTERS and then #include-ing this
>>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>>> development variant of 5.x) would use anything different from
>>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>>> normally be necessary anymore with gcc 5+.
>>>
>>
>> Right. I will do something about this. Thanks for catching this.
>>
>>> And then - how is all of this supposed to be working in conjucntion
>>> with live patching, where the patch may have been created by yet
>>> another compiler version?
>>>
>>
>> There is a version field in gcov_info, so we can compare that and reject
>> incompatible version.
>>
>> We need to use hooks in livepatching to call the constructor /
>> destructor when applying / reverting a live-patch.  We might need to be
>> cautious about locks or whatever, but I'm sure it can be figured out.
>>
>> But I have no idea how useful it would be to use gcov and livepatching
>> together.  For now the easiest thing to do is to
>>
>>depends on !LIVEPATCH
>>
>> in Kconfig.
> 
> Wouldn't it be just as easy, and more useful, to set a "has been
> livepatched" flag, and return errors for all gcov hypercalls if its' set?
> 
> I would expect most users to want to build a single hypervisor that can
> be used for both gcov testing and live patching (under different
> circumstances).

I mean software provider, not user, of course.  That's what I would want
for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
guys want as well.

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[George Dunlap]

On 12/10/16 14:06, Wei Liu wrote:
> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> On 11.10.16 at 12:31,  wrote:
>>> --- /dev/null
>>> +++ b/xen/common/gcov/gcc_4_7.c
>>> @@ -0,0 +1,205 @@
>>> +/*
>>> + *  This code provides functions to handle gcc's profiling data format
>>> + *  introduced with gcc 4.7.
>>> + *
>>> + *  This file is based heavily on gcc_3_4.c file.
>>> + *
>>> + *  For a better understanding, refer to gcc source:
>>> + *  gcc/gcov-io.h
>>> + *  libgcc/libgcov.c
>>> + *
>>> + *  Uses gcc-internal data definitions.
>>> + *
>>> + *  Imported from Linux and modified for Xen by
>>> + *Wei Liu 
>>> + */
>>> +
>>> +#include 
>>> +
>>> +#include "gcov.h"
>>> +
>>> +#if GCC_VERSION < 40700
>>> +#error "Wrong version of GCC used to compile gcov"
>>> +#endif
>>> +
>>> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
>>> +#define GCOV_COUNTERS   10
>>> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
>>> +#define GCOV_COUNTERS   9
>>> +#else
>>> +#define GCOV_COUNTERS   8
>>> +#endif
>>
>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> but then didn't finish analyzing the situation), but I'm afraid this
>> together with ...
>>
>>> +struct gcov_info {
>>> +unsigned int version;
>>> +struct gcov_info *next;
>>> +unsigned int stamp;
>>> +const char *filename;
>>> +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>> +unsigned int n_functions;
>>> +struct gcov_fn_info **functions;
>>> +};
>>
>> ... this structure's trailing fields actually getting used by the code
>> won't work well when changing compiler versions without cleaning
>> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
>> #define-ing their GCOV_COUNTERS and then #include-ing this
>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> development variant of 5.x) would use anything different from
>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> normally be necessary anymore with gcc 5+.
>>
> 
> Right. I will do something about this. Thanks for catching this.
> 
>> And then - how is all of this supposed to be working in conjucntion
>> with live patching, where the patch may have been created by yet
>> another compiler version?
>>
> 
> There is a version field in gcov_info, so we can compare that and reject
> incompatible version.
> 
> We need to use hooks in livepatching to call the constructor /
> destructor when applying / reverting a live-patch.  We might need to be
> cautious about locks or whatever, but I'm sure it can be figured out.
> 
> But I have no idea how useful it would be to use gcov and livepatching
> together.  For now the easiest thing to do is to
> 
>depends on !LIVEPATCH
> 
> in Kconfig.

Wouldn't it be just as easy, and more useful, to set a "has been
livepatched" flag, and return errors for all gcov hypercalls if its' set?

I would expect most users to want to build a single hypervisor that can
be used for both gcov testing and live patching (under different
circumstances).

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Konrad Rzeszutek Wilk]

> And then - how is all of this supposed to be working in conjucntion
> with live patching, where the patch may have been created by yet
> another compiler version?

Uh, I hope one does not create a livepatch patch with another compiler version!

Let me put on the TODO to make livepatch-build-tools check gcc against
compile.h so that one does not try this.
> 
> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 12.10.16 at 15:06,  wrote:
> But I have no idea how useful it would be to use gcov and livepatching
> together.  For now the easiest thing to do is to
> 
>depends on !LIVEPATCH
> 
> in Kconfig.

I agree.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >>> On 11.10.16 at 12:31,  wrote:
> > --- /dev/null
> > +++ b/xen/common/gcov/gcc_4_7.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + *  This code provides functions to handle gcc's profiling data format
> > + *  introduced with gcc 4.7.
> > + *
> > + *  This file is based heavily on gcc_3_4.c file.
> > + *
> > + *  For a better understanding, refer to gcc source:
> > + *  gcc/gcov-io.h
> > + *  libgcc/libgcov.c
> > + *
> > + *  Uses gcc-internal data definitions.
> > + *
> > + *  Imported from Linux and modified for Xen by
> > + *Wei Liu 
> > + */
> > +
> > +#include 
> > +
> > +#include "gcov.h"
> > +
> > +#if GCC_VERSION < 40700
> > +#error "Wrong version of GCC used to compile gcov"
> > +#endif
> > +
> > +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> > +#define GCOV_COUNTERS   10
> > +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> > +#define GCOV_COUNTERS   9
> > +#else
> > +#define GCOV_COUNTERS   8
> > +#endif
> 
> I'm sorry for not having pointed this out on v2 (I had noticed it,
> but then didn't finish analyzing the situation), but I'm afraid this
> together with ...
> 
> > +struct gcov_info {
> > +unsigned int version;
> > +struct gcov_info *next;
> > +unsigned int stamp;
> > +const char *filename;
> > +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> > +unsigned int n_functions;
> > +struct gcov_fn_info **functions;
> > +};
> 
> ... this structure's trailing fields actually getting used by the code
> won't work well when changing compiler versions without cleaning
> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
> #define-ing their GCOV_COUNTERS and then #include-ing this
> shared source file. Plus btw, I don't think gcc 5.0.x (the
> development variant of 5.x) would use anything different from
> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> normally be necessary anymore with gcc 5+.
> 

Right. I will do something about this. Thanks for catching this.

> And then - how is all of this supposed to be working in conjucntion
> with live patching, where the patch may have been created by yet
> another compiler version?
> 

There is a version field in gcov_info, so we can compare that and reject
incompatible version.

We need to use hooks in livepatching to call the constructor /
destructor when applying / reverting a live-patch.  We might need to be
cautious about locks or whatever, but I'm sure it can be figured out.

But I have no idea how useful it would be to use gcov and livepatching
together.  For now the easiest thing to do is to

   depends on !LIVEPATCH

in Kconfig.

Wei.

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Jan Beulich]

>>> On 11.10.16 at 12:31,  wrote:
> --- /dev/null
> +++ b/xen/common/gcov/gcc_4_7.c
> @@ -0,0 +1,205 @@
> +/*
> + *  This code provides functions to handle gcc's profiling data format
> + *  introduced with gcc 4.7.
> + *
> + *  This file is based heavily on gcc_3_4.c file.
> + *
> + *  For a better understanding, refer to gcc source:
> + *  gcc/gcov-io.h
> + *  libgcc/libgcov.c
> + *
> + *  Uses gcc-internal data definitions.
> + *
> + *  Imported from Linux and modified for Xen by
> + *Wei Liu 
> + */
> +
> +#include 
> +
> +#include "gcov.h"
> +
> +#if GCC_VERSION < 40700
> +#error "Wrong version of GCC used to compile gcov"
> +#endif
> +
> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> +#define GCOV_COUNTERS   10
> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> +#define GCOV_COUNTERS   9
> +#else
> +#define GCOV_COUNTERS   8
> +#endif

I'm sorry for not having pointed this out on v2 (I had noticed it,
but then didn't finish analyzing the situation), but I'm afraid this
together with ...

> +struct gcov_info {
> +unsigned int version;
> +struct gcov_info *next;
> +unsigned int stamp;
> +const char *filename;
> +void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> +unsigned int n_functions;
> +struct gcov_fn_info **functions;
> +};

... this structure's trailing fields actually getting used by the code
won't work well when changing compiler versions without cleaning
the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
#define-ing their GCOV_COUNTERS and then #include-ing this
shared source file. Plus btw, I don't think gcc 5.0.x (the
development variant of 5.x) would use anything different from
5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
normally be necessary anymore with gcc 5+.

And then - how is all of this supposed to be working in conjucntion
with live patching, where the patch may have been created by yet
another compiler version?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu]

A new sysctl interface for passing gcov data back to userspace. The new
interface uses a customised record file format. The new sysctl reuses
original sysctl number but renames the op to gcov_op.

Both gcc 3.4 and 4.7 format are supported. The code is rewritten so that
a new format can be easily added in the future.  Version specific code
is grouped into different files. The format one needs to use can be
picked via Kconfig. The default format is 4.7 format.

Userspace programs to handle extracted data will come in a later patch.

Signed-off-by: Wei Liu 
---
As always, v3 in my xenbits xen.git tree. Only sending this unacked
patch.

v3:
1. Check gcc version in gcc_*.c files.
2. Eliminate u32 / u64.
3. Mark __gcov_init as __init.
4. Fix a rebase error in v2.
5. Some other cosmetic fixes.

v2:
1. Use tab in Kconfig and indent help text properly.
2. Bump XEN_SYSCTL_INTERFACE_VERSION.
3. Fix cosmetic issues.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 xen/Kconfig.debug   |  24 ++-
 xen/common/gcov/Makefile|   3 +
 xen/common/gcov/gcc_3_4.c   | 367 
 xen/common/gcov/gcc_4_7.c   | 205 +
 xen/common/gcov/gcov.c  | 257 +++
 xen/common/gcov/gcov.h  |  40 +
 xen/common/gcov/gcov_base.c |  60 
 xen/common/sysctl.c |   8 +
 xen/include/public/sysctl.h |  39 -
 xen/include/xen/gcov.h  |   9 ++
 10 files changed, 1005 insertions(+), 7 deletions(-)
 create mode 100644 xen/common/gcov/gcc_3_4.c
 create mode 100644 xen/common/gcov/gcc_4_7.c
 create mode 100644 xen/common/gcov/gcov.c
 create mode 100644 xen/common/gcov/gcov.h
 create mode 100644 xen/common/gcov/gcov_base.c
 create mode 100644 xen/include/xen/gcov.h

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index ef863dc..c65e543 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -30,16 +30,30 @@ config FRAME_POINTER
 
 config GCOV
bool "Gcov Support"
-   depends on BROKEN
---help---
  Enable gcov (a test coverage program in GCC) support.
 
- Currently the data structure and hypercall interface are tied
- to GCC 3.4 gcov format. You need to have a version of GCC
- that is compatible with that format to make gcov work.
-
  If unsure, say N here.
 
+choice
+   prompt "Specify Gcov format"
+   depends on GCOV
+   default GCOV_FORMAT_4_7
+   ---help---
+ The gcov format is determined by gcc version.
+
+config GCOV_FORMAT_4_7
+   bool "GCC 4.7 format"
+   ---help---
+ Select this option to use the format specified in GCC 4.7.
+
+config GCOV_FORMAT_3_4
+   bool "GCC 3.4 format"
+   ---help---
+ Select this option to use the format specified in GCC 3.4.
+
+endchoice
+
 config LOCK_PROFILE
bool "Lock Profiling"
---help---
diff --git a/xen/common/gcov/Makefile b/xen/common/gcov/Makefile
index e69de29..03ac1e5 100644
--- a/xen/common/gcov/Makefile
+++ b/xen/common/gcov/Makefile
@@ -0,0 +1,3 @@
+obj-y += gcov_base.o gcov.o
+obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o
+obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o
diff --git a/xen/common/gcov/gcc_3_4.c b/xen/common/gcov/gcc_3_4.c
new file mode 100644
index 000..3631f4b
--- /dev/null
+++ b/xen/common/gcov/gcc_3_4.c
@@ -0,0 +1,367 @@
+/*
+ *  This code provides functions to handle gcc's profiling data format
+ *  introduced with gcc 3.4. Future versions of gcc may change the gcov
+ *  format (as happened before), so all format-specific information needs
+ *  to be kept modular and easily exchangeable.
+ *
+ *  This file is based on gcc-internal definitions. Functions and data
+ *  structures are defined to be compatible with gcc counterparts.
+ *  For a better understanding, refer to gcc source: gcc/gcov-io.h.
+ *
+ *Copyright IBM Corp. 2009
+ *Author(s): Peter Oberparleiter 
+ *
+ *Uses gcc-internal data definitions.
+ *
+ *  Imported from Linux and modified for Xen by
+ *Wei Liu 
+ */
+
+
+#include 
+
+#include "gcov.h"
+
+#if !(GCC_VERSION >= 30400 && GCC_VERSION < 40700)
+#error "Wrong version of GCC used to compile gcov"
+#endif
+
+#define GCOV_COUNTERS 5
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @ident: object file-unique function identifier
+ * @checksum: function checksum
+ * @n_ctrs: number of values per counter type belonging to this function
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ */
+struct gcov_fn_info
+{
+unsigned int ident;
+