This is the mail archive of the
mailing list for the binutils project.
Re: PATCH: PR bfd/14207: linker can produce a NULL GNU_RELRO segment
- From: nick clifton <nickc at redhat dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: "H.J. Lu" <hongjiu dot lu at intel dot com>, binutils at sourceware dot org
- Date: Mon, 11 Jun 2012 15:15:02 +0100
- Subject: Re: PATCH: PR bfd/14207: linker can produce a NULL GNU_RELRO segment
- References: <20120610175522.GA3022@intel.com>
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_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
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.