diff mbox series

PE/COFF: raise normal PE section limit safely

Message ID 20260630205916.1179-3-oleg.tolmatcev@gmail.com
State New
Headers
Series PE/COFF: raise normal PE section limit safely |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Oleg Tolmatcev June 30, 2026, 8:59 p.m. UTC 
  PE/COFF stores symbol section numbers in a 16-bit field.  Binutils used
signed 16-bit handling there, which limited normal PE objects to 32767
sections even though MSVC and Clang already accept a larger unsigned
range.

Raise the normal PE section limit to 65279, while keeping the PE/COFF
special section-number values for undefined, absolute and debug symbols
working correctly.  Do this by decoding and encoding normal PE symbol
section numbers as unsigned values in the ordinary range, but preserving
the reserved PE constants explicitly.

Also add a gas test that exercises a normal PE object above the old
32767-section limit and checks that objdump reports the high section
number correctly.

bfd/ChangeLog:

	* coffcode.h (COFF_DEFAULT_MAX_NSCNS): Define.
	(bfd_coff_std_swap_table): Use it for the default maximum section
	count.
	(ticoff0_swap_table): Likewise.
	(ticoff1_swap_table): Likewise.
	* peXXigen.c (pe_decode_sym_section_number): New function.
	(pe_encode_sym_section_number): New function.
	(_bfd_XXi_swap_sym_in): Use pe_decode_sym_section_number.
	(_bfd_XXi_swap_sym_out): Use pe_encode_sym_section_number.

include/ChangeLog:

	* coff/pe.h (IMAGE_SYM_UNDEFINED): Define.
	(IMAGE_SYM_ABSOLUTE): Define.
	(IMAGE_SYM_DEBUG): Define.
	(IMAGE_SYM_SECTION_MAX): Define.

gas/ChangeLog:

	* testsuite/gas/pe/pe.exp: Run large-obj-normal.
	* testsuite/gas/pe/large-obj-normal.s: New test.
	* testsuite/gas/pe/large-obj-normal.d: New test.

Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>
---
 bfd/coffcode.h                          | 12 +++++--
 bfd/peXXigen.c                          | 47 +++++++++++++++++++++++--
 gas/testsuite/gas/pe/large-obj-normal.d | 17 +++++++++
 gas/testsuite/gas/pe/large-obj-normal.s | 16 +++++++++
 gas/testsuite/gas/pe/pe.exp             |  1 +
 include/coff/pe.h                       |  5 +++
 6 files changed, 93 insertions(+), 5 deletions(-)
 create mode 100644 gas/testsuite/gas/pe/large-obj-normal.d
 create mode 100644 gas/testsuite/gas/pe/large-obj-normal.s

Comments

Jan Beulich July 1, 2026, 6:55 a.m. UTC | #1 
On 30.06.2026 22:59, Oleg Tolmatcev wrote:
> PE/COFF stores symbol section numbers in a 16-bit field.  Binutils used
> signed 16-bit handling there, which limited normal PE objects to 32767
> sections even though MSVC and Clang already accept a larger unsigned
> range.
> 
> Raise the normal PE section limit to 65279, while keeping the PE/COFF
> special section-number values for undefined, absolute and debug symbols
> working correctly.  Do this by decoding and encoding normal PE symbol
> section numbers as unsigned values in the ordinary range, but preserving
> the reserved PE constants explicitly.
> 
> Also add a gas test that exercises a normal PE object above the old
> 32767-section limit and checks that objdump reports the high section
> number correctly.
> 
> bfd/ChangeLog:
> 
> 	* coffcode.h (COFF_DEFAULT_MAX_NSCNS): Define.
> 	(bfd_coff_std_swap_table): Use it for the default maximum section
> 	count.
> 	(ticoff0_swap_table): Likewise.
> 	(ticoff1_swap_table): Likewise.
> 	* peXXigen.c (pe_decode_sym_section_number): New function.
> 	(pe_encode_sym_section_number): New function.
> 	(_bfd_XXi_swap_sym_in): Use pe_decode_sym_section_number.
> 	(_bfd_XXi_swap_sym_out): Use pe_encode_sym_section_number.
> 
> include/ChangeLog:
> 
> 	* coff/pe.h (IMAGE_SYM_UNDEFINED): Define.
> 	(IMAGE_SYM_ABSOLUTE): Define.
> 	(IMAGE_SYM_DEBUG): Define.
> 	(IMAGE_SYM_SECTION_MAX): Define.
> 
> gas/ChangeLog:
> 
> 	* testsuite/gas/pe/pe.exp: Run large-obj-normal.
> 	* testsuite/gas/pe/large-obj-normal.s: New test.
> 	* testsuite/gas/pe/large-obj-normal.d: New test.
> 
> Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>

Looks largely okay to me, there's just one concern I have:

> --- a/bfd/peXXigen.c
> +++ b/bfd/peXXigen.c
> @@ -109,6 +109,49 @@
>  #define SetHighBit(val)      ((val) | 0x80000000)
>  #define WithoutHighBit(val)  ((val) & 0x7fffffff)
>  
> +static int
> +pe_decode_sym_section_number (bfd *abfd, const char *raw_scnum)
> +{
> +  unsigned int scnum = H_GET_16 (abfd, raw_scnum);
> +
> +  switch (scnum)
> +    {
> +    case IMAGE_SYM_UNDEFINED:
> +      return N_UNDEF;
> +    case IMAGE_SYM_ABSOLUTE:
> +      return N_ABS;
> +    case IMAGE_SYM_DEBUG:
> +      return N_DEBUG;
> +    default:
> +      return scnum;
> +    }
> +}

When coming here for objcopy, upon reading the input we blindly accept any
other values in the reserved range. Then ...

> +static void
> +pe_encode_sym_section_number (bfd *abfd, int scnum, char *raw_scnum)
> +{
> +  unsigned int encoded_scnum;
> +
> +  switch (scnum)
> +    {
> +    case N_UNDEF:
> +      encoded_scnum = IMAGE_SYM_UNDEFINED;
> +      break;
> +    case N_ABS:
> +      encoded_scnum = IMAGE_SYM_ABSOLUTE;
> +      break;
> +    case N_DEBUG:
> +      encoded_scnum = IMAGE_SYM_DEBUG;
> +      break;
> +    default:
> +      BFD_ASSERT (scnum > 0 && (unsigned int) scnum <= IMAGE_SYM_SECTION_MAX);

... upon writing we'd stumble over this assertion. Assertions really should
be about internal state only, not about input we consumed.

Jan
diff mbox series

Patch

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 12cc7c3ca7..267c6ce278 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -376,6 +376,12 @@  extern const bfd_target TARGET_SYM_BIG;
 # define COFF_WITH_EXTENDED_RELOC_COUNTER
 #endif
 
+#if defined(COFF_WITH_PE) && !defined(COFF_IMAGE_WITH_PE)
+# define COFF_DEFAULT_MAX_NSCNS (IMAGE_SYM_SECTION_MAX + 1)
+#else
+# define COFF_DEFAULT_MAX_NSCNS 32768
+#endif
+
 #if defined (COFF_LONG_SECTION_NAMES)
 /* Needed to expand the inputs to BLANKOR1TOODD.  */
 #define COFFLONGSECTIONCATHELPER(x,y)    x ## y
@@ -5615,7 +5621,7 @@  static const bfd_coff_backend_data bfd_coff_std_swap_table ATTRIBUTE_UNUSED =
 #else
   2,
 #endif
-  32768,
+  COFF_DEFAULT_MAX_NSCNS,
   coff_SWAP_filehdr_in, coff_SWAP_aouthdr_in, coff_SWAP_scnhdr_in,
   coff_SWAP_reloc_in, coff_bad_format_hook, coff_set_arch_mach_hook,
   coff_mkobject_hook, styp_to_sec_flags, coff_set_alignment_hook,
@@ -5656,7 +5662,7 @@  static const bfd_coff_backend_data ticoff0_swap_table =
 #else
   2,
 #endif
-  32768,
+  COFF_DEFAULT_MAX_NSCNS,
   coff_SWAP_filehdr_in, coff_SWAP_aouthdr_in, coff_SWAP_scnhdr_in,
   coff_swap_reloc_v0_in, ticoff0_bad_format_hook, coff_set_arch_mach_hook,
   coff_mkobject_hook, styp_to_sec_flags, coff_set_alignment_hook,
@@ -5698,7 +5704,7 @@  static const bfd_coff_backend_data ticoff1_swap_table =
 #else
   2,
 #endif
-  32768,
+  COFF_DEFAULT_MAX_NSCNS,
   coff_SWAP_filehdr_in, coff_SWAP_aouthdr_in, coff_SWAP_scnhdr_in,
   coff_SWAP_reloc_in, ticoff1_bad_format_hook, coff_set_arch_mach_hook,
   coff_mkobject_hook, styp_to_sec_flags, coff_set_alignment_hook,
diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index bdb23dcbca..6ceb04378c 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -109,6 +109,49 @@ 
 #define SetHighBit(val)      ((val) | 0x80000000)
 #define WithoutHighBit(val)  ((val) & 0x7fffffff)
 
+static int
+pe_decode_sym_section_number (bfd *abfd, const char *raw_scnum)
+{
+  unsigned int scnum = H_GET_16 (abfd, raw_scnum);
+
+  switch (scnum)
+    {
+    case IMAGE_SYM_UNDEFINED:
+      return N_UNDEF;
+    case IMAGE_SYM_ABSOLUTE:
+      return N_ABS;
+    case IMAGE_SYM_DEBUG:
+      return N_DEBUG;
+    default:
+      return scnum;
+    }
+}
+
+static void
+pe_encode_sym_section_number (bfd *abfd, int scnum, char *raw_scnum)
+{
+  unsigned int encoded_scnum;
+
+  switch (scnum)
+    {
+    case N_UNDEF:
+      encoded_scnum = IMAGE_SYM_UNDEFINED;
+      break;
+    case N_ABS:
+      encoded_scnum = IMAGE_SYM_ABSOLUTE;
+      break;
+    case N_DEBUG:
+      encoded_scnum = IMAGE_SYM_DEBUG;
+      break;
+    default:
+      BFD_ASSERT (scnum > 0 && (unsigned int) scnum <= IMAGE_SYM_SECTION_MAX);
+      encoded_scnum = scnum;
+      break;
+    }
+
+  H_PUT_16 (abfd, encoded_scnum, raw_scnum);
+}
+
 void
 _bfd_XXi_swap_sym_in (bfd * abfd, void * ext1, void * in1)
 {
@@ -124,7 +167,7 @@  _bfd_XXi_swap_sym_in (bfd * abfd, void * ext1, void * in1)
     memcpy (in->_n._n_name, ext->e.e_name, SYMNMLEN);
 
   in->n_value = H_GET_32 (abfd, ext->e_value);
-  in->n_scnum = (short) H_GET_16 (abfd, ext->e_scnum);
+  in->n_scnum = pe_decode_sym_section_number (abfd, ext->e_scnum);
 
   if (sizeof (ext->e_type) == 2)
     in->n_type = H_GET_16 (abfd, ext->e_type);
@@ -262,7 +305,7 @@  _bfd_XXi_swap_sym_out (bfd * abfd, void * inp, void * extp)
     }
 
   H_PUT_32 (abfd, in->n_value, ext->e_value);
-  H_PUT_16 (abfd, in->n_scnum, ext->e_scnum);
+  pe_encode_sym_section_number (abfd, in->n_scnum, ext->e_scnum);
 
   if (sizeof (ext->e_type) == 2)
     H_PUT_16 (abfd, in->n_type, ext->e_type);
diff --git a/gas/testsuite/gas/pe/large-obj-normal.d b/gas/testsuite/gas/pe/large-obj-normal.d
new file mode 100644
index 0000000000..e9009e5795
--- /dev/null
+++ b/gas/testsuite/gas/pe/large-obj-normal.d
@@ -0,0 +1,17 @@ 
+#objdump: -h -t
+#name: PE large normal object after raising the normal section limit
+
+# This only becomes meaningful once normal PE/COFF is allowed past the
+# old 32767-section cutoff.  Before that, BFD errors out or auto-promotes
+# to bigobj before the normal 16-bit symbol-section encoding is exercised.
+
+.*: *file format pe-(aarch64-little|i386|x86-64)
+
+Sections:
+#...
+40002 +\.data\$a39999  .*
+                  CONTENTS, ALLOC, LOAD, DATA
+
+SYMBOL TABLE:
+#...
+.*\(sec 40003\).*\(scl +2\).*\) 0x[0-9a-f]+ a39999$
diff --git a/gas/testsuite/gas/pe/large-obj-normal.s b/gas/testsuite/gas/pe/large-obj-normal.s
new file mode 100644
index 0000000000..254733023d
--- /dev/null
+++ b/gas/testsuite/gas/pe/large-obj-normal.s
@@ -0,0 +1,16 @@ 
+	.file "large-obj-normal.s"
+
+	.irp n,0,1,2,3
+	.irp m,0,1,2,3,4,5,6,7,8,9
+	.irp c,0,1,2,3,4,5,6,7,8,9
+	.irp d,0,1,2,3,4,5,6,7,8,9
+	.irp u,0,1,2,3,4,5,6,7,8,9
+	.globl a\n\m\c\d\u
+	.section .data$a\n\m\c\d\u,"w"
+a\n\m\c\d\u :
+	.byte 1
+	.endr
+	.endr
+	.endr
+	.endr
+	.endr
diff --git a/gas/testsuite/gas/pe/pe.exp b/gas/testsuite/gas/pe/pe.exp
index 82f217cf94..600fd2425a 100644
--- a/gas/testsuite/gas/pe/pe.exp
+++ b/gas/testsuite/gas/pe/pe.exp
@@ -71,6 +71,7 @@  if {[istarget "aarch64-*-pe*"] || [istarget "aarch64-*-mingw*"]} {
 if { [istarget "aarch64-*-*"]
      || ([istarget "i*86-*-*"] && ![istarget "*-*-interix*"])
      || [istarget "x86_64-*-*"] } then {
+	run_dump_test "large-obj-normal"
 	run_dump_test "big-obj"
 	run_dump_test "big-obj-auto"
 }
diff --git a/include/coff/pe.h b/include/coff/pe.h
index ece4bf25db..133548ff4b 100644
--- a/include/coff/pe.h
+++ b/include/coff/pe.h
@@ -388,6 +388,11 @@  struct external_SYMBOL_EX
   char e_numaux[1];
 } ATTRIBUTE_PACKED ;
 
+#define IMAGE_SYM_UNDEFINED      0
+#define IMAGE_SYM_ABSOLUTE       0xffff
+#define IMAGE_SYM_DEBUG          0xfffe
+#define IMAGE_SYM_SECTION_MAX    0xfeff
+
 #define	SYMENT_BIGOBJ	struct external_SYMBOL_EX
 #define	SYMESZ_BIGOBJ	20