This is the mail archive of the binutils@sourceware.org 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 ld/4701: binutils generates invalid klibc-based binary on Linux x86_64


On Wed, Jun 27, 2007 at 09:20:51PM -0700, H. J. Lu wrote:
> On Thu, Jun 28, 2007 at 12:30:27PM +0930, Alan Modra wrote:
> > On Wed, Jun 27, 2007 at 07:40:27PM -0700, H. J. Lu wrote:
> > > Linux ld.so isn't OK with your change. We haven't seen the problem
> > > because we won't get a PT_LOAD segment with .bss sections only
> > > using the default linker script. I uploaded a testcase to:
> > > 
> > > http://sourceware.org/bugzilla/show_bug.cgi?id=4701
> > 
> > Arggh!  I was sure this worked!  So we should go back to using my voff
> > code probably, or otherwise hack p_offset.
> 
> Whatever we do, we should follow gABI.
> > 
> > Meanwhile, please let me know exactly why klibc was unhappy, as I'm
> > concerned that whatever is causing the segv there might also be
> > affected by overlapping p_offset for bss segments.
> 
> My testcase is derived from klibc. Executables have a one PT_LOAD
> segment with only a .bss section. The only difference is the failed
> ones in klibc are executables, which are loaded by kernel, and mine
> is a shared library, which is loaded by ld.so.

This should comply with alignment requirement but not waste file
space.  Can you try this against klibc executables?

	* elf.c (assign_file_positions_for_load_sections): Ensure bss
	segments meet gABI alignment requirements.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.391
diff -u -p -r1.391 elf.c
--- bfd/elf.c	30 May 2007 14:29:27 -0000	1.391
+++ bfd/elf.c	28 Jun 2007 05:21:57 -0000
@@ -4327,6 +4327,7 @@ assign_file_positions_for_load_sections 
        m = m->next, p++, j++)
     {
       asection **secpp;
+      bfd_vma off_adjust;
 
       /* If elf_segment_map is not from map_sections_to_segments, the
          sections may not be correctly ordered.  NOTE: sorting should
@@ -4382,11 +4383,11 @@ assign_file_positions_for_load_sections 
       else
 	p->p_align = 0;
 
+      off_adjust = 0;
       if (p->p_type == PT_LOAD
 	  && m->count > 0)
 	{
 	  bfd_size_type align;
-	  bfd_vma adjust;
 	  unsigned int align_power = 0;
 
 	  if (m->p_align_valid)
@@ -4413,13 +4414,19 @@ assign_file_positions_for_load_sections 
 		 set via struct bfd_elf_special_section.  */
 	      elf_section_type (m->sections[i]) = SHT_NOBITS;
 
-	  adjust = vma_page_aligned_bias (m->sections[0]->vma, off, align);
-	  if (adjust != 0)
+	  off_adjust = vma_page_aligned_bias (m->sections[0]->vma, off, align);
+	  if (off_adjust != 0)
 	    {
 	      /* If the first section isn't loadable, the same holds
-		 for any other sections.  We don't need to align the
-		 segment on disk since the segment doesn't need file
-		 space.  */
+		 for any other sections.  We shouldn't need to align
+		 the segment on disk since the segment doesn't need
+		 file space, but the gABI arguably requires the
+		 alignment and glibc ld.so checks it.  So to comply
+		 with the alignment requirement but not waste file
+		 space, we adjust p_offset for just this segment.
+		 This may put p_offset past the end of file, but
+		 that shouldn't matter.  */
+	      bfd_boolean must_pad = TRUE;
 	      i = 0;
 	      while (elf_section_type (m->sections[i]) == SHT_NOBITS)
 		{
@@ -4429,11 +4436,13 @@ assign_file_positions_for_load_sections 
 		  if ((elf_section_flags (m->sections[i]) & SHF_TLS) == 0
 		      || ++i >= m->count)
 		    {
-		      adjust = 0;
+		      must_pad = FALSE;
 		      break;
 		    }
 		}
-	      off += adjust;
+	      off += off_adjust;
+	      if (must_pad)
+		off_adjust = 0;
 	    }
 	}
       /* Make sure the .dynamic section is the first section in the
@@ -4631,6 +4640,7 @@ assign_file_positions_for_load_sections 
 		p->p_flags |= PF_W;
 	    }
 	}
+      off -= off_adjust;
 
       /* Check that all sections are in a PT_LOAD segment.
 	 Don't check funky gdb generated core files.  */

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre


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