Re: [Xen-devel] [PATCH v1 7/9] livepatch: ARM64: Ignore mapping symbols: $[a, d, x, p]

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Aug 17, 2016 at 06:21:03AM -0600, Jan Beulich wrote:
> >>> On 15.08.16 at 01:07, <konrad.wilk@xxxxxxxxxx> wrote:
> 
> According to the code you mean $t instead of $p in the subject.

Yes! Thanks for noticing that.
> 
> > --- a/xen/arch/arm/livepatch.c
> > +++ b/xen/arch/arm/livepatch.c
> > @@ -90,6 +90,35 @@ void arch_livepatch_unmask(void)
> >      local_abort_enable();
> >  }
> >  
> > +int arch_is_payload_symbol(const struct livepatch_elf *elf,
> > +                           const struct livepatch_elf_sym *sym)
> > +{
> > +    /*
> > +     * - Mapping symbols - denote the "start of a sequence of bytes of the
> > +     *   appropiate type" to mark certain features - such as start of 
> > region
> > +     *   containing A64 ($x), ARM ($a), or Thumb instructions ($t); or 
> > data ($d)
> > +     *
> > +     * The format is either short: '$x' or long: '$x.<any>'. We do not
> > +     * need this and more importantly - each payload will contain this
> > +     * resulting in symbol collisions.
> > +     */
> > +    if ( *sym->name == '$' && sym->name[1] != '\0' )
> > +    {
> > +        char p = sym->name[1];
> > +        size_t len = strlen(sym->name);
> > +
> > +        if ( (len >= 3 && ( sym->name[2] == '.' )) || (len == 2) )
> > +            if ( p == 'd' ||
> 
> May I suggest not nesting two if()-s like this?
> 
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -780,7 +780,7 @@ static bool_t is_payload_symbol(const struct 
> > livepatch_elf *elf,
> >           !strncmp(sym->name, ".L", 2) )
> >          return 0;
> >  
> > -    return 1;
> > +    return arch_is_payload_symbol(elf, sym);
> >  }
> 
> Taking into consideration what's still visible as context here - are .L
> prefixed symbols really architecture independent? If not, checking

They are architecture independent and even the ARM ELF document mentions
it:
"Note: Multiple conventions exist for the names of compiler temporary symbols
(for example, ARMCC uses Lxxx.yyy, while GNU uses .Lxxx)."

(pg 23)

> for them should probably be moved.
> 
> Jan
> 

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