Commit: Cygwin/Mingw: Improve .rsrc section merging

[Nick Clifton <nickc at redhat dot com>]

Hi Guys,

  I am applying the patch below to fix a couple of problems with the
  code to create a default manifest and then merge it into a linked
  binary.

  The first problem was that the wrong version of windres was being used
  to compile the default manifest.  This was easily fixed.

  The second problem is that when .rsrc sections are linked together a
  gap is left between them based upon their alignment requirements.
  This causes a problem for the merging code as it no longer has access
  to the original input sections (in the order in which they were linked
  into the output .rsrc section).  So it has to guess at the alignment.
  A user found a test case where the original heuristic guessed
  incorrectly, so I have created an improved - but still not perfect -
  heuristic.

Cheers
  Nick

bfd/ChangeLog
2014-03-19  Nick Clifton  <nickc@redhat.com>

	* peXXigen.c (rsrc_align): New function.  Attempts to cope with
	alignment variances when .rsrc sections are merged.
	(rsrc_process_section): Use rsrc_align.

ld/ChangeLog
2014-03-19  Nick Clifton  <nickc@redhat.com>

	* Makefile.am (default-manifest.o): Use WINDRES_FOR_TARGET.
	* Makefile.in: Regenerate.
	* emultempl/default-manifest.rc: Fix typo.
	* scripttempl/pe.sc (R_RSRC): Fix default-manifest exclusion.
	(.rsrc): Add SUBALIGN(4).
	* scripttempl/pep.sc: Likewise.

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 8219ab9..e53d6c7 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -3477,6 +3477,63 @@ rsrc_merge (struct rsrc_entry * a, struct rsrc_entry * b)
   rsrc_sort_entries (& adir->ids, FALSE, adir);
 }
 
+static bfd_byte *
+rsrc_align (bfd * abfd, bfd_byte * data, bfd_byte * dataend)
+{
+  ptrdiff_t d;
+
+  /* Align the data pointer - we no longer have access to the original sections
+     so we do not know the alignment value they used.  We default to 1^2 alignment
+     but check to see if 1^3 is better.  */
+  d = (ptrdiff_t) data;
+  d = (d + 3) & ~3;
+
+  if ((bfd_byte *) d == (dataend - 4))
+    return dataend;
+      
+  if ((d & 0x4) == 0)
+    return (bfd_byte *) d;
+
+  /* Aligning to 1^3 would change the value of the pointer.  See if the
+     next 16 bytes (without aligning to 1^3) would form a valid Resource
+     Directory Table.  If not then increase the alignment.  */
+  data = (bfd_byte *) d;
+
+  if (data + 16 >= dataend)
+    /* Not enough room left for a resource table anyway.  Just stop.  */
+    return dataend;
+
+  if (bfd_get_32 (abfd, data) != 0)
+    /* A non-zero characteristics field.  This should not happen.
+       Possibly the padding between merged .rsrc sections was not zero.
+       Choose to advance the pointer.  */
+    return (bfd_byte *) (d + 4);
+
+  if (bfd_get_32 (abfd, data + 4) != 0)
+    /* A non-zero time field.  It cannot be the characteristics field
+       of a 1^3 aligned .rsrc section because the characteristics are
+       always zero.   Hence we should not increase the alignment.  */
+    return (bfd_byte *) d;
+
+  /* Looking at bytes 8..11 does not help.  These are either the time stamp
+     or the version fields.  They can both have arbitary values, and zero
+     is quite commmon, so we have no way to distinguish them.  */
+
+  /* Bytes 12..15 are either the version values or the number of entries
+     to follow.  If the value is zero then this must be the version fields,
+     since we must always have at least one entry.  A non-zero value on the
+     other hand is ambiguous.  */
+  if (bfd_get_32 (abfd, data + 12) == 0)
+    return (bfd_byte *) (d + 4);
+
+  /* Ho Hum, we have no easy way to resolve this problem, so punt for now.
+     FIXME: try parsing the entire remaining .rsrc section.  If it fails,
+     try re-aligning data and reparsing.  If that works go with the new
+     alignment.  Note - this has the potential to be dangerously recursive.  */
+
+  return (bfd_byte *) d;
+}
+
 /* Check the .rsrc section.  If it contains multiple concatenated
    resources then we must merge them properly.  Otherwise Windows
    will ignore all but the first set.  */
@@ -3540,13 +3598,8 @@ rsrc_process_section (bfd * abfd,
 	  goto end;
 	}
 
-      /* Align the data pointer - we assume 1^2 alignment.  */
-      data = (bfd_byte *) (((ptrdiff_t) (data + 3)) & ~ 3);
+      data = rsrc_align (abfd, data, dataend);
       rva_bias += data - p;
-
-      if (data == (dataend - 4))
-	data = dataend;
-
       ++ num_resource_sets;
     }
 
@@ -3569,10 +3622,8 @@ rsrc_process_section (bfd * abfd,
 
       data = rsrc_parse_directory (abfd, type_tables + indx, data, data,
 				   dataend, rva_bias, NULL);
-      data = (bfd_byte *) (((ptrdiff_t) (data + 3)) & ~ 3);
+      data = rsrc_align (abfd, data, dataend);
       rva_bias += data - p;
-      if (data == (dataend - 4))
-	data = dataend;
       indx ++;
     }
   BFD_ASSERT (indx == num_resource_sets);
diff --git a/ld/Makefile.am b/ld/Makefile.am
index e89a6c1..795663f 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -1927,7 +1927,7 @@ eshlelf64_nbsd.c: $(srcdir)/emulparams/shlelf64_nbsd.sh \
 
 # Rule to create a manifest file for Cygwin and Mingw.
 default-manifest.o: $(srcdir)/emultempl/default-manifest.rc
-	${WINDRES} -o $@ $<
+	${WINDRES_FOR_TARGET} -o $@ $<
 
 # We need this for automake to use YLWRAP.
 EXTRA_ld_new_SOURCES = deffilep.y ldlex.l
diff --git a/ld/Makefile.in b/ld/Makefile.in
index 0c14694..3c9f8f4 100644
--- a/ld/Makefile.in
+++ b/ld/Makefile.in
@@ -3359,7 +3359,7 @@ eshlelf64_nbsd.c: $(srcdir)/emulparams/shlelf64_nbsd.sh \
 
 # Rule to create a manifest file for Cygwin and Mingw.
 default-manifest.o: $(srcdir)/emultempl/default-manifest.rc
-	${WINDRES} -o $@ $<
+	${WINDRES_FOR_TARGET} -o $@ $<
 
 check-DEJAGNU: site.exp
 	srcroot=`cd $(srcdir) && pwd`; export srcroot; \
diff --git a/ld/emultempl/default-manifest.rc b/ld/emultempl/default-manifest.rc
index a4d303f..122f5e8 100644
--- a/ld/emultempl/default-manifest.rc
+++ b/ld/emultempl/default-manifest.rc
@@ -1,7 +1,7 @@
 LANGUAGE 0, 0
 
 /* CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST MOVEABLE PURE DISCARDABLE */
-1 9 MOVEABLE PURE DISCARDABLE
+1 24 MOVEABLE PURE DISCARDABLE
 BEGIN
   "<?xml version=""1.0"" encoding=""UTF-8"" standalone=""yes""?>\n"
   "<assembly xmlns=""urn:schemas-microsoft-com:asm.v1"" manifestVersion=""1.0"">\n"
diff --git a/ld/scripttempl/pe.sc b/ld/scripttempl/pe.sc
index 6cf59ea..4a1951c 100644
--- a/ld/scripttempl/pe.sc
+++ b/ld/scripttempl/pe.sc
@@ -54,7 +54,7 @@ if test "${RELOCATING}"; then
          binaries to run under Windows 8 (or later).  It is included as
          the last resource file so that if the application has provided
          its own manifest then that one will take precedence.  */
-      *(EXCLUDE_FILE ($DEFAULT_MANIFEST) .rsrc)
+      *(EXCLUDE_FILE (*$DEFAULT_MANIFEST) .rsrc)
       *(SORT(.rsrc*))
       KEEP ($DEFAULT_MANIFEST(.rsrc))"
   fi
@@ -214,7 +214,7 @@ SECTIONS
     ${RELOCATING+ __end__ = .;}
   }
 
-  .rsrc ${RELOCATING+BLOCK(__section_alignment__)} :
+  .rsrc ${RELOCATING+BLOCK(__section_alignment__)} : SUBALIGN(4)
   {
     ${R_RSRC}
   }
diff --git a/ld/scripttempl/pep.sc b/ld/scripttempl/pep.sc
index 324a743..592489a 100644
--- a/ld/scripttempl/pep.sc
+++ b/ld/scripttempl/pep.sc
@@ -54,7 +54,7 @@ if test "${RELOCATING}"; then
          binaries to run under Windows 8 (or later).  It is included as
          the last resource file so that if the application has provided
          its own manifest then that one will take precedence.  */
-      *(EXCLUDE_FILE ($DEFAULT_MANIFEST) .rsrc)
+      *(EXCLUDE_FILE (*$DEFAULT_MANIFEST) .rsrc)
       *(SORT(.rsrc*))
       KEEP ($DEFAULT_MANIFEST(.rsrc))"
   fi
@@ -219,7 +219,7 @@ SECTIONS
     ${RELOCATING+ __end__ = .;}
   }
 
-  .rsrc ${RELOCATING+BLOCK(__section_alignment__)} :
+  .rsrc ${RELOCATING+BLOCK(__section_alignment__)} : SUBALIGN(4)
   {
     ${R_RSRC}
   }