Re: [Xen-devel] [PATCH] coverage: fix on ARM

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-02-28 at 10:00 +0000, Frediano Ziglio wrote:
> On Wed, 2013-02-27 at 13:17 +0000, Ian Campbell wrote:
> > On Fri, 2013-02-22 at 10:57 +0000, Ian Campbell wrote:
> > > Use a list of pointers to simplify the handling of 32- vs 64-bit.
> > > 
> > > Also on ARM the section name is ".init_array" and not ".ctors".
> > 
> > Any comments/ack/nacks?
> > 
> 
> Looks fine to me. Did you test it?

I tested on x86_64 and arm32/64 by annotating the init function to print
the function names as it called them and observing that a) a bunch of
functions were called (on x86 it looked similar before and after this
change) and b) it didn't crash.

>  Actually the way I test is:
> - compile with coverage
> - run xencov to extract information (more or less for x64 is 500k)
> - check file content to see if counters increase while taking multiple
> snapshot (xencov read - | hexdump -C | tail -100, counters are after
> 0CTX string which means "Xen Test Coverage count 0")
> - reset counters (but this has nothing to do with constructors)
> 
> I actually cannot test for Arm.
> 
> Oh.. there is a missing space after a "for("

So there is, thanks for spotting. A resend will have to wait until after
my flight to Hong Kong or else whoever applies, which may end up being
me, can fix it as they go.

Cheers,
Ian.

> 
> Frediano
> 
> > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > Cc: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> > > Cc: keir@xxxxxxx
> > > ---
> > > This applies independently of my 64-bit ARM series.
> > > ---
> > >  xen/arch/arm/xen.lds.S |   10 ++++------
> > >  xen/arch/x86/xen.lds.S |    6 ++----
> > >  xen/common/lib.c       |   13 +++++--------
> > >  3 files changed, 11 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> > > index 9043994..fd755d7 100644
> > > --- a/xen/arch/arm/xen.lds.S
> > > +++ b/xen/arch/arm/xen.lds.S
> > > @@ -91,12 +91,10 @@ SECTIONS
> > >         *(.init.data.rel)
> > >         *(.init.data.rel.*)
> > >  
> > > -       . = ALIGN(4);
> > > -       __CTOR_LIST__ = .;
> > > -       LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2)
> > > -       *(.ctors)
> > > -       LONG(0)
> > > -       __CTOR_END__ = .;
> > > +       . = ALIGN(8);
> > > +       __ctors_start = .;
> > > +       *(.init_array)
> > > +       __ctors_end = .;
> > >    } :text
> > >    . = ALIGN(32);
> > >    .init.setup : {
> > > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> > > index 5570389..d959941 100644
> > > --- a/xen/arch/x86/xen.lds.S
> > > +++ b/xen/arch/x86/xen.lds.S
> > > @@ -110,11 +110,9 @@ SECTIONS
> > >         __trampoline_seg_stop = .;
> > >  
> > >         . = ALIGN(8);
> > > -       __CTOR_LIST__ = .;
> > > -       QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2)
> > > +       __ctors_start = .;
> > >         *(.ctors)
> > > -       QUAD(0)
> > > -       __CTOR_END__ = .;
> > > +       __ctors_end = .;
> > >    } :text
> > >    . = ALIGN(32);
> > >    .init.setup : {
> > > diff --git a/xen/common/lib.c b/xen/common/lib.c
> > > index e0c65cf..6b80601 100644
> > > --- a/xen/common/lib.c
> > > +++ b/xen/common/lib.c
> > > @@ -479,17 +479,14 @@ unsigned long long parse_size_and_unit(const char 
> > > *s, const char **ps)
> > >      return ret;
> > >  }
> > >  
> > > -extern const struct
> > > -{
> > > -    unsigned long count;
> > > -    void (*funcs[1])(void);
> > > -} __CTOR_LIST__;
> > > +typedef void (*ctor_func_t)(void);
> > > +extern const ctor_func_t __ctors_start[], __ctors_end[];
> > >  
> > >  void __init init_constructors(void)
> > >  {
> > > -    unsigned long n;
> > > -    for ( n = 0; n < __CTOR_LIST__.count; ++n )
> > > -        __CTOR_LIST__.funcs[n]();
> > > +    const ctor_func_t *f;
> > > +    for (f = __ctors_start; f < __ctors_end; ++f )
> > > +        (*f)();
> > >  }
> > >  
> > >  /*
> > 
> > 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel