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

[Richard Sandiford <richard.sandiford@arm.com>]

Hi,

Sorry for the slow reply.  Just one minor nit:

HAO CHEN GUI <guihaoc@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