PR20659, Objcopy and change section lma failing

Message ID 20180512065940.GB23663@bubble.grove.modra.org
State New
Headers show
Series
  • PR20659, Objcopy and change section lma failing
Related show

Commit Message

Alan Modra May 12, 2018, 6:59 a.m.
Sections may well belong in multiple segments.  The testcase in the PR
saw an allocated section being assigned to an ABIFLAGS segment, then
not being assigned to a LOAD segment because it had already been
handled.  To fix that particular problem this patch sets and tests
segment_mark only for LOAD segments.  I kept the segment_mark test for
LOAD segments because I think there may otherwise be a problem with
zero size sections.

A few other problems showed up with the testcase.  Some targets align
.dynamic, resulting in the test failing with "section .dynamic lma
0x800000c0 adjusted to 0x800000cc" and similar messages.  I've tried
to handle that with some more hacks to the segment lma, which do the
right thing for the testcase, but may well fail in other situations..

I've also removed the tests of segment lma (p_paddr) and code involved
in deciding that an adjusted segment no longer covers the file or
program headers.  Those test can't be correct in the face of objcopy
--change-section-lma.  It may be necessary to reinstate the tests but
do them modulo page size, but we'll see how this goes.

	PR 20659
bfd/
	* elf.c (rewrite_elf_program_header): Use segment_mark only for
	PT_LOAD headers.  Delete first_matching_lma and first_suggested_lma.
	Instead make matching_lma and suggested_lma pointers to the
	sections.  Align section vma and lma calculated from segment.
	Don't clear includes_phdrs or includes_filehdr based on p_paddr
	test.  Try to handle alignment padding before first section by
	adjusting new segment lma down.  Adjust PT_PHDR map p_paddr.
ld/
	* testsuite/ld-elf/changelma.d,
	* testsuite/ld-elf/changelma.lnk,
	* testsuite/ld-elf/changelma.s: New test.


-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/bfd/elf.c b/bfd/elf.c
index 3e8d5103e4..024b6cd86a 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -6641,7 +6641,7 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 	   : segment->p_vaddr != section->vma)				\
        || (strcmp (bfd_get_section_name (ibfd, section), ".dynamic")	\
 	   == 0))							\
-   && !section->segment_mark)
+   && (segment->p_type != PT_LOAD || !section->segment_mark))
 
 /* If the output section of a section in the input segment is NULL,
    it is removed from the corresponding output segment.   */
@@ -6769,13 +6769,11 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
       asection **sections;
       asection *output_section;
       unsigned int isec;
-      bfd_vma matching_lma;
-      bfd_vma suggested_lma;
+      asection *matching_lma;
+      asection *suggested_lma;
       unsigned int j;
       bfd_size_type amt;
       asection *first_section;
-      bfd_boolean first_matching_lma;
-      bfd_boolean first_suggested_lma;
 
       if (segment->p_type == PT_NULL)
 	continue;
@@ -6902,10 +6900,8 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 	 we have completely filled the segment, and there is nothing
 	 more to do.  */
       isec = 0;
-      matching_lma = 0;
-      suggested_lma = 0;
-      first_matching_lma = TRUE;
-      first_suggested_lma = TRUE;
+      matching_lma = NULL;
+      suggested_lma = NULL;
 
       for (section = first_section, j = 0;
 	   section != NULL;
@@ -6926,14 +6922,14 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 		  && !bed->want_p_paddr_set_to_zero
 		  && isec == 0
 		  && output_section->lma != 0
-		  && output_section->vma == (segment->p_vaddr
-					     + (map->includes_filehdr
-						? iehdr->e_ehsize
-						: 0)
-					     + (map->includes_phdrs
-						? (iehdr->e_phnum
-						   * iehdr->e_phentsize)
-						: 0)))
+		  && (align_power (segment->p_vaddr
+				   + (map->includes_filehdr
+				      ? iehdr->e_ehsize : 0)
+				   + (map->includes_phdrs
+				      ? iehdr->e_phnum * iehdr->e_phentsize
+				      : 0),
+				   output_section->alignment_power)
+		      == output_section->vma))
 		map->p_paddr = segment->p_vaddr;
 
 	      /* Match up the physical address of the segment with the
@@ -6943,22 +6939,17 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 		  || (bed->want_p_paddr_set_to_zero
 		      && IS_CONTAINED_BY_VMA (output_section, segment)))
 		{
-		  if (first_matching_lma || output_section->lma < matching_lma)
-		    {
-		      matching_lma = output_section->lma;
-		      first_matching_lma = FALSE;
-		    }
+		  if (matching_lma == NULL
+		      || output_section->lma < matching_lma->lma)
+		    matching_lma = output_section;
 
 		  /* We assume that if the section fits within the segment
 		     then it does not overlap any other section within that
 		     segment.  */
 		  map->sections[isec++] = output_section;
 		}
-	      else if (first_suggested_lma)
-		{
-		  suggested_lma = output_section->lma;
-		  first_suggested_lma = FALSE;
-		}
+	      else if (suggested_lma == NULL)
+		suggested_lma = output_section;
 
 	      if (j == section_count)
 		break;
@@ -6981,63 +6972,54 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 
 	  if (p_paddr_valid
 	      && !bed->want_p_paddr_set_to_zero
-	      && matching_lma != map->p_paddr
+	      && matching_lma->lma != map->p_paddr
 	      && !map->includes_filehdr
 	      && !map->includes_phdrs)
 	    /* There is some padding before the first section in the
 	       segment.  So, we must account for that in the output
 	       segment's vma.  */
-	    map->p_vaddr_offset = matching_lma - map->p_paddr;
+	    map->p_vaddr_offset = matching_lma->lma - map->p_paddr;
 
 	  free (sections);
 	  continue;
 	}
       else
 	{
-	  if (!first_matching_lma)
-	    {
-	      /* At least one section fits inside the current segment.
-		 Keep it, but modify its physical address to match the
-		 LMA of the first section that fitted.  */
-	      map->p_paddr = matching_lma;
-	    }
-	  else
-	    {
-	      /* None of the sections fitted inside the current segment.
-		 Change the current segment's physical address to match
-		 the LMA of the first section.  */
-	      map->p_paddr = suggested_lma;
-	    }
+	  /* Change the current segment's physical address to match
+	     the LMA of the first section that fitted, or if no
+	     section fitted, the first section.  */
+	  if (matching_lma == NULL)
+	    matching_lma = suggested_lma;
+
+	  map->p_paddr = matching_lma->lma;
 
 	  /* Offset the segment physical address from the lma
 	     to allow for space taken up by elf headers.  */
-	  if (map->includes_filehdr)
+	  if (map->includes_phdrs)
 	    {
-	      if (map->p_paddr >= iehdr->e_ehsize)
-		map->p_paddr -= iehdr->e_ehsize;
-	      else
-		{
-		  map->includes_filehdr = FALSE;
-		  map->includes_phdrs = FALSE;
-		}
+	      map->p_paddr -= iehdr->e_phnum * iehdr->e_phentsize;
+
+	      /* iehdr->e_phnum is just an estimate of the number
+		 of program headers that we will need.  Make a note
+		 here of the number we used and the segment we chose
+		 to hold these headers, so that we can adjust the
+		 offset when we know the correct value.  */
+	      phdr_adjust_num = iehdr->e_phnum;
+	      phdr_adjust_seg = map;
 	    }
 
-	  if (map->includes_phdrs)
+	  if (map->includes_filehdr)
 	    {
-	      if (map->p_paddr >= iehdr->e_phnum * iehdr->e_phentsize)
-		{
-		  map->p_paddr -= iehdr->e_phnum * iehdr->e_phentsize;
-
-		  /* iehdr->e_phnum is just an estimate of the number
-		     of program headers that we will need.  Make a note
-		     here of the number we used and the segment we chose
-		     to hold these headers, so that we can adjust the
-		     offset when we know the correct value.  */
-		  phdr_adjust_num = iehdr->e_phnum;
-		  phdr_adjust_seg = map;
-		}
-	      else
-		map->includes_phdrs = FALSE;
+	      bfd_vma align = (bfd_vma) 1 << matching_lma->alignment_power;
+	      map->p_paddr -= iehdr->e_ehsize;
+	      /* We've subtracted off the size of headers from the
+		 first section lma, but there may have been some
+		 alignment padding before that section too.  Try to
+		 account for that by adjusting the segment lma down to
+		 the same alignment.  */
+	      if (segment->p_align != 0 && segment->p_align < align)
+		align = segment->p_align;
+	      map->p_paddr &= -align;
 	    }
 	}
 
@@ -7052,8 +7034,7 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
       do
 	{
 	  map->count = 0;
-	  suggested_lma = 0;
-	  first_suggested_lma = TRUE;
+	  suggested_lma = NULL;
 
 	  /* Fill the current segment with sections that fit.  */
 	  for (j = 0; j < section_count; j++)
@@ -7075,12 +7056,14 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 		      /* If the first section in a segment does not start at
 			 the beginning of the segment, then something is
 			 wrong.  */
-		      if (output_section->lma
-			  != (map->p_paddr
-			      + (map->includes_filehdr ? iehdr->e_ehsize : 0)
-			      + (map->includes_phdrs
-				 ? iehdr->e_phnum * iehdr->e_phentsize
-				 : 0)))
+		      if (align_power (map->p_paddr
+				       + (map->includes_filehdr
+					  ? iehdr->e_ehsize : 0)
+				       + (map->includes_phdrs
+					  ? iehdr->e_phnum * iehdr->e_phentsize
+					  : 0),
+				       output_section->alignment_power)
+			  != output_section->lma)
 			abort ();
 		    }
 		  else
@@ -7098,11 +7081,8 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 			  || (prev_sec->lma + prev_sec->size
 			      > output_section->lma))
 			{
-			  if (first_suggested_lma)
-			    {
-			      suggested_lma = output_section->lma;
-			      first_suggested_lma = FALSE;
-			    }
+			  if (suggested_lma == NULL)
+			    suggested_lma = output_section;
 
 			  continue;
 			}
@@ -7111,13 +7091,11 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 		  map->sections[map->count++] = output_section;
 		  ++isec;
 		  sections[j] = NULL;
-		  section->segment_mark = TRUE;
-		}
-	      else if (first_suggested_lma)
-		{
-		  suggested_lma = output_section->lma;
-		  first_suggested_lma = FALSE;
+		  if (segment->p_type == PT_LOAD)
+		    section->segment_mark = TRUE;
 		}
+	      else if (suggested_lma == NULL)
+		suggested_lma = output_section;
 	    }
 
 	  BFD_ASSERT (map->count > 0);
@@ -7147,7 +7125,7 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
 	      map->p_type = segment->p_type;
 	      map->p_flags = segment->p_flags;
 	      map->p_flags_valid = 1;
-	      map->p_paddr = suggested_lma;
+	      map->p_paddr = suggested_lma->lma;
 	      map->p_paddr_valid = p_paddr_valid;
 	      map->includes_filehdr = 0;
 	      map->includes_phdrs = 0;
@@ -7173,6 +7151,15 @@  rewrite_elf_program_header (bfd *ibfd, bfd *obfd)
       if (count > phdr_adjust_num)
 	phdr_adjust_seg->p_paddr
 	  -= (count - phdr_adjust_num) * iehdr->e_phentsize;
+
+      for (map = map_first; map != NULL; map = map->next)
+	if (map->p_type == PT_PHDR)
+	  {
+	    bfd_vma adjust
+	      = phdr_adjust_seg->includes_filehdr ? iehdr->e_ehsize : 0;
+	    map->p_paddr = phdr_adjust_seg->p_paddr + adjust;
+	    break;
+	  }
     }
 
 #undef SEGMENT_END
diff --git a/ld/testsuite/ld-elf/changelma.d b/ld/testsuite/ld-elf/changelma.d
new file mode 100644
index 0000000000..858a8db37a
--- /dev/null
+++ b/ld/testsuite/ld-elf/changelma.d
@@ -0,0 +1,12 @@ 
+#name: changelma (pr20659)
+#ld: -T changelma.lnk
+#objcopy_linked_file: --change-section-lma .dynamic+0x80000000
+#readelf: -l --wide
+#xfail: rx-*-*
+# rx-elf fails since it forces vma equal to lma
+
+#...
+  PHDR +0x[0-9a-f]* 0x[0-9a-f]* 0x0*8000[0-9a-f]{4} .*
+  LOAD +0x0+ 0x0+ 0x0*80000000 .*
+  DYNAMIC +0x[0-9a-f]* 0x[0-9a-f]* 0x0*8000[0-9a-f]{4} .*
+#pass
diff --git a/ld/testsuite/ld-elf/changelma.lnk b/ld/testsuite/ld-elf/changelma.lnk
new file mode 100644
index 0000000000..fd55516fc8
--- /dev/null
+++ b/ld/testsuite/ld-elf/changelma.lnk
@@ -0,0 +1,11 @@ 
+PHDRS
+{
+  headers PT_PHDR PHDRS ;
+  text PT_LOAD FILEHDR PHDRS ;
+  dynamic PT_DYNAMIC ;
+}
+SECTIONS
+{
+  . = SIZEOF_HEADERS;
+  .dynamic : { *(*) } :text :dynamic
+}
diff --git a/ld/testsuite/ld-elf/changelma.s b/ld/testsuite/ld-elf/changelma.s
new file mode 100644
index 0000000000..458b40e6c0
--- /dev/null
+++ b/ld/testsuite/ld-elf/changelma.s
@@ -0,0 +1,2 @@ 
+ .text
+ .space 16