PR 33302, Symbols truncated on i386pep target
Frediano Ziglio
freddy77@gmail.com
Fri Aug 22 11:51:55 GMT 2025
On Fri, Aug 22, 2025 at 12:33 PM Alan Modra <amodra@gmail.com> wrote:
>
> This tidies a few things in the COFF/PE code I noticed while
> correcting this bug, which was caused by commit 012d44268695
> effectively making peXXigen.c _bfd_XXi_swap_aux_out always use the
> COFF E_FILNMLEN of 14. The problem was that the auxent x_fname field
> was defined in include/coff/external.h using a length of 14. Later,
> E_FILNMLEN is redefined to 18 in coff/pe.h. This no doubt falsely
> tripped memory checking tools. (AUXESZ is 18, so no actual buffer
> overrun.)
>
> There are multiple ways to fix this problem. One is to define x_fname
> as an 18 char field, and use E_FILNMLEN when accessing. I chose to
> set E_FILNMLEN to the correct value in include/coff/external.h rather
> than overriding it later so that the field is the correct size. I did
> the same for FILNMLEN. This requires COFF_WITH_PE to be defined
> before including coff/external.h or coff/internal.h, if compiling for
> PE COFF. Much of the patch is about doing just that. I'll note the
> the gdb patch isn't strictly needed (no change in compiler output
> except assert line numbers) but is there for consistency.
>
Maybe just style, won't it be better to use 0/1 as COFF_WITH_PE and
give an error if the macro is not defined? This will make sure the
unit explicitly get what it wants instead of getting a wrong default
of 14.
> Some of the coff-<arch>.c files have PE specific code, unconditionally
> compiled. They now get an error if they are compiled stand-alone
> rather than being included from pe-<arch>.c or pei-<arch>.c.
>
> include/
> * coff/external.h (E_FILNMLEN): Define value for PE if
> COFF_WITH_PE.
> (E_FILNMLEN, E_DIMNUM): Move before auxent.
> * coff/internal.h (FILNMLEN): Define value for PE if
> COFF_WITH_PE.
> (FILNMLEN, DIMNUM): Move before auxent.
> * coff/pe.h (FILNMLEN, E_FILNMLEN): Don't define.
> bfd/
> * coff-aarch64.c (COFF_WITH_peAArch64): Don't define here.
> (COFF_WITH_PE): Error if not defined.
> * coff-ia64.c (COFF_WITH_PE): Error if not defined.
> * coff-loongarch64.c (COFF_WITH_peLoongArch64): Don't define here.
> (COFF_WITH_PE): Error if not defined.
> * coff-mcore.c (COFF_WITH_PE): Error if not defined.
> * coff-riscv64.c (COFF_WITH_PE): Error if not defined.
> * pe-x86_64.c (COFF_WITH_PE): Define earlier.
> * peXXigen.c (COFF_WITH_PE): Define earlier. Include
> coff/internal.h later.
> * pei-aarch64.c: Remove duplicate includes.
> * pei-loongarch64.c: Likewise.
> * pei-riscv64.c: Likewise.
> * pei-x86_64.c: Likewise.
> binutils/
> * dlltool.c (COFF_WITH_PE): Define.
> * objcopy.c (COFF_WITH_PE): Define.
> * od-pe.c (COFF_WITH_PE): Define.
> gas/
> * config/obj-coff.h (COFF_WITH_PE): Don't define.
> * config/te-pe.h (COFF_WITH_PE): Define.
> * config/te-pep.h (COFF_WITH_PE): Define.
> * testsuite/gas/pe/long_file_symbol.d,
> * testsuite/gas/pe/long_file_symbol.s: New test.
> * testsuite/gas/pe/pe.exp: Run it.
> gdb/
> * amd64-windows-tdep.c (COFF_WITH_PE): Define.
> ld/
> * emultempl/pe.em (COFF_WITH_PE): Define.
> * pdb.c (COFF_WITH_PE): Define.
>
Another style, no signed-off-by or reported-by ?
> diff --git a/bfd/coff-aarch64.c b/bfd/coff-aarch64.c
> index af4a44dede0..48b8bfaf496 100644
> --- a/bfd/coff-aarch64.c
> +++ b/bfd/coff-aarch64.c
> @@ -18,9 +18,8 @@
> Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
> MA 02110-1301, USA. */
>
> -
> -#ifndef COFF_WITH_peAArch64
> -#define COFF_WITH_peAArch64
> +#ifndef COFF_WITH_PE
> +#error non-PE COFF unsupported
> #endif
>
> #include "sysdep.h"
> diff --git a/bfd/coff-ia64.c b/bfd/coff-ia64.c
> index 3a947ef17a3..b2873ae8684 100644
> --- a/bfd/coff-ia64.c
> +++ b/bfd/coff-ia64.c
> @@ -19,6 +19,10 @@
> Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
> MA 02110-1301, USA. */
>
> +#ifndef COFF_WITH_PE
> +#error non-PE COFF unsupported
> +#endif
> +
> #include "sysdep.h"
> #include "bfd.h"
> #include "libbfd.h"
> diff --git a/bfd/coff-loongarch64.c b/bfd/coff-loongarch64.c
> index c1fe70ba593..a85210864ae 100644
> --- a/bfd/coff-loongarch64.c
> +++ b/bfd/coff-loongarch64.c
> @@ -18,9 +18,8 @@
> Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
> MA 02110-1301, USA. */
>
> -
> -#ifndef COFF_WITH_peLoongArch64
> -#define COFF_WITH_peLoongArch64
> +#ifndef COFF_WITH_PE
> +#error non-PE COFF unsupported
> #endif
>
> #include "sysdep.h"
> diff --git a/bfd/coff-mcore.c b/bfd/coff-mcore.c
> index e15c4b4a6e2..fab10a7ae08 100644
> --- a/bfd/coff-mcore.c
> +++ b/bfd/coff-mcore.c
> @@ -18,6 +18,10 @@
> Foundation, 51 Franklin Street - Fifth Floor,
> Boston, MA 02110-1301, USA. */
>
> +#ifndef COFF_WITH_PE
> +#error non-PE COFF unsupported
> +#endif
> +
> #include "sysdep.h"
> #include "bfd.h"
> #include "libbfd.h"
> diff --git a/bfd/coff-riscv64.c b/bfd/coff-riscv64.c
> index 870eef35305..2356a6bb94d 100644
> --- a/bfd/coff-riscv64.c
> +++ b/bfd/coff-riscv64.c
> @@ -18,6 +18,10 @@
> Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
> MA 02110-1301, USA. */
>
> +#ifndef COFF_WITH_PE
> +#error non-PE COFF unsupported
> +#endif
> +
> #include "sysdep.h"
> #include "bfd.h"
> #include "libbfd.h"
> diff --git a/bfd/pe-x86_64.c b/bfd/pe-x86_64.c
> index 9151aac5792..695cc0f0b2e 100644
> --- a/bfd/pe-x86_64.c
> +++ b/bfd/pe-x86_64.c
> @@ -24,6 +24,7 @@
> #include "bfd.h"
> #include "libbfd.h"
> #include "libiberty.h"
> +#define COFF_WITH_PE
> #include "coff/x86_64.h"
> #include "coff/internal.h"
> #include "coff/pe.h"
> @@ -33,7 +34,6 @@
> #define TARGET_NAME "pe-x86-64"
> #define TARGET_SYM_BIG x86_64_pe_big_vec
> #define TARGET_NAME_BIG "pe-bigobj-x86-64"
> -#define COFF_WITH_PE
> #define COFF_WITH_pex64
> #define COFF_WITH_PE_BIGOBJ
> #define PCRELOFFSET true
> diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
> index c337fc5b282..70e0d26de45 100644
> --- a/bfd/peXXigen.c
> +++ b/bfd/peXXigen.c
> @@ -68,12 +68,13 @@
> #include "sysdep.h"
> #include "bfd.h"
> #include "libbfd.h"
> -#include "coff/internal.h"
> #include "bfdver.h"
> #include "libiberty.h"
> #include <wchar.h>
> #include <wctype.h>
>
> +#define COFF_WITH_PE
> +
> /* NOTE: it's strange to be including an architecture specific header
> in what's supposed to be general (to PE/PEI) code. However, that's
> where the definitions are, and they don't vary per architecture
> @@ -94,6 +95,7 @@
> # include "coff/i386.h"
> #endif
>
> +#include "coff/internal.h"
> #include "coff/pe.h"
> #include "libcoff.h"
> #include "libpei.h"
> diff --git a/bfd/pei-aarch64.c b/bfd/pei-aarch64.c
> index 00f38e0f9bb..835b547ebc6 100644
> --- a/bfd/pei-aarch64.c
> +++ b/bfd/pei-aarch64.c
> @@ -20,6 +20,7 @@
>
> #include "sysdep.h"
> #include "bfd.h"
> +#include "libbfd.h"
>
> #define TARGET_SYM aarch64_pei_le_vec
> #define TARGET_NAME "pei-aarch64-little"
> @@ -58,9 +59,6 @@
> { COFF_SECTION_NAME_PARTIAL_MATCH (".gnu.linkonce.wi."), \
> COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 0 }
>
> -#include "sysdep.h"
> -#include "bfd.h"
> -#include "libbfd.h"
> #include "coff/aarch64.h"
> #include "coff/internal.h"
> #include "coff/pe.h"
> diff --git a/bfd/pei-loongarch64.c b/bfd/pei-loongarch64.c
> index f22498cab21..0b00cb2d557 100644
> --- a/bfd/pei-loongarch64.c
> +++ b/bfd/pei-loongarch64.c
> @@ -20,6 +20,7 @@
>
> #include "sysdep.h"
> #include "bfd.h"
> +#include "libbfd.h"
>
> #define TARGET_SYM loongarch64_pei_vec
> #define TARGET_NAME "pei-loongarch64"
> @@ -58,9 +59,6 @@
> { COFF_SECTION_NAME_PARTIAL_MATCH (".gnu.linkonce.wi."), \
> COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 0 }
>
> -#include "sysdep.h"
> -#include "bfd.h"
> -#include "libbfd.h"
> #include "coff/loongarch64.h"
> #include "coff/internal.h"
> #include "coff/pe.h"
> diff --git a/bfd/pei-riscv64.c b/bfd/pei-riscv64.c
> index c4ae7bfb89a..d8d4b5de145 100644
> --- a/bfd/pei-riscv64.c
> +++ b/bfd/pei-riscv64.c
> @@ -20,6 +20,7 @@
>
> #include "sysdep.h"
> #include "bfd.h"
> +#include "libbfd.h"
>
> #define TARGET_SYM riscv64_pei_vec
> #define TARGET_NAME "pei-riscv64-little"
> @@ -58,9 +59,6 @@
> { COFF_SECTION_NAME_PARTIAL_MATCH (".gnu.linkonce.wi."), \
> COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 0 }
>
> -#include "sysdep.h"
> -#include "bfd.h"
> -#include "libbfd.h"
> #include "coff/riscv64.h"
> #include "coff/internal.h"
> #include "coff/pe.h"
> diff --git a/bfd/pei-x86_64.c b/bfd/pei-x86_64.c
> index a5c2fd85f06..045fedb79a5 100644
> --- a/bfd/pei-x86_64.c
> +++ b/bfd/pei-x86_64.c
> @@ -22,6 +22,7 @@
>
> #include "sysdep.h"
> #include "bfd.h"
> +#include "libbfd.h"
>
> #define TARGET_SYM x86_64_pei_vec
> #define TARGET_NAME "pei-x86-64"
> @@ -60,9 +61,6 @@
> { COFF_SECTION_NAME_PARTIAL_MATCH (".gnu.linkonce.wi."), \
> COFF_ALIGNMENT_FIELD_EMPTY, COFF_ALIGNMENT_FIELD_EMPTY, 0 }
>
> -#include "sysdep.h"
> -#include "bfd.h"
> -#include "libbfd.h"
> #include "coff/x86_64.h"
> #include "coff/internal.h"
> #include "coff/pe.h"
> diff --git a/binutils/dlltool.c b/binutils/dlltool.c
> index 99c651f7f9b..afe1228d97f 100644
> --- a/binutils/dlltool.c
> +++ b/binutils/dlltool.c
> @@ -252,6 +252,8 @@
> #include <time.h>
> #include <assert.h>
>
> +#define COFF_WITH_PE
> +
> #ifdef DLLTOOL_ARM
> #include "coff/arm.h"
> #include "coff/internal.h"
> diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> index 51d87d6d359..497bf441187 100644
> --- a/binutils/objcopy.c
> +++ b/binutils/objcopy.c
> @@ -27,6 +27,7 @@
> #include "filenames.h"
> #include "fnmatch.h"
> #include "elf-bfd.h"
> +#define COFF_WITH_PE
> #include "coff/internal.h"
> #include "libcoff.h"
> #include "safe-ctype.h"
> diff --git a/binutils/od-pe.c b/binutils/od-pe.c
> index c3341fb15ca..c3d504a39ee 100644
> --- a/binutils/od-pe.c
> +++ b/binutils/od-pe.c
> @@ -27,6 +27,7 @@
> #include "objdump.h"
> #include "bucomm.h"
> #include "bfdlink.h"
> +#define COFF_WITH_PE
> #include "coff/internal.h"
> #define L_LNNO_SIZE 4 /* FIXME: which value should we use ? */
> #include "coff/external.h"
> diff --git a/gas/config/obj-coff.h b/gas/config/obj-coff.h
> index 51dfd56e447..50b5a06b66e 100644
> --- a/gas/config/obj-coff.h
> +++ b/gas/config/obj-coff.h
> @@ -71,10 +71,6 @@
>
> #ifdef TC_SH
>
> -#ifdef TE_PE
> -#define COFF_WITH_PE
> -#endif
> -
> #include "coff/sh.h"
>
> #ifdef TE_PE
> diff --git a/gas/config/te-pe.h b/gas/config/te-pe.h
> index d77dc05e3b1..ed5c959c490 100644
> --- a/gas/config/te-pe.h
> +++ b/gas/config/te-pe.h
> @@ -18,6 +18,7 @@
> 02110-1301, USA. */
>
> #define TE_PE
> +#define COFF_WITH_PE
> #define LEX_AT (LEX_BEGIN_NAME | LEX_NAME) /* Can have @'s inside labels. */
>
> /* The PE format supports long section names. */
> diff --git a/gas/config/te-pep.h b/gas/config/te-pep.h
> index ed5c0ac7f54..658207506eb 100644
> --- a/gas/config/te-pep.h
> +++ b/gas/config/te-pep.h
> @@ -18,6 +18,7 @@
> 02110-1301, USA. */
>
> #define TE_PEP
> +#define COFF_WITH_PE
> #define COFF_WITH_pex64
>
> #define TE_PE
> diff --git a/gas/testsuite/gas/pe/long_file_symbol.d b/gas/testsuite/gas/pe/long_file_symbol.d
> new file mode 100644
> index 00000000000..43510b98df7
> --- /dev/null
> +++ b/gas/testsuite/gas/pe/long_file_symbol.d
> @@ -0,0 +1,5 @@
> +#nm: -a
> +
> +#...
> +.* long_file_symbol.s
> +#pass
> diff --git a/gas/testsuite/gas/pe/long_file_symbol.s b/gas/testsuite/gas/pe/long_file_symbol.s
> new file mode 100644
> index 00000000000..066499fefba
> --- /dev/null
> +++ b/gas/testsuite/gas/pe/long_file_symbol.s
> @@ -0,0 +1 @@
> + .file "long_file_symbol.s"
Well, not any long name, any name between 15 and 18 bytes. In this
case it is exactly 18.
I would add at least a comment here about it.
Thanks for the test.
> diff --git a/gas/testsuite/gas/pe/pe.exp b/gas/testsuite/gas/pe/pe.exp
> index 00a6d14fbfa..8682789d209 100644
> --- a/gas/testsuite/gas/pe/pe.exp
> +++ b/gas/testsuite/gas/pe/pe.exp
> @@ -31,7 +31,7 @@ run_dump_test "aligncomm-a"
> run_dump_test "aligncomm-b"
> run_dump_test "aligncomm-c"
> run_dump_test "aligncomm-d"
> -
> +run_dump_test "long_file_symbol"
> run_dump_test "section-align-1"
> run_dump_test "section-align-3"
> run_dump_test "section-exclude"
> diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
> index c7977d29c3f..fbec7a78002 100644
> --- a/gdb/amd64-windows-tdep.c
> +++ b/gdb/amd64-windows-tdep.c
> @@ -27,6 +27,7 @@
> #include "frame.h"
> #include "objfiles.h"
> #include "frame-unwind.h"
> +#define COFF_WITH_PE
> #include "coff/internal.h"
> #include "coff/i386.h"
> #include "coff/pe.h"
> diff --git a/include/coff/external.h b/include/coff/external.h
> index 772b86601af..e8c6003648c 100644
> --- a/include/coff/external.h
> +++ b/include/coff/external.h
> @@ -142,10 +142,6 @@ struct external_lineno
> /********************** SYMBOLS **********************/
>
> #define E_SYMNMLEN 8 /* # characters in a symbol name */
> -#ifndef E_FILNMLEN
> -#define E_FILNMLEN 14
> -#endif
> -#define E_DIMNUM 4 /* # array dimensions in auxiliary entry */
>
> struct external_syment
> {
> @@ -190,6 +186,13 @@ struct external_syment
>
> #ifndef DO_NOT_DEFINE_AUXENT
>
> +#ifdef COFF_WITH_PE
> +#define E_FILNMLEN 18 /* # characters in a file name */
> +#else
> +#define E_FILNMLEN 14 /* # characters in a file name */
> +#endif
> +#define E_DIMNUM 4 /* # array dimensions in auxiliary entry */
> +
> union external_auxent
> {
> struct
> diff --git a/include/coff/internal.h b/include/coff/internal.h
> index 9532b0beb9c..1d1388ba6c4 100644
> --- a/include/coff/internal.h
> +++ b/include/coff/internal.h
> @@ -459,8 +459,6 @@ struct internal_lineno
> /********************** SYMBOLS **********************/
>
> #define SYMNMLEN 8 /* # characters in a symbol name */
> -#define FILNMLEN 14 /* # characters in a file name */
> -#define DIMNUM 4 /* # array dimensions in auxiliary entry */
>
> struct internal_syment
> {
> @@ -543,6 +541,13 @@ struct internal_syment
> #define SYM_V_EXPORTED 0x4000
> #define SYM_V_MASK 0xF000
>
> +#ifdef COFF_WITH_PE
> +#define FILNMLEN 18 /* # characters in a file name */
> +#else
> +#define FILNMLEN 14 /* # characters in a file name */
> +#endif
> +#define DIMNUM 4 /* # array dimensions in auxiliary entry */
> +
> union internal_auxent
> {
> struct
> diff --git a/include/coff/pe.h b/include/coff/pe.h
> index 59ac6366bf8..6bb8bc9c793 100644
> --- a/include/coff/pe.h
> +++ b/include/coff/pe.h
> @@ -179,11 +179,6 @@
> #define IMAGE_SUBSYSTEM_SAL_RUNTIME_DRIVER 13
> #define IMAGE_SUBSYSTEM_XBOX 14
>
> -/* NT allows long filenames, we want to accommodate this.
> - This may break some of the bfd functions. */
> -#undef FILNMLEN
> -#define FILNMLEN 18 /* # characters in a file name. */
> -
> struct external_PEI_IMAGE_hdr
> {
> char nt_signature[4]; /* Required NT signature, 0x4550. */
> @@ -325,9 +320,6 @@ typedef struct
> #define PEPAOUTSZ 240
> #endif
>
> -#undef E_FILNMLEN
> -#define E_FILNMLEN 18 /* # characters in a file name. */
> -
> /* Import Tyoes fot ILF format object files.. */
> #define IMPORT_CODE 0
> #define IMPORT_DATA 1
> diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
> index b522687c1d5..eeabcebb494 100644
> --- a/ld/emultempl/pe.em
> +++ b/ld/emultempl/pe.em
> @@ -65,6 +65,7 @@ fragment <<EOF
> #include "ldmisc.h"
> #include "ldctor.h"
> #include "ldbuildid.h"
> +#define COFF_WITH_PE
> #include "coff/internal.h"
> EOF
>
> diff --git a/ld/pdb.c b/ld/pdb.c
> index d0d51c7a79a..e4bed3ab9b1 100644
> --- a/ld/pdb.c
> +++ b/ld/pdb.c
> @@ -25,6 +25,7 @@
> #include "ldmisc.h"
> #include "libbfd.h"
> #include "libiberty.h"
> +#define COFF_WITH_PE
> #include "coff/i386.h"
> #include "coff/external.h"
> #include "coff/internal.h"
>
> --
> Alan Modra
Frediano
More information about the Binutils
mailing list