Fix relro strip test for MIPS

Fix relro strip test for MIPS

[Daniel Jacobowitz]

There are two unique things about MIPS which conspire to fail the
ld -z relro -shared tests (which test that objcopy and strip do not
change the binary).  One is that .dynamic is writable and there is no
.got.plt section, so a typical shared library has no PT_GNU_RELRO;
it gets added and then converted to PT_NULL.  The other is that it
adds a second PT_NULL program header for prelinker support.
These combine to produce an input file with two PT_NULL segments.
There's a FIXME in copy_elf_program_header above some code which
seems clearly wrong for this case.

Deleting that turns up a second problem.  The input file to strip
has a PT_NULL segment with alignment of zero (I believe this is the
aborted PT_GNU_RELRO segment, but it might have been the MIPS-specific
one; I'm not completely sure).  We were overriding its input alignment
and I don't see any reason to do so.

With this patch two more tests pass on mips-linux.  Unless someone
sees a problem with them, I will apply them in a few days.

-- 
Daniel Jacobowitz
CodeSourcery

2007-09-19  Daniel Jacobowitz  <dan@codesourcery.com>

	* elf.c (assign_file_positions_for_load_sections): Trust
	p_align_valid.
	(copy_elf_program_header): Copy PT_NULL segments.

Index: elf.c
===================================================================
--- elf.c	(revision 181978)
+++ elf.c	(working copy)
@@ -4108,10 +4108,10 @@ assign_file_positions_for_load_sections 
 
 	  p->p_align = maxpagesize;
 	}
-      else if (m->count == 0)
-	p->p_align = 1 << bed->s->log_file_align;
       else if (m->p_align_valid)
 	p->p_align = m->p_align;
+      else if (m->count == 0)
+	p->p_align = 1 << bed->s->log_file_align;
       else
 	p->p_align = 0;
 
@@ -5590,10 +5590,6 @@ copy_elf_program_header (bfd *ibfd, bfd 
       asection *first_section = NULL;
       asection *lowest_section = NULL;
 
-      /* FIXME: Do we need to copy PT_NULL segment?  */
-      if (segment->p_type == PT_NULL)
-	continue;
-
       /* Compute how many sections are in this segment.  */
       for (section = ibfd->sections, section_count = 0;
 	   section != NULL;

Re: Fix relro strip test for MIPS

[H.J. Lu]

On Wed, Sep 19, 2007 at 12:05:54PM -0400, Daniel Jacobowitz wrote:
> There are two unique things about MIPS which conspire to fail the
> ld -z relro -shared tests (which test that objcopy and strip do not
> change the binary).  One is that .dynamic is writable and there is no
> .got.plt section, so a typical shared library has no PT_GNU_RELRO;
> it gets added and then converted to PT_NULL.  The other is that it

We shouldn't generate a PT_NULL segment when PT_GNU_RELRO is unused.
X86 linkers should handle it correctly.

> adds a second PT_NULL program header for prelinker support.

That may be a MIPS specific problem since PT_NULL is special
for MIPS.

> These combine to produce an input file with two PT_NULL segments.
> There's a FIXME in copy_elf_program_header above some code which
> seems clearly wrong for this case.
> 

H.J.

Re: Fix relro strip test for MIPS

[Daniel Jacobowitz]

On Wed, Sep 19, 2007 at 10:49:36AM -0700, H.J. Lu wrote:
> On Wed, Sep 19, 2007 at 12:05:54PM -0400, Daniel Jacobowitz wrote:
> > There are two unique things about MIPS which conspire to fail the
> > ld -z relro -shared tests (which test that objcopy and strip do not
> > change the binary).  One is that .dynamic is writable and there is no
> > .got.plt section, so a typical shared library has no PT_GNU_RELRO;
> > it gets added and then converted to PT_NULL.  The other is that it
> 
> We shouldn't generate a PT_NULL segment when PT_GNU_RELRO is unused.
> X86 linkers should handle it correctly.

What avoids the extra segment?  There's definitely code in elf.c to
convert PT_GNU_RELRO to PT_NULL.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Fix relro strip test for MIPS

[H.J. Lu]

On Wed, Sep 19, 2007 at 01:52:01PM -0400, Daniel Jacobowitz wrote:
> On Wed, Sep 19, 2007 at 10:49:36AM -0700, H.J. Lu wrote:
> > On Wed, Sep 19, 2007 at 12:05:54PM -0400, Daniel Jacobowitz wrote:
> > > There are two unique things about MIPS which conspire to fail the
> > > ld -z relro -shared tests (which test that objcopy and strip do not
> > > change the binary).  One is that .dynamic is writable and there is no
> > > .got.plt section, so a typical shared library has no PT_GNU_RELRO;
> > > it gets added and then converted to PT_NULL.  The other is that it
> > 
> > We shouldn't generate a PT_NULL segment when PT_GNU_RELRO is unused.
> > X86 linkers should handle it correctly.
> 
> What avoids the extra segment?  There's definitely code in elf.c to
> convert PT_GNU_RELRO to PT_NULL.

I believe Alan checked in a patch to remove PT_NULL segment.
x86 linker won't generate PT_GNU_RELRO for -z relro if it
isn't needed. Some linker relro tests I checked in are specific
to test this.  My goal was to turn on -z relro unconditonally and
PT_GNU_RELRO won't be generated if it isn't supported or used. It
works on x86 and ia64.



H.J.

Re: Fix relro strip test for MIPS

[Alan Modra]

On Wed, Sep 19, 2007 at 11:14:13AM -0700, H.J. Lu wrote:
> I believe Alan checked in a patch to remove PT_NULL segment.

I did commit a patch to remove empty PT_LOAD segments, but I don't
recall doing anything with PT_NULL segments.

-- 
Alan Modra
Australia Development Lab, IBM

Re: Fix relro strip test for MIPS

[H.J. Lu]

On Thu, Sep 20, 2007 at 04:02:12PM +0930, Alan Modra wrote:
> On Wed, Sep 19, 2007 at 11:14:13AM -0700, H.J. Lu wrote:
> > I believe Alan checked in a patch to remove PT_NULL segment.
> 
> I did commit a patch to remove empty PT_LOAD segments, but I don't
> recall doing anything with PT_NULL segments.
> 

What happens is linker will allocate a segment for PT_GNU_RELRO
if there is a .dynamic section. But it doesn't work for MIPS.
Is there a way for linker to remove a segment reserved for
PT_GNU_RELRO?


H.J.

Re: Fix relro strip test for MIPS

[Daniel Jacobowitz]

On Thu, Sep 20, 2007 at 10:57:25AM -0700, H.J. Lu wrote:
> What happens is linker will allocate a segment for PT_GNU_RELRO
> if there is a .dynamic section. But it doesn't work for MIPS.
> Is there a way for linker to remove a segment reserved for
> PT_GNU_RELRO?

Not and reclaim its space.  That's why we convert it to PT_NULL.

On targets where .dynamic is not required to be writable, we know in
advance whether PT_GNU_RELRO will be necessary.  On MIPS we don't;
it depends on the link inputs.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Fix relro strip test for MIPS

[H.J. Lu]

On Thu, Sep 20, 2007 at 02:07:51PM -0400, Daniel Jacobowitz wrote:
> On Thu, Sep 20, 2007 at 10:57:25AM -0700, H.J. Lu wrote:
> > What happens is linker will allocate a segment for PT_GNU_RELRO
> > if there is a .dynamic section. But it doesn't work for MIPS.
> > Is there a way for linker to remove a segment reserved for
> > PT_GNU_RELRO?
> 
> Not and reclaim its space.  That's why we convert it to PT_NULL.
> 

That was done before Alan wrote a patch to remove the empty PT_LOAD
segment. Maybe we can use the similar approach to remove empty
PT_GNU_RELRO segment.


H.J.

PATCH: Fix relro strip test for MIPS

[H.J. Lu]

On Thu, Sep 20, 2007 at 11:58:20AM -0700, H.J. Lu wrote:
> On Thu, Sep 20, 2007 at 02:07:51PM -0400, Daniel Jacobowitz wrote:
> > On Thu, Sep 20, 2007 at 10:57:25AM -0700, H.J. Lu wrote:
> > > What happens is linker will allocate a segment for PT_GNU_RELRO
> > > if there is a .dynamic section. But it doesn't work for MIPS.
> > > Is there a way for linker to remove a segment reserved for
> > > PT_GNU_RELRO?
> > 
> > Not and reclaim its space.  That's why we convert it to PT_NULL.
> > 
> 
> That was done before Alan wrote a patch to remove the empty PT_LOAD
> segment. Maybe we can use the similar approach to remove empty
> PT_GNU_RELRO segment.
> 
> 

This patch may not be perfect. We may need to a better way to check if
a PT_GNU_RELRO segment is needed in _bfd_elf_map_sections_to_segments.


H.J.
----
2007-09-21  H.J. Lu  <hongjiu.lu@intel.com>

	* elf.c (get_program_header_size): Always add a PT_GNU_RELRO
	segment for -z relro.
	(_bfd_elf_map_sections_to_segments): Make a PT_GNU_RELRO
	segment only when needed.

--- bfd/elf.c.relro	2007-09-21 09:37:29.000000000 -0700
+++ bfd/elf.c	2007-09-21 09:52:42.000000000 -0700
@@ -3362,13 +3362,12 @@ get_program_header_size (bfd *abfd, stru
     {
       /* We need a PT_DYNAMIC segment.  */
       ++segs;
+    }
 
-      if (info->relro)
-	{
-	  /* We need a PT_GNU_RELRO segment only when there is a
-	     PT_DYNAMIC segment.  */
-	  ++segs;
-	}
+  if (info->relro)
+    {
+      /* We need a PT_GNU_RELRO segment.  */
+      ++segs;
     }
 
   if (elf_tdata (abfd)->eh_frame_hdr)
@@ -3991,21 +3990,43 @@ _bfd_elf_map_sections_to_segments (bfd *
 	  pm = &m->next;
 	}
 
-      if (dynsec != NULL && info->relro)
+      if (info->relro)
 	{
-	  /* We make a PT_GNU_RELRO segment only when there is a
-	     PT_DYNAMIC segment.  */
-	  amt = sizeof (struct elf_segment_map);
-	  m = bfd_zalloc (abfd, amt);
-	  if (m == NULL)
-	    goto error_return;
-	  m->next = NULL;
-	  m->p_type = PT_GNU_RELRO;
-	  m->p_flags = PF_R;
-	  m->p_flags_valid = 1;
+	  bfd_vma vaddr;
 
-	  *pm = m;
-	  pm = &m->next;
+	  for (m = mfirst; m != NULL; m = m->next)
+	    {
+	      vaddr = 0;
+	      if (m->p_type == PT_LOAD)
+		{
+		  bfd_vma filesz = 0;
+
+		  vaddr = m->sections[0]->vma;
+		  for (i = 0; i < m->count; i++)
+		    filesz = (m->sections[i]->vma - vaddr
+			      + m->sections[i]->size);
+
+		  if (vaddr <= info->relro_end
+		      && vaddr >= info->relro_start
+		      && (vaddr + filesz >= info->relro_end))
+		    break;
+		}
+	      }
+
+	  if (m != NULL && info->relro_end > vaddr)
+	    {
+	      amt = sizeof (struct elf_segment_map);
+	      m = bfd_zalloc (abfd, amt);
+	      if (m == NULL)
+		goto error_return;
+	      m->next = NULL;
+	      m->p_type = PT_GNU_RELRO;
+	      m->p_flags = PF_R;
+	      m->p_flags_valid = 1;
+
+	      *pm = m;
+	      pm = &m->next;
+	    }
 	}
 
       free (sections);

Re: PATCH: Fix relro strip test for MIPS

[H.J. Lu]

On Fri, Sep 21, 2007 at 09:59:00AM -0700, H.J. Lu wrote:
> On Thu, Sep 20, 2007 at 11:58:20AM -0700, H.J. Lu wrote:
> > On Thu, Sep 20, 2007 at 02:07:51PM -0400, Daniel Jacobowitz wrote:
> > > On Thu, Sep 20, 2007 at 10:57:25AM -0700, H.J. Lu wrote:
> > > > What happens is linker will allocate a segment for PT_GNU_RELRO
> > > > if there is a .dynamic section. But it doesn't work for MIPS.
> > > > Is there a way for linker to remove a segment reserved for
> > > > PT_GNU_RELRO?
> > > 
> > > Not and reclaim its space.  That's why we convert it to PT_NULL.
> > > 
> > 
> > That was done before Alan wrote a patch to remove the empty PT_LOAD
> > segment. Maybe we can use the similar approach to remove empty
> > PT_GNU_RELRO segment.
> > 
> > 
> 
> This patch may not be perfect. We may need to a better way to check if
> a PT_GNU_RELRO segment is needed in _bfd_elf_map_sections_to_segments.
> 
> 

I believe each call to assign_file_positions_for_load_sections
will assign file positions for load sections based on the current
segment layout. For -z relro, we only need to know if we need
to make a PT_GNU_RELRO segment in _bfd_elf_map_sections_to_segments.
We can call assign_file_positions_for_load_sections to get the
load section layout and find out if the PT_GNU_RELRO segment will
be empty or not.  If it will be empty, we don't need to make the
PT_GNU_RELRO segment. I don't think a PT_GNU_RELRO segment will
affect load section lay out due to extra segment in such a way
that the PT_GNU_RELRO segment won't be empty. If it is the case,
we don't need to call assign_file_positions_for_load_sections with
an argument of extra segments.

assign_file_positions_for_load_sections needs to know ELF section
header. I break set_elf_section from elf_fake_sections and call
it before calling assign_file_positions_for_load_sections.

H.J.
----
2007-09-22  H.J. Lu  <hongjiu.lu@intel.com>

	* elf.c (assign_file_positions_for_load_sections): Declared.
	Take an argument of extra segments.
	(set_elf_section): New.
	(elf_fake_sections ): Use set_elf_section.
	(get_program_header_size): Always add a PT_GNU_RELRO segment
	for -z relro.
	(_bfd_elf_map_sections_to_segments): For -z relro, call
	assign_file_positions_for_load_sections and make a PT_GNU_RELRO
	segment only when it isn't empty.
	(assign_file_positions_for_non_load_sections): Abort when
	the PT_GNU_RELRO segment is empty.
	(assign_file_positions_except_relocs): Updated.
	(prep_headers): Don't set e_phentsize and e_phoff twice.

--- bfd/elf.c.relro	2007-09-22 08:26:46.000000000 -0700
+++ bfd/elf.c	2007-09-22 14:09:55.000000000 -0700
@@ -51,6 +51,8 @@ static bfd_boolean swap_out_syms (bfd *,
 static bfd_boolean elf_read_notes (bfd *, file_ptr, bfd_size_type) ;
 static bfd_boolean elf_parse_notes (bfd *abfd, char *buf, size_t size,
 				    file_ptr offset);
+static bfd_boolean assign_file_positions_for_load_sections 
+  (bfd *abfd, struct bfd_link_info *link_info, unsigned int extra);
 
 /* Swap version information in and out.  The version information is
    currently size independent.  If that ever changes, this code will
@@ -2434,15 +2436,18 @@ _bfd_elf_init_reloc_shdr (bfd *abfd,
   return TRUE;
 }
 
-/* Set up an ELF internal section header for a section.  */
+/* Set the ELF internal section header fields for a section, except for
+   sh_name
+   */
 
 static void
-elf_fake_sections (bfd *abfd, asection *asect, void *failedptrarg)
+set_elf_section (bfd *abfd, asection *asect, void *failedptrarg)
 {
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   bfd_boolean *failedptr = failedptrarg;
   Elf_Internal_Shdr *this_hdr;
   unsigned int sh_type;
+  bfd_vma sh_flags;
 
   if (*failedptr)
     {
@@ -2453,14 +2458,6 @@ elf_fake_sections (bfd *abfd, asection *
 
   this_hdr = &elf_section_data (asect)->this_hdr;
 
-  this_hdr->sh_name = (unsigned int) _bfd_elf_strtab_add (elf_shstrtab (abfd),
-							  asect->name, FALSE);
-  if (this_hdr->sh_name == (unsigned int) -1)
-    {
-      *failedptr = TRUE;
-      return;
-    }
-
   /* Don't clear sh_flags. Assembler may set additional bits.  */
 
   if ((asect->flags & SEC_ALLOC) != 0
@@ -2470,13 +2467,15 @@ elf_fake_sections (bfd *abfd, asection *
     this_hdr->sh_addr = 0;
 
   this_hdr->sh_offset = 0;
-  this_hdr->sh_size = asect->size;
+  if (this_hdr->bfd_section)
+    BFD_ASSERT (this_hdr->sh_size == asect->size);
+  else
+    this_hdr->sh_size = asect->size;
   this_hdr->sh_link = 0;
   this_hdr->sh_addralign = 1 << asect->alignment_power;
   /* The sh_entsize and sh_info fields may have been set already by
      copy_private_section_data.  */
 
-  this_hdr->bfd_section = asect;
   this_hdr->contents = NULL;
 
   /* If the section type is unspecified, we set it based on
@@ -2578,24 +2577,25 @@ elf_fake_sections (bfd *abfd, asection *
       break;
     }
 
+  sh_flags = this_hdr->sh_flags;
   if ((asect->flags & SEC_ALLOC) != 0)
-    this_hdr->sh_flags |= SHF_ALLOC;
+    sh_flags |= SHF_ALLOC;
   if ((asect->flags & SEC_READONLY) == 0)
-    this_hdr->sh_flags |= SHF_WRITE;
+    sh_flags |= SHF_WRITE;
   if ((asect->flags & SEC_CODE) != 0)
-    this_hdr->sh_flags |= SHF_EXECINSTR;
+    sh_flags |= SHF_EXECINSTR;
   if ((asect->flags & SEC_MERGE) != 0)
     {
-      this_hdr->sh_flags |= SHF_MERGE;
+      sh_flags |= SHF_MERGE;
       this_hdr->sh_entsize = asect->entsize;
       if ((asect->flags & SEC_STRINGS) != 0)
-	this_hdr->sh_flags |= SHF_STRINGS;
+	sh_flags |= SHF_STRINGS;
     }
   if ((asect->flags & SEC_GROUP) == 0 && elf_group_name (asect) != NULL)
-    this_hdr->sh_flags |= SHF_GROUP;
+    sh_flags |= SHF_GROUP;
   if ((asect->flags & SEC_THREAD_LOCAL) != 0)
     {
-      this_hdr->sh_flags |= SHF_TLS;
+      sh_flags |= SHF_TLS;
       if (asect->size == 0
 	  && (asect->flags & SEC_HAS_CONTENTS) == 0)
 	{
@@ -2611,6 +2611,16 @@ elf_fake_sections (bfd *abfd, asection *
 	}
     }
 
+  if (this_hdr->bfd_section)
+    BFD_ASSERT (this_hdr->sh_flags == sh_flags);
+  else
+    this_hdr->sh_flags = sh_flags;
+
+  if (this_hdr->bfd_section)
+    BFD_ASSERT (this_hdr->bfd_section == asect);
+  else
+    this_hdr->bfd_section = asect;
+
   /* Check for processor-specific section types.  */
   sh_type = this_hdr->sh_type;
   if (bed->elf_backend_fake_sections
@@ -2623,6 +2633,34 @@ elf_fake_sections (bfd *abfd, asection *
 	 called for objcopy --only-keep-debug.  */
       this_hdr->sh_type = sh_type;
     }
+}
+
+/* Set up an ELF internal section header for a section.  */
+
+static void
+elf_fake_sections (bfd *abfd, asection *asect, void *failedptrarg)
+{
+  bfd_boolean *failedptr = failedptrarg;
+  Elf_Internal_Shdr *this_hdr;
+
+  set_elf_section (abfd, asect, failedptrarg);
+
+  if (*failedptr)
+    {
+      /* We already failed; just get out of the bfd_map_over_sections
+	 loop.  */
+      return;
+    }
+
+  this_hdr = &elf_section_data (asect)->this_hdr;
+
+  this_hdr->sh_name = (unsigned int) _bfd_elf_strtab_add (elf_shstrtab (abfd),
+							  asect->name, FALSE);
+  if (this_hdr->sh_name == (unsigned int) -1)
+    {
+      *failedptr = TRUE;
+      return;
+    }
 
   /* If the section has relocs, set up a section header for the
      SHT_REL[A] section.  If two relocation sections are required for
@@ -3362,13 +3400,12 @@ get_program_header_size (bfd *abfd, stru
     {
       /* We need a PT_DYNAMIC segment.  */
       ++segs;
+    }
 
-      if (info->relro)
-	{
-	  /* We need a PT_GNU_RELRO segment only when there is a
-	     PT_DYNAMIC segment.  */
-	  ++segs;
-	}
+  if (info->relro)
+    {
+      /* We need a PT_GNU_RELRO segment.  */
+      ++segs;
     }
 
   if (elf_tdata (abfd)->eh_frame_hdr)
@@ -3991,25 +4028,58 @@ _bfd_elf_map_sections_to_segments (bfd *
 	  pm = &m->next;
 	}
 
-      if (dynsec != NULL && info->relro)
+      elf_tdata (abfd)->segment_map = mfirst;
+
+      if (info->relro)
 	{
-	  /* We make a PT_GNU_RELRO segment only when there is a
-	     PT_DYNAMIC segment.  */
-	  amt = sizeof (struct elf_segment_map);
-	  m = bfd_zalloc (abfd, amt);
-	  if (m == NULL)
-	    goto error_return;
-	  m->next = NULL;
-	  m->p_type = PT_GNU_RELRO;
-	  m->p_flags = PF_R;
-	  m->p_flags_valid = 1;
+	  Elf_Internal_Phdr *p;
+	  bfd_boolean failed = FALSE;
+	  
+	  if (!elf_modify_segment_map (abfd, info, no_user_phdrs))
+	    return FALSE;
 
-	  *pm = m;
-	  pm = &m->next;
+	  bfd_map_over_sections (abfd, set_elf_section, &failed);
+	  if (failed)
+	    return FALSE;
+
+	  /* We need file positions of loaded sectons to check if
+	     the PT_GNU_RELRO segment is needed.  Assign file positions
+	     for the loaded sections based on the assignment of sections
+	     to segments.  */
+	  if (!assign_file_positions_for_load_sections (abfd, info, 1))
+	    {
+	      elf_tdata (abfd)->segment_map = NULL;
+	      return FALSE;
+	    }
+
+	  p = elf_tdata (abfd)->phdr;
+	  for (m = mfirst; m != NULL; m = m->next, p++)
+	    {
+	      if (p->p_type == PT_LOAD
+		  && p->p_vaddr <= info->relro_end
+		  && p->p_vaddr >= info->relro_start
+		  && (p->p_vaddr + p->p_filesz >= info->relro_end))
+		break;
+	    }
+
+	  /* Make a PT_GNU_RELRO segment only when it isn't empty.  */
+	  if (m != NULL && info->relro_end > p->p_vaddr)
+	    {
+	      amt = sizeof (struct elf_segment_map);
+	      m = bfd_zalloc (abfd, amt);
+	      if (m == NULL)
+		goto error_return;
+	      m->next = NULL;
+	      m->p_type = PT_GNU_RELRO;
+	      m->p_flags = PF_R;
+	      m->p_flags_valid = 1;
+
+	      *pm = m;
+	      pm = &m->next;
+	    }
 	}
 
       free (sections);
-      elf_tdata (abfd)->segment_map = mfirst;
     }
 
   if (!elf_modify_segment_map (abfd, info, no_user_phdrs))
@@ -4146,7 +4216,8 @@ print_segment_map (const struct elf_segm
 
 static bfd_boolean
 assign_file_positions_for_load_sections (bfd *abfd,
-					 struct bfd_link_info *link_info)
+					 struct bfd_link_info *link_info,
+					 unsigned int extra)
 {
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   struct elf_segment_map *m;
@@ -4169,7 +4240,9 @@ assign_file_positions_for_load_sections 
   elf_elfheader (abfd)->e_phentsize = bed->s->sizeof_phdr;
   elf_elfheader (abfd)->e_phnum = alloc;
 
-  if (elf_tdata (abfd)->program_header_size == (bfd_size_type) -1)
+  /* When extra is 0, number of segments won't change.  */
+  if (extra == 0
+      || elf_tdata (abfd)->program_header_size == (bfd_size_type) -1)
     elf_tdata (abfd)->program_header_size = alloc * bed->s->sizeof_phdr;
   else
     BFD_ASSERT (elf_tdata (abfd)->program_header_size
@@ -4191,7 +4264,7 @@ assign_file_positions_for_load_sections 
     maxpagesize = bed->maxpagesize;
 
   off = bed->s->sizeof_ehdr;
-  off += alloc * bed->s->sizeof_phdr;
+  off += (alloc + extra) * bed->s->sizeof_phdr;
 
   for (m = elf_tdata (abfd)->segment_map, p = phdrs, j = 0;
        m != NULL;
@@ -4380,8 +4453,8 @@ assign_file_positions_for_load_sections 
 		}
 	    }
 
-	  p->p_filesz += alloc * bed->s->sizeof_phdr;
-	  p->p_memsz += alloc * bed->s->sizeof_phdr;
+	  p->p_filesz += (alloc + extra) * bed->s->sizeof_phdr;
+	  p->p_memsz += (alloc + extra) * bed->s->sizeof_phdr;
 	}
 
       if (p->p_type == PT_LOAD
@@ -4735,10 +4808,7 @@ assign_file_positions_for_non_load_secti
 		  p->p_flags = (lp->p_flags & ~PF_W);
 		}
 	      else
-		{
-		  memset (p, 0, sizeof *p);
-		  p->p_type = PT_NULL;
-		}
+		abort ();
 	    }
 	}
     }
@@ -4814,7 +4884,7 @@ assign_file_positions_except_relocs (bfd
 
       /* Assign file positions for the loaded sections based on the
 	 assignment of sections to segments.  */
-      if (!assign_file_positions_for_load_sections (abfd, link_info))
+      if (!assign_file_positions_for_load_sections (abfd, link_info, 0))
 	return FALSE;
 
       /* And for non-load sections.  */
@@ -4918,11 +4988,7 @@ prep_headers (bfd *abfd)
     /* It all happens later.  */
     ;
   else
-    {
-      i_ehdrp->e_phentsize = 0;
-      i_phdrp = 0;
-      i_ehdrp->e_phoff = 0;
-    }
+    i_phdrp = 0;
 
   elf_tdata (abfd)->symtab_hdr.sh_name =
     (unsigned int) _bfd_elf_strtab_add (shstrtab, ".symtab", FALSE);

Re: Fix relro strip test for MIPS

[Daniel Jacobowitz]

On Wed, Sep 19, 2007 at 12:05:54PM -0400, Daniel Jacobowitz wrote:
> There are two unique things about MIPS which conspire to fail the
> ld -z relro -shared tests (which test that objcopy and strip do not
> change the binary).  One is that .dynamic is writable and there is no
> .got.plt section, so a typical shared library has no PT_GNU_RELRO;
> it gets added and then converted to PT_NULL.  The other is that it
> adds a second PT_NULL program header for prelinker support.
> These combine to produce an input file with two PT_NULL segments.
> There's a FIXME in copy_elf_program_header above some code which
> seems clearly wrong for this case.
> 
> Deleting that turns up a second problem.  The input file to strip
> has a PT_NULL segment with alignment of zero (I believe this is the
> aborted PT_GNU_RELRO segment, but it might have been the MIPS-specific
> one; I'm not completely sure).  We were overriding its input alignment
> and I don't see any reason to do so.
> 
> With this patch two more tests pass on mips-linux.  Unless someone
> sees a problem with them, I will apply them in a few days.

I agree with H.J. that we should not generate the extra PT_NULL
segment, and I'm keeping an eye on his patch to prevent that
(thanks!).  Meanwhile, there were no concerns about this patch
for copying odd binaries; I've checked it in.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: Fix relro strip test for MIPS

[Alan Modra]

On Sat, Sep 22, 2007 at 02:17:08PM -0700, H.J. Lu wrote:
> We can call assign_file_positions_for_load_sections to get the
> load section layout and find out if the PT_GNU_RELRO segment will
> be empty or not.

I haven't been following this thread closely, but I really don't like
calling assign_file_positions_for_load_sections early like this.
Can't you determine whether a PT_GNU_RELRO program header will be
needed just by looking at the PT_LOAD sections in
_bfd_elf_map_sections_to_segments?

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: Fix relro strip test for MIPS

[H.J. Lu]

On Thu, Sep 27, 2007 at 01:54:44PM +0930, Alan Modra wrote:
> On Sat, Sep 22, 2007 at 02:17:08PM -0700, H.J. Lu wrote:
> > We can call assign_file_positions_for_load_sections to get the
> > load section layout and find out if the PT_GNU_RELRO segment will
> > be empty or not.
> 
> I haven't been following this thread closely, but I really don't like
> calling assign_file_positions_for_load_sections early like this.
> Can't you determine whether a PT_GNU_RELRO program header will be
> needed just by looking at the PT_LOAD sections in
> _bfd_elf_map_sections_to_segments?
> 

This patch

http://sourceware.org/ml/binutils/2007-09/msg00324.html

doesn't call assign_file_positions_for_load_sections.


H.J.

Re: PATCH: Fix relro strip test for MIPS

[Alan Modra]

On Wed, Oct 10, 2007 at 11:11:18AM -0700, H.J. Lu wrote:
> On Thu, Sep 27, 2007 at 01:54:44PM +0930, Alan Modra wrote:
> > On Sat, Sep 22, 2007 at 02:17:08PM -0700, H.J. Lu wrote:
> > > We can call assign_file_positions_for_load_sections to get the
> > > load section layout and find out if the PT_GNU_RELRO segment will
> > > be empty or not.
> > 
> > I haven't been following this thread closely, but I really don't like
> > calling assign_file_positions_for_load_sections early like this.
> > Can't you determine whether a PT_GNU_RELRO program header will be
> > needed just by looking at the PT_LOAD sections in
> > _bfd_elf_map_sections_to_segments?
> > 
> 
> This patch
> 
> http://sourceware.org/ml/binutils/2007-09/msg00324.html
> 
> doesn't call assign_file_positions_for_load_sections.

This patch is OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: Fix relro strip test for MIPS

[H.J. Lu]

On Thu, Oct 11, 2007 at 09:35:31AM +0930, Alan Modra wrote:
> On Wed, Oct 10, 2007 at 11:11:18AM -0700, H.J. Lu wrote:
> > On Thu, Sep 27, 2007 at 01:54:44PM +0930, Alan Modra wrote:
> > > On Sat, Sep 22, 2007 at 02:17:08PM -0700, H.J. Lu wrote:
> > > > We can call assign_file_positions_for_load_sections to get the
> > > > load section layout and find out if the PT_GNU_RELRO segment will
> > > > be empty or not.
> > > 
> > > I haven't been following this thread closely, but I really don't like
> > > calling assign_file_positions_for_load_sections early like this.
> > > Can't you determine whether a PT_GNU_RELRO program header will be
> > > needed just by looking at the PT_LOAD sections in
> > > _bfd_elf_map_sections_to_segments?
> > > 
> > 
> > This patch
> > 
> > http://sourceware.org/ml/binutils/2007-09/msg00324.html
> > 
> > doesn't call assign_file_positions_for_load_sections.
> 
> This patch is OK.
> 

I am checking in this equivalent patch. There is no need for the
loop. We can just use the first and last sections to compute the
file size.


H.J.
----
2007-10-10  H.J. Lu  <hongjiu.lu@intel.com>

	* elf.c (get_program_header_size): Always add a PT_GNU_RELRO
	segment for -z relro.
	(_bfd_elf_map_sections_to_segments): Make a PT_GNU_RELRO
	segment only when needed.

--- bfd/elf.c.relro	2007-10-10 19:02:12.000000000 -0700
+++ bfd/elf.c	2007-10-10 19:16:42.000000000 -0700
@@ -3362,13 +3362,12 @@ get_program_header_size (bfd *abfd, stru
     {
       /* We need a PT_DYNAMIC segment.  */
       ++segs;
+    }
 
-      if (info->relro)
-	{
-	  /* We need a PT_GNU_RELRO segment only when there is a
-	     PT_DYNAMIC segment.  */
-	  ++segs;
-	}
+  if (info->relro)
+    {
+      /* We need a PT_GNU_RELRO segment.  */
+      ++segs;
     }
 
   if (elf_tdata (abfd)->eh_frame_hdr)
@@ -3991,21 +3990,38 @@ _bfd_elf_map_sections_to_segments (bfd *
 	  pm = &m->next;
 	}
 
-      if (dynsec != NULL && info->relro)
+      if (info->relro)
 	{
-	  /* We make a PT_GNU_RELRO segment only when there is a
-	     PT_DYNAMIC segment.  */
-	  amt = sizeof (struct elf_segment_map);
-	  m = bfd_zalloc (abfd, amt);
-	  if (m == NULL)
-	    goto error_return;
-	  m->next = NULL;
-	  m->p_type = PT_GNU_RELRO;
-	  m->p_flags = PF_R;
-	  m->p_flags_valid = 1;
+	  for (m = mfirst; m != NULL; m = m->next)
+	    {
+	      if (m->p_type == PT_LOAD)
+		{
+		  asection *last = m->sections[m->count - 1];
+		  bfd_vma vaddr = m->sections[0]->vma;
+		  bfd_vma filesz = last->vma - vaddr + last->size;
+
+		  if (vaddr < info->relro_end
+		      && vaddr >= info->relro_start
+		      && (vaddr + filesz) >= info->relro_end)
+		    break;
+		}
+	      }
 
-	  *pm = m;
-	  pm = &m->next;
+	  /* Make a PT_GNU_RELRO segment only when it isn't empty.  */
+	  if (m != NULL)
+	    {
+	      amt = sizeof (struct elf_segment_map);
+	      m = bfd_zalloc (abfd, amt);
+	      if (m == NULL)
+		goto error_return;
+	      m->next = NULL;
+	      m->p_type = PT_GNU_RELRO;
+	      m->p_flags = PF_R;
+	      m->p_flags_valid = 1;
+
+	      *pm = m;
+	      pm = &m->next;
+	    }
 	}
 
       free (sections);

Re: PATCH: Fix relro strip test for MIPS

[Alan Modra]

On Wed, Oct 10, 2007 at 07:25:27PM -0700, H.J. Lu wrote:
> I am checking in this equivalent patch. There is no need for the
> loop. We can just use the first and last sections to compute the
> file size.

Fine by me.

-- 
Alan Modra
Australia Development Lab, IBM