Re: [Xen-devel] [PATCH v2 LIVEPATCH-BUILD-TOOLS] Fix patch creation with GCC 6.1+

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

On Mon, Dec 12, 2016 at 02:55:51PM +0000, Ross Lagerwall wrote:
> GCC 6.1+ fixed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=192 which
> means that .rodata.str1.[0-9]+ sections are now split by function.  We
> could probably be smarter about including just the sections we need, but
> for now, simply include the string sections for all functions as is done
> for previous versions of GCC.

Are there plans to push this to the upstream kpatch repo? Or did
they do it in some other way?
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> Reported-by: M A Young <m.a.young@xxxxxxxxxxxx>
> ---
> 
> Changed in v2:
> * Clarified commit message.

Thanks. Two little nitpicks - feel free to either ignore them
or take them into account.

Either way:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
>  create-diff-object.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/create-diff-object.c b/create-diff-object.c
> index 69bcd88..b0d1348 100644
> --- a/create-diff-object.c
> +++ b/create-diff-object.c
> @@ -1184,6 +1184,43 @@ static void kpatch_process_special_sections(struct 
> kpatch_elf *kelf)
>       }
>  }
>  
> +/* Returns true if s is a string of only numbers with length > 0. */
> +static int isnumber(const char *s)

Could you use bool?

> +{
> +     do {
> +             if (!*s || !isdigit(*s))
> +                     return 0;
> +     } while (*++s);
> +
> +     return 1;
> +}
> +
> +/*
> + * String sections are always included even if unchanged.
> + * The format is either:
> + * .rodata.<func>.str1.[0-9]+ (new in GCC 6.1.0)
> + * or .rodata.str1.[0-9]+ (older versions of GCC)
> + * For the new format we could be smarter and only include the needed
> + * strings sections.
> + */
> +static int should_include_str_section(const char *name)

Ditto here? bool?

> +{
> +     const char *s;
> +
> +     if (strncmp(name, ".rodata.", 8))
> +             return 0;
> +
> +     /* Check if name matches ".rodata.str1.[0-9]+" */
> +     if (!strncmp(name, ".rodata.str1.", 13))
> +             return isnumber(name + 13);

I would make an #define for the '13'
> +
> +     /* Check if name matches ".rodata.<func>.str1.[0-9]+" */
> +     s = strstr(name, ".str1.");
> +     if (!s)
> +             return 0;
> +     return isnumber(s + 6);
> +}
> +
>  static void kpatch_include_standard_elements(struct kpatch_elf *kelf)
>  {
>       struct section *sec;
> @@ -1193,7 +1230,7 @@ static void kpatch_include_standard_elements(struct 
> kpatch_elf *kelf)
>               if (!strcmp(sec->name, ".shstrtab") ||
>                   !strcmp(sec->name, ".strtab") ||
>                   !strcmp(sec->name, ".symtab") ||
> -                 !strncmp(sec->name, ".rodata.str1.", 13)) {
> +                 should_include_str_section(sec->name)) {
>                       sec->include = 1;
>                       if (sec->secsym)
>                               sec->secsym->include = 1;
> -- 
> 2.7.4
> 

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