PATCH: PR bfd/14207: linker can produce a NULL GNU_RELRO segment

[nick clifton]

Hi H.J.

> This patch add
> a dummy .relro section to make sure that on-disk portion of PT_LOAD
> segment is aligned to common page size.

I have a couple of comments on the patch:

> +
> +	      if (lp == (phdrs + count)&&  link_info->relro)
> +		abort ();

I think that there ought to be a comment associated with this abort so 
that if it triggers in the future then whomever has to investigate it 
will have some clue as to what is going on.  A reference back to this PR 
would be good.


> +  if (info->relro&&  htab->srelro == NULL)
> +    {
> +      asection *s;
> +      s = bfd_make_section_anyway_with_flags (abfd, ".relro",
> +					      (SEC_LINKER_CREATED
> +					       | SEC_ALLOC
> +					       | SEC_LOAD
> +					       | SEC_KEEP));
> +      if (s == NULL
> +	  || ! bfd_set_section_alignment (abfd, s,
> +					  bfd_log2 (bed->commonpagesize)))
> +	{
> +	  info->callbacks->einfo ("%P: Cannot create .relro section.\n");
> +	  goto error_return;
> +	}
> +      htab->srelro = s;
> +    }

A comment here would be helpful too.  Something like:

   /* Create a dummy section to force the alignment of the segment
      that contains it to be at least the commonpagesize.  This
      matches the expectations of the linker directive
      DATA_SEGMENT_ALIGN.  */

Also the name of the section is a bit misleading, since it does not 
contain anything that is associated with relocations or read-only data. 
  How about .relro.end or .gnu.relro.aligner ?


 > diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc

There are other linker scripts apart from elf.sc that use the 
DATA_SEGMENT_RELRO_END directive.  These ought to be updated as well.

Cheers
   Nick