Update objcopy's --section-alignment option so that it sets the alignment flag on PE sections. Add a check for aligned sections not matching their VMAs.

5 files changed, 182 insertions, 20 deletions

diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 50cc470..7946155 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -1737,6 +1737,10 @@ above.  If @var{sectionpattern} does not match any sections in the
 input file, a warning will be issued, unless
 @option{--no-change-warnings} is used.
 
+Note - changing the VMA of sections in a fully linked binary can be
+dangerous since there may be code that expects the sections to be
+located at their old address.
+
 @item --change-warnings
 @itemx --adjust-warnings
 If @option{--change-section-address} or @option{--change-section-lma} or
@@ -1766,7 +1770,14 @@ SHF_X86_64_LARGE.
 @item --set-section-alignment @var{sectionpattern}=@var{align}
 Set the alignment for any sections matching @var{sectionpattern}.
 @var{align} specifies the alignment in bytes and must be a power of
-two, i.e. 1, 2, 4, 8@dots{}. 
+two, i.e. 1, 2, 4, 8@dots{}.
+
+Note - setting a section's alignment will not automatically align its
+LMA or VMA addresses.  If those need to be changed as well then the
+@option{--change-section-lma} and/or @option{--change-section-vma}
+options should be used.  Also note that changing VMAs can cause
+problems in fully linked binaries where there may be code that expects
+the contents of the sections to be located at their old address.
 
 @item --add-section @var{sectionname}=@var{filename}
 Add a new section named @var{sectionname} while copying the file.  The
@@ -2129,11 +2140,21 @@ for dlls.
 [This option is specific to PE targets.]
 
 @item --section-alignment @var{num}
-Sets the section alignment field in the PE header.  Sections in memory
-will always begin at addresses which are a multiple of this number.
-Defaults to 0x1000.
 [This option is specific to PE targets.]
 
+Sets the section alignment field in the PE header - if one is present
+in the binary.  Sections in memory will always begin at addresses
+which are a multiple of this number.  Defaults to 0x1000.
+
+Note - this option will also set the alignment field in each section's
+flags.
+
+Note - if a section's LMA or VMA addresses are no longer aligned, and
+those addresses have not been set via the @option{--set-section-lma} or
+@option{--set-section-vma} options, and the file has been fully
+relocated then a warning message will be issued.  It will then be up
+to the user to decide if the LMA and VMA need updating.
+
 @item --stack @var{reserve}
 @itemx --stack @var{reserve},@var{commit}
 Specify the number of bytes of memory to reserve (and optionally commit)
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index a8e0f15..77ab908 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -4100,6 +4100,50 @@ setup_bfd_headers (bfd *ibfd, bfd *obfd)
   return;
 }
 
+static inline signed int
+power_of_two (bfd_vma val)
+{
+  signed int result = 0;
+
+  if (val == 0)
+    return 0;
+
+  while ((val & 1) == 0)
+    {
+      val >>= 1;
+      ++result;
+    }
+
+  if (val != 1)
+    /* Number has more than one 1, i.e. wasn't a power of 2.  */
+    return -1;
+
+  return result;
+}
+
+static unsigned int
+image_scn_align (unsigned int alignment)
+{
+  switch (alignment)
+    {
+    case 8192: return IMAGE_SCN_ALIGN_8192BYTES;
+    case 4096: return IMAGE_SCN_ALIGN_4096BYTES;
+    case 2048: return IMAGE_SCN_ALIGN_2048BYTES;
+    case 1024: return IMAGE_SCN_ALIGN_1024BYTES;
+    case  512: return IMAGE_SCN_ALIGN_512BYTES;
+    case  256: return IMAGE_SCN_ALIGN_256BYTES;
+    case  128: return IMAGE_SCN_ALIGN_128BYTES;
+    case   64: return IMAGE_SCN_ALIGN_64BYTES;
+    case   32: return IMAGE_SCN_ALIGN_32BYTES;
+    case   16: return IMAGE_SCN_ALIGN_16BYTES;
+    case    8: return IMAGE_SCN_ALIGN_8BYTES;
+    case    4: return IMAGE_SCN_ALIGN_4BYTES;
+    case    2: return IMAGE_SCN_ALIGN_2BYTES;
+    case    1: return IMAGE_SCN_ALIGN_1BYTES;
+    default: return 0;
+    }
+}
+
 /* Create a section in OBFD with the same
    name and attributes as ISECTION in IBFD.  */
 
@@ -4224,6 +4268,8 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   if (!bfd_set_section_size (osection, size))
     err = _("failed to set size");
 
+  bool vma_set_by_user = false;
+
   vma = bfd_section_vma (isection);
   p = find_section_list (bfd_section_name (isection), false,
 			 SECTION_CONTEXT_ALTER_VMA | SECTION_CONTEXT_SET_VMA);
@@ -4233,6 +4279,7 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
 	vma = p->vma_val;
       else
 	vma += p->vma_val;
+      vma_set_by_user = true;
     }
   else
     vma += change_section_address;
@@ -4240,6 +4287,8 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   if (!bfd_set_section_vma (osection, vma))
     err = _("failed to set vma");
 
+  bool lma_set_by_user = false;
+
   lma = isection->lma;
   p = find_section_list (bfd_section_name (isection), false,
 			 SECTION_CONTEXT_ALTER_LMA | SECTION_CONTEXT_SET_LMA);
@@ -4249,6 +4298,7 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
 	lma += p->lma_val;
       else
 	lma = p->lma_val;
+      lma_set_by_user = true;
     }
   else
     lma += change_section_address;
@@ -4259,6 +4309,24 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
 			 SECTION_CONTEXT_SET_ALIGNMENT);
   if (p != NULL)
     alignment = p->alignment;
+  else if (pe_section_alignment != (bfd_vma) -1
+	   && bfd_get_flavour (obfd) == bfd_target_coff_flavour)
+    {
+      alignment = power_of_two (pe_section_alignment);
+
+      if (coff_section_data (ibfd, isection))
+	{
+	  struct pei_section_tdata * pei_data = pei_section_data (ibfd, isection);
+
+	  if (pei_data != NULL)
+	    {
+	      /* Set the alignment flag of the input section, which will
+		 be copied to the output section later on.  */
+	      pei_data->pe_flags &= ~IMAGE_SCN_ALIGN_POWER_BIT_MASK;
+	      pei_data->pe_flags |= image_scn_align (pe_section_alignment);
+	    }
+	}
+    }
   else
     alignment = bfd_section_alignment (isection);
 
@@ -4267,6 +4335,32 @@ setup_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   if (!bfd_set_section_alignment (osection, alignment))
     err = _("failed to set alignment");
 
+  /* If the output section's VMA is not aligned
+     and the alignment has changed
+     and the VMA was not set by the user
+     and the section does not have relocations associated with it
+     then warn the user.  */
+  if (osection->vma & ((1 << alignment) - 1)
+      && alignment != bfd_section_alignment (isection)
+      && change_section_address == 0
+      && ! vma_set_by_user
+      && bfd_get_reloc_upper_bound (ibfd, isection) < 1)
+    {
+      non_fatal (_("output section %s's alignment does not match its VMA"), name);
+    }
+
+  /* Similar check for a non-aligned LMA.
+     FIXME: Since this is only an LMA, maybe it does not matter if
+     it is not aligned ?  */
+  if (osection->lma & ((1 << alignment) - 1)
+      && alignment != bfd_section_alignment (isection)
+      && change_section_address == 0
+      && ! lma_set_by_user
+      && bfd_get_reloc_upper_bound (ibfd, isection) < 1)
+    {
+      non_fatal (_("output section %s's alignment does not match its LMA"), name);
+    }
+
   /* Copy merge entity size.  */
   osection->entsize = isection->entsize;
 
@@ -5713,15 +5807,8 @@ copy_main (int argc, char *argv[])
 	      fatal (_("bad format for --set-section-alignment: numeric argument needed"));
 
 	    /* Convert integer alignment into a power-of-two alignment.  */
-	    palign = 0;
-	    while ((align & 1) == 0)
-	      {
-	    	align >>= 1;
-	    	++palign;
-	      }
-
-	    if (align != 1)
-	      /* Number has more than on 1, i.e. wasn't a power of 2.  */
+	    palign = power_of_two (align);
+	    if (palign == -1)
 	      fatal (_("bad format for --set-section-alignment: alignment is not a power of two"));
 
 	    /* Add the alignment setting to the section list.  */
@@ -5938,6 +6025,11 @@ copy_main (int argc, char *argv[])
 	case OPTION_PE_SECTION_ALIGNMENT:
 	  pe_section_alignment = parse_vma (optarg,
 					    "--section-alignment");
+	  if (power_of_two (pe_section_alignment) == -1)
+	    {
+	      non_fatal (_("--section-alignment argument is not a power of two: %s - ignoring"), optarg);
+	      pe_section_alignment = (bfd_vma) -1;
+	    }
 	  break;
 
 	case OPTION_SUBSYSTEM:
diff --git a/binutils/od-pe.c b/binutils/od-pe.c
index 5d2b776..28bce3b 100644
--- a/binutils/od-pe.c
+++ b/binutils/od-pe.c
@@ -138,7 +138,7 @@ static const struct xlat_table section_flag_xlat[] =
   { IMAGE_SCN_LNK_NRELOC_OVFL, "NRELOC OVFL" },
   { IMAGE_SCN_MEM_NOT_CACHED,  "NOT CACHED" },
   { IMAGE_SCN_MEM_NOT_PAGED,   "NOT PAGED" },
-  { IMAGE_SCN_MEM_SHARED,      "SHARED" },    
+  { IMAGE_SCN_MEM_SHARED,      "SHARED" },
   { 0, NULL }
 };
 
@@ -591,7 +591,44 @@ dump_pe_file_header (bfd *                            abfd,
     printf (_("\n  Optional header not present\n"));
 }
 
-/* Dump the sections header.  */
+static void
+dump_alignment (unsigned int flags)
+{
+  flags &= IMAGE_SCN_ALIGN_POWER_BIT_MASK;
+
+  if (flags == IMAGE_SCN_ALIGN_8192BYTES)
+    printf (_("Align: 8192 "));
+  else if (flags == IMAGE_SCN_ALIGN_4096BYTES)
+    printf (_("Align: 4096 "));
+  else if (flags == IMAGE_SCN_ALIGN_2048BYTES)
+    printf (_("Align: 2048 "));
+  else if (flags == IMAGE_SCN_ALIGN_1024BYTES)
+    printf (_("Align: 1024 "));
+  else if (flags == IMAGE_SCN_ALIGN_512BYTES)
+    printf (_("Align: 512 "));
+  else if (flags == IMAGE_SCN_ALIGN_256BYTES)
+    printf (_("Align: 256 "));
+  else if (flags == IMAGE_SCN_ALIGN_128BYTES)
+    printf (_("Align: 128 "));
+  else if (flags == IMAGE_SCN_ALIGN_64BYTES)
+    printf (_("Align: 64 "));
+  else if (flags == IMAGE_SCN_ALIGN_32BYTES)
+    printf (_("Align: 32 "));
+  else if (flags == IMAGE_SCN_ALIGN_16BYTES)
+    printf (_("Align: 16 "));
+  else if (flags == IMAGE_SCN_ALIGN_8BYTES)
+    printf (_("Align: 8 "));
+  else if (flags == IMAGE_SCN_ALIGN_4BYTES)
+    printf (_("Align: 4 "));
+  else if (flags == IMAGE_SCN_ALIGN_2BYTES)
+    printf (_("Align: 2 "));
+  else if (flags == IMAGE_SCN_ALIGN_1BYTES)
+    printf (_("Align: 1 "));
+  else
+    printf (_("Align: *unknown* "));
+}
+
+/* Dump the section's header.  */
 
 static void
 dump_pe_sections_header (bfd *                            abfd,
@@ -656,12 +693,14 @@ dump_pe_sections_header (bfd *                            abfd,
       else
 	printf (_("\n            Flags: %08x: "), flags);
 
-      if (flags != 0)
-        {
-	  /* Skip the alignment bits.  */
+      if (flags & IMAGE_SCN_ALIGN_POWER_BIT_MASK)
+	{
+	  dump_alignment (flags);
 	  flags &= ~ IMAGE_SCN_ALIGN_POWER_BIT_MASK;
-          dump_flags (section_flag_xlat, flags);
-        }
+	}
+
+      if (flags != 0)
+	dump_flags (section_flag_xlat, flags);
 
       putchar ('\n');
     }
diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index d884822..8b432f3 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -1463,6 +1463,7 @@ if [is_elf_format] {
 run_dump_test "pr23633"
 
 run_dump_test "set-section-alignment"
+run_dump_test "section-alignment"
 
 setup_xfail "hppa*-*-*"
 setup_xfail "spu-*-*"
diff --git a/binutils/testsuite/binutils-all/section-alignment.d b/binutils/testsuite/binutils-all/section-alignment.d
new file mode 100644
index 0000000..d62528c
--- /dev/null
+++ b/binutils/testsuite/binutils-all/section-alignment.d
@@ -0,0 +1,9 @@
+#source: pr23633.s
+#PROG: objcopy
+#objcopy: --section-alignment=512
+#objdump: -P sections
+#target: [is_pecoff_format]
+
+#...
+.* Align: 512.*
+#pass