This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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_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/ b/ld/scripttempl/

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]