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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.08.16 at 01:07, <konrad.wilk@xxxxxxxxxx> wrote:

According to the code you mean $t instead of $p in the subject.

> --- 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
for them should probably be moved.

Jan


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