Re: [PATCH] Put absolute address jump table in data.rel.ro.local if targets support relocations

[Richard Sandiford via Gcc-patches]

Hi,

Sorry for the slow reply.  Just one minor nit:

HAO CHEN GUI <guih...@linux.ibm.com> writes:
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index ea0b59cf44a..40502049b61 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -727,12 +727,26 @@ switch_to_other_text_partition (void)
>    switch_to_section (current_function_section ());
>  }
>  
> -/* Return the read-only data section associated with function DECL.  */
> +/* Return the read-only or relocated read-only data section
> +   associated with function DECL.  */
>  
>  section *
> -default_function_rodata_section (tree decl)
> +default_function_rodata_section (tree decl, bool relocatable)
>  {
> -  if (decl != NULL_TREE && DECL_SECTION_NAME (decl))
> +  const char* sname;
> +  unsigned int flags;
> +
> +  flags = 0;
> +
> +  if (relocatable)
> +    {
> +      sname = ".data.rel.ro.local";
> +      flags = (SECTION_WRITE | SECTION_RELRO);
> +    }
> +  else
> +    sname = ".rodata";
> +
> +  if (decl && DECL_SECTION_NAME (decl))
>      {
>        const char *name = DECL_SECTION_NAME (decl);
>  
> @@ -745,38 +759,57 @@ default_function_rodata_section (tree decl)
>         dot = strchr (name + 1, '.');
>         if (!dot)
>           dot = name;
> -       len = strlen (dot) + 8;
> +       len = strlen (dot) + strlen (sname) + 1;
>         rname = (char *) alloca (len);
>  
> -       strcpy (rname, ".rodata");
> +       strcpy (rname, sname);
>         strcat (rname, dot);
> -       return get_section (rname, SECTION_LINKONCE, decl);
> +       return get_section (rname, (SECTION_LINKONCE | flags), decl);
>       }
> -      /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
> +      /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo or
> +      .gnu.linkonce.d.rel.ro.local.foo if the jump table is relocatable.  */
>        else if (DECL_COMDAT_GROUP (decl)
>              && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
>       {
> -       size_t len = strlen (name) + 1;
> -       char *rname = (char *) alloca (len);
> +       size_t len;
> +       char *rname;
>  
> -       memcpy (rname, name, len);
> -       rname[14] = 'r';
> -       return get_section (rname, SECTION_LINKONCE, decl);
> +       if (relocatable)
> +         {
> +           len = strlen (name) + strlen (".rel.ro.local") + 1;
> +           rname = (char *) alloca (len);
> +
> +           strcpy (rname, ".gnu.linkonce.d");
> +           strcat (rname, ".rel.ro.local");
> +           strcat (rname, name + 15);

I realise you probably wrote it like this to make the correlation
between the length calculation and the string operations more
obvious, but IMO it would be less surprising to have:

              strcpy (rname, ".gnu.linkonce.d.rel.ro.local");

OK with that change, thanks.

Richard