Re: PATCH: Short circuit when copying ELF program header

["H. J. Lu" <hjl at lucon dot org>]

On Wed, Jan 24, 2007 at 09:34:18AM -0800, H. J. Lu wrote:
> On Wed, Jan 24, 2007 at 04:21:45PM +0000, Nick Clifton wrote:
> > Hi H.J.
> > 
> > >When we copy sections in a segment, we can stop when all sections
> > >in a segment are accounted for.
> > 
> > Looks good, but how did you test it ?
> > 
> > >2007-01-23  H.J. Lu  <hongjiu.lu@intel.com>
> > >
> > >	* elf.c (copy_elf_program_header): Stop when all sections in
> > >	a segment are ccounted for.
> > 
> > Approved once/if testing has shown that it introduces no regressions.
> > 
> 
> Here is an updated version. We only need to start from the first
> section in a segment. There are many ld testcases which test
> objcopy to see if the input program header is identical to the
> output. If you make a mistake in copy_elf_program_header, some of
> those testcases will fail.
> 
> It is tested on ia32, x86-64 and ia64. Is this OK to install OK?
> 
> Thanks.
> 
> 

Here is an updated patch.  Nathan, it is based on your patch

http://sourceware.org/ml/binutils/2006-12/msg00165.html

The main change is

-	      if (!section_count || section->lma < first_lma)
-		first_lma = section->lma;
+	      if (!first_section)
+		first_section = section;

I believe that the sections are ordered by lma so that first_lma is
the lma of the first section.  Can you verify it on arm-wrs-vxworks
since there is no testcase in binutils for your change?

Thanks.


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

	* elf.c (copy_elf_program_header): Start from the first section
	in a segment and stop when all sections in a segment are
	accounted for.

--- bfd/elf.c.speed	2007-01-24 18:32:10.000000000 -0800
+++ bfd/elf.c	2007-01-25 05:54:58.000000000 -0800
@@ -5966,7 +5966,7 @@ copy_elf_program_header (bfd *ibfd, bfd 
       unsigned int section_count;
       bfd_size_type amt;
       Elf_Internal_Shdr *this_hdr;
-      bfd_vma first_lma = 0;
+      asection *first_section = NULL;
 
       /* FIXME: Do we need to copy PT_NULL segment?  */
       if (segment->p_type == PT_NULL)
@@ -5980,8 +5980,8 @@ copy_elf_program_header (bfd *ibfd, bfd 
 	  this_hdr = &(elf_section_data(section)->this_hdr);
 	  if (ELF_IS_SECTION_IN_SEGMENT_FILE (this_hdr, segment))
 	    {
-	      if (!section_count || section->lma < first_lma)
-		first_lma = section->lma;
+	      if (!first_section)
+		first_section = section;
 	      section_count++;
 	    }
 	}
@@ -6027,19 +6027,24 @@ copy_elf_program_header (bfd *ibfd, bfd 
 
       if (!map->includes_phdrs && !map->includes_filehdr)
 	/* There is some other padding before the first section.  */
-	map->p_vaddr_offset = first_lma - segment->p_paddr;
+	map->p_vaddr_offset = ((first_section ? first_section->lma : 0)
+			       - segment->p_paddr);
       
       if (section_count != 0)
 	{
 	  unsigned int isec = 0;
 
-	  for (section = ibfd->sections;
+	  for (section = first_section;
 	       section != NULL;
 	       section = section->next)
 	    {
 	      this_hdr = &(elf_section_data(section)->this_hdr);
 	      if (ELF_IS_SECTION_IN_SEGMENT_FILE (this_hdr, segment))
-		map->sections[isec++] = section->output_section;
+		{
+		  map->sections[isec++] = section->output_section;
+		  if (isec == section_count)
+		    break;
+		}
 	    }
 	}