From 3714081cb37fc60f3262b4c64e81539eb4f3592f Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Thu, 24 Apr 2014 11:15:43 +0100 Subject: [PATCH] Fix PE/COFF resource merging problems. There were two issues: 1. Strings (and then resource data) must follow immediately after the end of the tables. 2. Units of resource data must be 8-byte aligned. PR ld/16807 * peXXigen.c (struct rsrc_regions): New structure. (rsrc_print_resource_directory): Use new structure. Include offset of directory in listing. (rsrc_print_resource_entry): Likewise. (rsrc_print_section): Likewise. (rsrc_count_entries): Do not increment sizeof_strings or sizeof_leaves. (rsrc_count_directory): Do not increment sizeof_tables. (rsrc_compute_region_sizes): New function. (rsrc_write_leaf): Maintain 8-byte alignment for resource data. (rsrc_process_section): Compute size of regions after merging entries. --- bfd/ChangeLog | 16 +++++ bfd/peXXigen.c | 205 +++++++++++++++++++++++++++++++++++---------------------- 2 files changed, 144 insertions(+), 77 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 89c496f..cd439b1 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,19 @@ +2014-04-24 Nick Clifton + + PR ld/16807 + * peXXigen.c (struct rsrc_regions): New structure. + (rsrc_print_resource_directory): Use new structure. Include + offset of directory in listing. + (rsrc_print_resource_entry): Likewise. + (rsrc_print_section): Likewise. + (rsrc_count_entries): Do not increment sizeof_strings or + sizeof_leaves. + (rsrc_count_directory): Do not increment sizeof_tables. + (rsrc_compute_region_sizes): New function. + (rsrc_write_leaf): Maintain 8-byte alignment for resource data. + (rsrc_process_section): Compute size of regions after merging + entries. + 2014-04-23 Alan Modra PR ld/16787 diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index acd4278..5e97588 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -212,7 +212,7 @@ abs_finder (bfd * abfd ATTRIBUTE_UNUSED, asection * sec, void * data) { bfd_vma abs_val = * (bfd_vma *) data; - return (sec->vma <= abs_val) && ((sec->vma + (1LL << 32)) > abs_val); + return (sec->vma <= abs_val) && ((sec->vma + (1ULL << 32)) > abs_val); } unsigned int @@ -2145,48 +2145,61 @@ pe_print_reloc (bfd * abfd, void * vfile) return TRUE; } +/* A data structure describing the regions of a .rsrc section. + Some fields are filled in as the section is parsed. */ + +typedef struct rsrc_regions +{ + bfd_byte * section_start; + bfd_byte * section_end; + bfd_byte * strings_start; + bfd_byte * resource_start; +} rsrc_regions; static bfd_byte * -rsrc_print_resource_directory (FILE * , bfd *, unsigned int, - bfd_byte *, bfd_byte *, bfd_byte *, bfd_vma); +rsrc_print_resource_directory (FILE * , bfd *, unsigned int, bfd_byte *, + rsrc_regions *, bfd_vma); static bfd_byte * -rsrc_print_resource_entries (FILE * file, - bfd * abfd, - unsigned int indent, - bfd_boolean is_name, - bfd_byte * datastart, - bfd_byte * data, - bfd_byte * dataend, - bfd_vma rva_bias) +rsrc_print_resource_entries (FILE * file, + bfd * abfd, + unsigned int indent, + bfd_boolean is_name, + bfd_byte * data, + rsrc_regions * regions, + bfd_vma rva_bias) { unsigned long entry, addr, size; - if (data + 8 >= dataend) - return dataend + 1; + if (data + 8 >= regions->section_end) + return regions->section_end + 1; - fprintf (file, _("%*.s Entry: "), indent, " "); + fprintf (file, _("%03x %*.s Entry: "), (int)(data - regions->section_start), indent, " "); entry = (long) bfd_get_32 (abfd, data); if (is_name) { bfd_byte * name; - /* Note - the documenation says that this field is an RVA value + /* Note - the documentation says that this field is an RVA value but windres appears to produce a section relative offset with the top bit set. Support both styles for now. */ if (HighBitSet (entry)) - name = datastart + WithoutHighBit (entry); + name = regions->section_start + WithoutHighBit (entry); else - name = datastart + entry - rva_bias; + name = regions->section_start + entry - rva_bias; - if (name + 2 < dataend) + if (name + 2 < regions->section_end) { unsigned int len; + + if (regions->strings_start == NULL) + regions->strings_start = name; + len = bfd_get_16 (abfd, name); fprintf (file, _("name: [val: %08lx len %d]: "), entry, len); - if (name + 2 + len * 2 < dataend) + if (name + 2 + len * 2 < regions->section_end) { /* This strange loop is to cope with multibyte characters. */ while (len --) @@ -2209,47 +2222,49 @@ rsrc_print_resource_entries (FILE * file, if (HighBitSet (entry)) return rsrc_print_resource_directory (file, abfd, indent + 1, - datastart, - datastart + WithoutHighBit (entry), - dataend, rva_bias); + regions->section_start + WithoutHighBit (entry), + regions, rva_bias); - if (datastart + entry + 16 >= dataend) - return dataend + 1; + if (regions->section_start + entry + 16 >= regions->section_end) + return regions->section_end + 1; - fprintf (file, _("%*.s Leaf: Addr: %#08lx, Size: %#08lx, Codepage: %d\n"), + fprintf (file, _("%03x %*.s Leaf: Addr: %#08lx, Size: %#08lx, Codepage: %d\n"), + (int) (entry), indent, " ", - addr = (long) bfd_get_32 (abfd, datastart + entry), - size = (long) bfd_get_32 (abfd, datastart + entry + 4), - (int) bfd_get_32 (abfd, datastart + entry + 8)); + addr = (long) bfd_get_32 (abfd, regions->section_start + entry), + size = (long) bfd_get_32 (abfd, regions->section_start + entry + 4), + (int) bfd_get_32 (abfd, regions->section_start + entry + 8)); /* Check that the reserved entry is 0. */ - if (bfd_get_32 (abfd, datastart + entry + 12) != 0 + if (bfd_get_32 (abfd, regions->section_start + entry + 12) != 0 /* And that the data address/size is valid too. */ - || (datastart + (addr - rva_bias) + size > dataend)) - return dataend + 1; + || (regions->section_start + (addr - rva_bias) + size > regions->section_end)) + return regions->section_end + 1; - return datastart + (addr - rva_bias) + size; + if (regions->resource_start == NULL) + regions->resource_start = regions->section_start + (addr - rva_bias); + + return regions->section_start + (addr - rva_bias) + size; } #define max(a,b) ((a) > (b) ? (a) : (b)) #define min(a,b) ((a) < (b) ? (a) : (b)) static bfd_byte * -rsrc_print_resource_directory (FILE * file, - bfd * abfd, - unsigned int indent, - bfd_byte * datastart, - bfd_byte * data, - bfd_byte * dataend, - bfd_vma rva_bias) +rsrc_print_resource_directory (FILE * file, + bfd * abfd, + unsigned int indent, + bfd_byte * data, + rsrc_regions * regions, + bfd_vma rva_bias) { unsigned int num_names, num_ids; bfd_byte * highest_data = data; - if (data + 16 >= dataend) - return dataend + 1; + if (data + 16 >= regions->section_end) + return regions->section_end + 1; - fprintf (file, "%*.s ", indent, " "); + fprintf (file, "%03x %*.s ", (int)(data - regions->section_start), indent, " "); switch (indent) { case 0: fprintf (file, "Type"); break; @@ -2272,10 +2287,10 @@ rsrc_print_resource_directory (FILE * file, bfd_byte * entry_end; entry_end = rsrc_print_resource_entries (file, abfd, indent + 1, TRUE, - datastart, data, dataend, rva_bias); + data, regions, rva_bias); data += 8; highest_data = max (highest_data, entry_end); - if (entry_end >= dataend) + if (entry_end >= regions->section_end) return entry_end; } @@ -2284,11 +2299,10 @@ rsrc_print_resource_directory (FILE * file, bfd_byte * entry_end; entry_end = rsrc_print_resource_entries (file, abfd, indent + 1, FALSE, - datastart, data, dataend, - rva_bias); + data, regions, rva_bias); data += 8; highest_data = max (highest_data, entry_end); - if (entry_end >= dataend) + if (entry_end >= regions->section_end) return entry_end; } @@ -2308,8 +2322,7 @@ rsrc_print_section (bfd * abfd, void * vfile) bfd_size_type datasize; asection * section; bfd_byte * data; - bfd_byte * dataend; - bfd_byte * datastart; + rsrc_regions regions; pe = pe_data (abfd); if (pe == NULL) @@ -2333,20 +2346,22 @@ rsrc_print_section (bfd * abfd, void * vfile) free (data); return FALSE; } - datastart = data; - dataend = data + datasize; + + regions.section_start = data; + regions.section_end = data + datasize; + regions.strings_start = NULL; + regions.resource_start = NULL; fflush (file); fprintf (file, "\nThe .rsrc Resource Directory section:\n"); - while (data < dataend) + while (data < regions.section_end) { bfd_byte * p = data; - data = rsrc_print_resource_directory (file, abfd, 0, data, data, - dataend, rva_bias); + data = rsrc_print_resource_directory (file, abfd, 0, data, & regions, rva_bias); - if (data == dataend + 1) + if (data == regions.section_end + 1) fprintf (file, _("Corrupt .rsrc section detected!\n")); else { @@ -2360,14 +2375,19 @@ rsrc_print_section (bfd * abfd, void * vfile) aligned to a 1^3 boundary even when their alignment is set at 1^2. Catch that case here before we issue a spurious warning message. */ - if (data == (dataend - 4)) - data = dataend; - else if (data < dataend) + if (data == (regions.section_end - 4)) + data = regions.section_end; + else if (data < regions.section_end) fprintf (file, _("\nWARNING: Extra data in .rsrc section - it will be ignored by Windows:\n")); } } - free (datastart); + if (regions.strings_start != NULL) + fprintf (file, " String table starts at %lx\n", regions.strings_start - regions.section_start); + if (regions.resource_start != NULL) + fprintf (file, " Resources start at %lx\n", regions.resource_start - regions.section_start); + + free (regions.section_start); return TRUE; } @@ -2803,8 +2823,6 @@ rsrc_count_entries (bfd * abfd, unsigned int len = bfd_get_16 (abfd, name); if (len == 0 || len > 256) return dataend + 1; - - sizeof_strings += (len + 1) * 2; } entry = (long) bfd_get_32 (abfd, data + 4); @@ -2821,8 +2839,6 @@ rsrc_count_entries (bfd * abfd, addr = (long) bfd_get_32 (abfd, datastart + entry); size = (long) bfd_get_32 (abfd, datastart + entry + 4); - sizeof_leaves += 16; - return datastart + addr - rva_bias + size; } @@ -2845,7 +2861,6 @@ rsrc_count_directory (bfd * abfd, num_entries += num_ids; data += 16; - sizeof_tables_and_entries += 16; while (num_entries --) { @@ -2854,7 +2869,6 @@ rsrc_count_directory (bfd * abfd, entry_end = rsrc_count_entries (abfd, num_entries >= num_ids, datastart, data, dataend, rva_bias); data += 8; - sizeof_tables_and_entries += 8; highest_data = max (highest_data, entry_end); if (entry_end >= dataend) break; @@ -3116,7 +3130,9 @@ rsrc_write_leaf (rsrc_write_data * data, data->next_leaf += 16; memcpy (data->next_data, leaf->data, leaf->size); - data->next_data += leaf->size; + /* An undocumented feature of Windows resources is that each unit + of raw data is 8-byte aligned... */ + data->next_data += ((leaf->size + 7) & ~7); } static void rsrc_write_directory (rsrc_write_data *, rsrc_directory *); @@ -3151,6 +3167,39 @@ rsrc_write_entry (rsrc_write_data * data, } static void +rsrc_compute_region_sizes (rsrc_directory * dir) +{ + struct rsrc_entry * entry; + + if (dir == NULL) + return; + + sizeof_tables_and_entries += 16; + + for (entry = dir->names.first_entry; entry != NULL; entry = entry->next_entry) + { + sizeof_tables_and_entries += 8; + + sizeof_strings += (entry->name_id.name.len + 1) * 2; + + if (entry->is_dir) + rsrc_compute_region_sizes (entry->value.directory); + else + sizeof_leaves += 16; + } + + for (entry = dir->ids.first_entry; entry != NULL; entry = entry->next_entry) + { + sizeof_tables_and_entries += 8; + + if (entry->is_dir) + rsrc_compute_region_sizes (entry->value.directory); + else + sizeof_leaves += 16; + } +} + +static void rsrc_write_directory (rsrc_write_data * data, rsrc_directory * dir) { @@ -3177,6 +3226,7 @@ rsrc_write_directory (rsrc_write_data * data, i > 0 && entry != NULL; i--, entry = entry->next_entry) { + BFD_ASSERT (entry->is_name); rsrc_write_entry (data, next_entry, entry); next_entry += 8; } @@ -3187,6 +3237,7 @@ rsrc_write_directory (rsrc_write_data * data, i > 0 && entry != NULL; i--, entry = entry->next_entry) { + BFD_ASSERT (! entry->is_name); rsrc_write_entry (data, next_entry, entry); next_entry += 8; } @@ -3551,7 +3602,7 @@ rsrc_sort_entries (rsrc_dir_chain * chain, resource manifests - there can only be one of these, even if they differ in language. Zero-language manifests are assumed to be default manifests (provided by the - cygwin build system) and these can be silently dropped, + Cygwin/MinGW build system) and these can be silently dropped, unless that would reduce the number of manifests to zero. There should only ever be one non-zero lang manifest - if there are more it is an error. A non-zero lang @@ -3591,7 +3642,6 @@ rsrc_sort_entries (rsrc_dir_chain * chain, /* Unhook NEXT from the chain. */ /* FIXME: memory loss here. */ - /* FIXME: do we need to decrement sizeof_tables_and_entries ? */ entry->next_entry = next->next_entry; chain->num_entries --; if (chain->num_entries < 2) @@ -3655,7 +3705,6 @@ rsrc_sort_entries (rsrc_dir_chain * chain, } /* Unhook NEXT from the chain. */ - /* FIXME: do we need to decrement sizeof_tables_and_entries ? */ entry->next_entry = next->next_entry; chain->num_entries --; if (chain->num_entries < 2) @@ -3826,7 +3875,6 @@ rsrc_process_section (bfd * abfd, leaves and data and decide if we need to do anything. */ dataend = data + size; num_resource_sets = 0; - sizeof_leaves = sizeof_strings = sizeof_tables_and_entries = 0; while (data < dataend) { @@ -3912,16 +3960,19 @@ rsrc_process_section (bfd * abfd, rsrc_sort_entries (& new_table.ids, FALSE, & new_table); /* Step four: Create new contents for the .rsrc section. */ + /* Step four point one: Compute the size of each region of the .rsrc section. + We do this now, rather than earlier, as the merging above may have dropped + some entries. */ + sizeof_leaves = sizeof_strings = sizeof_tables_and_entries = 0; + rsrc_compute_region_sizes (& new_table); + /* We increment sizeof_strings to make sure that resource data + starts on an 8-byte boundary. FIXME: Is this correct ? */ + sizeof_strings = (sizeof_strings + 7) & ~ 7; + new_data = bfd_malloc (size); if (new_data == NULL) goto end; - /* We have merged the top level Type Tables of all of the input - .rsrc sections into one Type Table. So we can (and must) - reduce the count of the number of tables that we will be - emitting appropriately. */ - sizeof_tables_and_entries -= 16 * (num_resource_sets - 1); - write_data.abfd = abfd; write_data.datastart = new_data; write_data.next_table = new_data; @@ -3939,7 +3990,7 @@ rsrc_process_section (bfd * abfd, sec->size = sec->rawsize = size; end: - /* Step size: Free all the memory that we have used. */ + /* Step six: Free all the memory that we have used. */ /* FIXME: Free the resource tree, if we have one. */ free (datastart); free (rsrc_sizes); -- 2.7.4