possible BFD regression from a change made last September

possible BFD regression from a change made last September

[Bob Wilson]

Alan,

I just discovered a problem related to the following change:

2004-09-22  Alan Modra  <amodra@bigpond.net.au>

	* elf.c (IS_LOADED): Define.
	(assign_file_positions_for_segments): Don't round up file offset of
	PT_LOAD segments containing no SEC_LOAD sections, instead round down.
	Delete code handling link script adjustment of lma.  Do the adjust
	in later code handling similar ajustments.  Remove dead code error
	check.  Warn if section lma would require a negative offset
	adjustment.  Tweak lma adjustment to use p_filesz rather than p_memsz.
	Use p_vaddr + p_memsz inside section loop in place of voff.  Don't
	update voff in section loop.  Change voff in segment loop to be an
	adjustment on top of "off".  Set sec->filepos and update "off" later.
	Test for loadable sections consistently using IS_LOADED.  Similarly,
	test for alloc-only sections other than .tbss consistently.
	Don't bother checking SEC_ALLOC in PT_LOAD segments.  Remove FIXME.
	Tidy PT_NOTE handling.  Use %B and %A in error messages.
	(assign_file_positions_except_relocs): Use %B in error message.

I'm still working on creating a simple testcase, but I thought it might be 
worthwhile to see if you have any ideas.

Here is the situation: I have a linker script with explicit PHDRS, an Xtensa 
processor without an MMU so that maxpagesize is 1, and a .bss section with 
16-byte alignment.  The code in assign_file_positions_for_segments is setting 
the segment size 8 bytes too small, because it is not including 8 bytes of 
padding needed to align the .bss section.  Prior to your change, this was 
handled by incrementing p_memsz in the following code:

@@ -4067,117 +4102,82 @@ assign_file_positions_for_segments (bfd
  	  flags = sec->flags;
  	  align = 1 << bfd_get_section_alignment (abfd, sec);

-	  /* The section may have artificial alignment forced by a
-	     link script.  Notice this case by the gap between the
-	     cumulative phdr lma and the section's lma.  */
-	  if (p->p_paddr + p->p_memsz < sec->lma)
-	    {
-	      bfd_vma adjust = sec->lma - (p->p_paddr + p->p_memsz);
-
-	      p->p_memsz += adjust;
-	      if (p->p_type == PT_LOAD
-		  || (p->p_type == PT_NOTE
-		      && bfd_get_format (abfd) == bfd_core))
-		{
-		  off += adjust;
-		  voff += adjust;
-		}
-	      if ((flags & SEC_LOAD) != 0
-		  || (flags & SEC_THREAD_LOCAL) != 0)
-		p->p_filesz += adjust;
-	    }
-

If the page size is not 1, I can see how this case is now handled by:

		  /* The section VMA must equal the file position
		     modulo the page size.  */
		  bfd_size_type page = align;
		  if ((abfd->flags & D_PAGED) != 0)
		    page = bed->maxpagesize;
		  adjust = vma_page_aligned_bias (sec->vma,
						  p->p_vaddr + p->p_memsz,
						  page);
		  p->p_memsz += adjust;

but that breaks in my scenario where align == 16 but maxpagesize == 1.

Any ideas on how this ought to be handled?

--Bob

Re: possible BFD regression from a change made last September

[Alan Modra]

On Tue, May 17, 2005 at 10:15:32AM -0700, Bob Wilson wrote:
> If the page size is not 1, I can see how this case is now handled by:
> 
> 		  /* The section VMA must equal the file position
> 		     modulo the page size.  */
> 		  bfd_size_type page = align;
> 		  if ((abfd->flags & D_PAGED) != 0)
> 		    page = bed->maxpagesize;
> 		  adjust = vma_page_aligned_bias (sec->vma,
> 						  p->p_vaddr + p->p_memsz,
> 						  page);
> 		  p->p_memsz += adjust;
> 
> but that breaks in my scenario where align == 16 but maxpagesize == 1.

We ought to be taking the maximum of align and maxpagesize.  I think the
following should do the trick.

	* elf.c (assign_file_positions_for_segments): Use maximum of
	maxpagesize and section alignment when adjusting initial
	segment offset and section offsets.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.292
diff -u -p -r1.292 elf.c
--- bfd/elf.c	11 May 2005 02:15:46 -0000	1.292
+++ bfd/elf.c	18 May 2005 00:02:48 -0000
@@ -4145,22 +4145,20 @@ assign_file_positions_for_segments (bfd 
 	{
 	  bfd_size_type align;
 	  bfd_vma adjust;
+	  unsigned int align_power = 0;
 
-	  if ((abfd->flags & D_PAGED) != 0)
-	    align = bed->maxpagesize;
-	  else
+	  for (i = 0, secpp = m->sections; i < m->count; i++, secpp++)
 	    {
-	      unsigned int align_power = 0;
-	      for (i = 0, secpp = m->sections; i < m->count; i++, secpp++)
-		{
-		  unsigned int secalign;
+	      unsigned int secalign;
 
-		  secalign = bfd_get_section_alignment (abfd, *secpp);
-		  if (secalign > align_power)
-		    align_power = secalign;
-		}
-	      align = (bfd_size_type) 1 << align_power;
+	      secalign = bfd_get_section_alignment (abfd, *secpp);
+	      if (secalign > align_power)
+		align_power = secalign;
 	    }
+	  align = (bfd_size_type) 1 << align_power;
+
+	  if ((abfd->flags & D_PAGED) != 0 && bed->maxpagesize > align)
+	    align = bed->maxpagesize;
 
 	  adjust = vma_page_aligned_bias (m->sections[0]->vma, off, align);
 	  off += adjust;
@@ -4346,7 +4344,7 @@ assign_file_positions_for_segments (bfd 
 		  /* The section VMA must equal the file position
 		     modulo the page size.  */
 		  bfd_size_type page = align;
-		  if ((abfd->flags & D_PAGED) != 0)
+		  if ((abfd->flags & D_PAGED) != 0 && bed->maxpagesize > page)
 		    page = bed->maxpagesize;
 		  adjust = vma_page_aligned_bias (sec->vma,
 						  p->p_vaddr + p->p_memsz,

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: possible BFD regression from a change made last September

[Bob Wilson]

Alan Modra wrote:
> On Tue, May 17, 2005 at 10:15:32AM -0700, Bob Wilson wrote:
> 
>>If the page size is not 1, I can see how this case is now handled by:
>>
>>		  /* The section VMA must equal the file position
>>		     modulo the page size.  */
>>		  bfd_size_type page = align;
>>		  if ((abfd->flags & D_PAGED) != 0)
>>		    page = bed->maxpagesize;
>>		  adjust = vma_page_aligned_bias (sec->vma,
>>						  p->p_vaddr + p->p_memsz,
>>						  page);
>>		  p->p_memsz += adjust;
>>
>>but that breaks in my scenario where align == 16 but maxpagesize == 1.
> 
> 
> We ought to be taking the maximum of align and maxpagesize.  I think the
> following should do the trick.
> 
> 	* elf.c (assign_file_positions_for_segments): Use maximum of
> 	maxpagesize and section alignment when adjusting initial
> 	segment offset and section offsets.

Yes, indeed, that fixes it.  Thanks for the patch!

--Bob