[PATCH v2 12/13] bfd/PE: respect SEC_ALLOC when deciding whether to force IMAGE_SCN_MEM_DISCARDABLE

Jan Beulich jbeulich@suse.com
Fri Aug 29 12:06:20 GMT 2025


While in the common case sections like .reloc don't need to be accessed
by a binary (that's the job of the loader), there are rare cases where
the situation is different. Unconditionally forcing the discardable flag
is therefore unhelpful. Avoid doing so when SEC_ALLOC is set, which
would in particular be the case when .reloc isn't (solely) linker-
generated.

While adjusting the comment, drop the wrong (stale?) following sentence:
.rsrc, as per its table entry, doesn't need to be writable. And
commenting on it wanting to be readable would then need to be repeated
for all other sections as well.
---
Question is whether we should respect (retain) the discardable flag when it is
specified already in all incoming object's sections.

In the ld testcase I'm not quite sure about really expecting .idata: In
an executable that doesn't import anything, there should be no .idata at
all? Why do we nevertheless produce such a stub?
---
v2: Adjust comment in _bfd_XXi_swap_scnhdr_out().

--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -916,7 +916,7 @@ _bfd_XX_only_swap_filehdr_out (bfd * abf
 
 unsigned int
 _bfd_XXi_swap_scnhdr_out (bfd * abfd, void * in, void * out,
-			  const asection *section ATTRIBUTE_UNUSED)
+			  const asection *section)
 {
   struct internal_scnhdr *scnhdr_int = (struct internal_scnhdr *) in;
   SCNHDR *scnhdr_ext = (SCNHDR *) out;
@@ -985,9 +985,8 @@ _bfd_XXi_swap_scnhdr_out (bfd * abfd, vo
        sections (.idata, .data, .bss, .CRT) must have IMAGE_SCN_MEM_WRITE set
        (this is especially important when dealing with the .idata section since
        the addresses for routines from .dlls must be overwritten).  If .reloc
-       section data is ever generated, we must add IMAGE_SCN_MEM_DISCARDABLE
-       (0x02000000).  Also, the resource data should also be read and
-       writable.  */
+       section data is ever generated, we generally need to add
+       IMAGE_SCN_MEM_DISCARDABLE (0x02000000).  */
 
     /* FIXME: Alignment is also encoded in this field, at least on
        ARM-WINCE.  Although - how do we get the original alignment field
@@ -1033,10 +1032,16 @@ _bfd_XXi_swap_scnhdr_out (bfd * abfd, vo
 	 p++)
       if (memcmp (scnhdr_int->s_name, p->section_name, SCNNMLEN) == 0)
 	{
+	  unsigned long must_have = p->must_have;
+
 	  if (memcmp (scnhdr_int->s_name, ".text", sizeof ".text")
 	      || (bfd_get_file_flags (abfd) & WP_TEXT))
 	    scnhdr_int->s_flags &= ~IMAGE_SCN_MEM_WRITE;
-	  scnhdr_int->s_flags |= p->must_have;
+	  /* Avoid forcing in the discardable flag if the section itself is
+	     allocated.  */
+	  if (section->flags & SEC_ALLOC)
+	    must_have &= ~IMAGE_SCN_MEM_DISCARDABLE;
+	  scnhdr_int->s_flags |= must_have;
 	  break;
 	}
 
--- a/gas/testsuite/gas/pe/pe.exp
+++ b/gas/testsuite/gas/pe/pe.exp
@@ -38,6 +38,8 @@ run_dump_test "section-exclude"
 
 run_dump_test "set"
 
+run_dump_test "reloc"
+
 if { [istarget "aarch64-*-*"] || [istarget "arm*-*-*"] || [istarget "i*86-*-*"]
     || [istarget "x86_64-*-*"] } then {
   run_dump_test "cv_comp"
--- /dev/null
+++ b/gas/testsuite/gas/pe/reloc.d
@@ -0,0 +1,13 @@
+#as: --no-pad-sections
+#objdump: -wPsections
+#name: .reloc section attributes
+
+.*: .*
+
+Section headers.*:
+ # Name    *paddr *vaddr *size  .* Flags
+ 1 \.text  *0*0000 0*0000 0*0(4|10) .* Align: (4|16) EXECUTE,READ,CODE
+ 2 \.data  *0*0000 0*0000 0*0(8|10) .* Align: (4|16) READ,WRITE,INITIALIZED DATA
+ 3 \.bss   .*
+ 4 \.rdata *0*0000 0*0000 0*0(8|10) .* Align: (4|16) READ,INITIALIZED DATA
+ 5 \.reloc *0*0000 0*0000 0*00c .* Align: 4 READ,INITIALIZED DATA
--- /dev/null
+++ b/gas/testsuite/gas/pe/reloc.s
@@ -0,0 +1,19 @@
+	.text
+	.global _start
+_start:
+	.nop
+
+	.section .rdata, "r"
+	.dc.a	_start
+	.dc.a	.
+
+	.data
+.L1:	.rva	_start
+.L2:	.rva	.
+
+	.section .reloc, "r"
+	.p2align 2
+	.rva	.data
+	.long	2 * 4 + 2 * 2
+	.short	.L1 - .data + (3 << 12)
+	.short	.L2 - .data + (3 << 12)
--- a/ld/testsuite/ld-pe/pe.exp
+++ b/ld/testsuite/ld-pe/pe.exp
@@ -137,6 +137,9 @@ run_dump_test "orphan"
 run_dump_test "orphan_nu"
 
 run_dump_test "reloc"
+run_dump_test "reloc2a"
+run_dump_test "reloc2b"
+run_dump_test "reloc2c"
 
 run_dump_test "weakdef-1"
 
--- /dev/null
+++ b/ld/testsuite/ld-pe/reloc2.s
@@ -0,0 +1,19 @@
+	.text
+	.global _start
+_start:
+	.nop
+
+	.section .rdata, "r"
+	.dc.a	_start
+	.dc.a	.
+
+	.data
+.L1:	.rva	_start
+.L2:	.rva	.
+
+	.section .reloc, "r"
+	.p2align 2
+	.rva	.data
+	.long	2 * 4 + 2 * 2
+	.short	.L1 - .data + (3 << 12)
+	.short	.L2 - .data + (3 << 12)
--- /dev/null
+++ b/ld/testsuite/ld-pe/reloc2a.d
@@ -0,0 +1,35 @@
+#source: reloc2.s
+#as: --no-pad-sections
+#name: PE .reloc section attributes (builtin+custom)
+#ld: --enable-reloc-section --section-align=0x1000 -e _start
+#objdump: -hpwPsections
+
+.*:     file format .*
+
+#...
+PE File Base Relocations.*
+Virtual Address: .* Number of fixups 2
+[ 	]*reloc    0 offset    0 .* HIGHLOW
+[ 	]*reloc    1 offset    [48] .* HIGHLOW
+
+Virtual Address: .* Number of fixups 2
+[ 	]*reloc    0 offset    0 .* (HIGHLOW|DIR64)
+[ 	]*reloc    1 offset    [48] .* (HIGHLOW|DIR64)
+
+#?private flags = .*
+
+Section headers.*:
+ # Name    *paddr *vaddr *size  .* Flags
+ 1 \.text  *0000.* 0.*000 0*200 .* EXECUTE,READ,CODE
+ 2 \.data  *0000.* 0.*000 0*200 .* READ,WRITE,INITIALIZED DATA
+ 3 \.rdata *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+ 4 \.idata *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+ 5 \.reloc *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+
+Sections:
+Idx Name    *Size     *VMA    *LMA  *File off  .* Flags
+  0 \.text  *0*(1?4|8|10) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, CODE
+  1 \.data  *0*(8|10) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, DATA
+  2 \.rdata *0*0000.. .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
+  3 \.idata *0*1(4|8) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
+  4 \.reloc *0*000018 .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
--- /dev/null
+++ b/ld/testsuite/ld-pe/reloc2b.d
@@ -0,0 +1,32 @@
+#source: reloc2.s RUN_OBJCOPY
+#as: --no-pad-sections
+#name: PE .reloc section attributes (builtin only)
+#objcopy_objects: --rename-section .reloc=.rdata$1
+#ld: --enable-reloc-section --section-align=0x1000 -e _start
+#objdump: -hpwPsections
+
+.*:     file format .*
+
+#...
+PE File Base Relocations.*
+Virtual Address: .* Number of fixups 2
+[ 	]*reloc    0 offset    0 .* (HIGHLOW|DIR64)
+[ 	]*reloc    1 offset    [48] .* (HIGHLOW|DIR64)
+
+#?private flags = .*
+
+Section headers.*:
+ # Name    *paddr *vaddr *size  .* Flags
+ 1 \.text  *0000.* 0.*000 0*200 .* EXECUTE,READ,CODE
+ 2 \.data  *0000.* 0.*000 0*200 .* READ,WRITE,INITIALIZED DATA
+ 3 \.rdata *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+ 4 \.idata *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+ 5 \.reloc *0000.* 0.*000 0*200 .* DISCARDABLE,READ,INITIALIZED DATA
+
+Sections:
+Idx Name    *Size     *VMA    *LMA  *File off  .* Flags
+  0 \.text  *0*(1?4|8|10) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, CODE
+  1 \.data  *0*(8|10) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, DATA
+  2 \.rdata *0*0000.. .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
+  3 \.idata *0*1(4|8) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
+  4 \.reloc *0*00000c .*000  .*000  0*0000... .* CONTENTS, LOAD, READONLY, DATA
--- /dev/null
+++ b/ld/testsuite/ld-pe/reloc2c.d
@@ -0,0 +1,31 @@
+#source: reloc2.s
+#as: --no-pad-sections
+#name: PE .reloc section attributes (custom only)
+#ld: --disable-reloc-section --section-align=0x1000 -e _start
+#objdump: -hpwPsections
+
+.*:     file format .*
+
+#...
+PE File Base Relocations.*
+Virtual Address: .* Number of fixups 2
+[ 	]*reloc    0 offset    0 .* HIGHLOW
+[ 	]*reloc    1 offset    [48] .* HIGHLOW
+
+#?private flags = .*
+
+Section headers.*:
+ # Name    *paddr *vaddr *size  .* Flags
+ 1 \.text  *0000.* 0.*000 0*200 .* EXECUTE,READ,CODE
+ 2 \.data  *0000.* 0.*000 0*200 .* READ,WRITE,INITIALIZED DATA
+ 3 \.rdata *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+ 4 \.idata *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+ 5 \.reloc *0000.* 0.*000 0*200 .* READ,INITIALIZED DATA
+
+Sections:
+Idx Name    *Size     *VMA    *LMA  *File off  .* Flags
+  0 \.text  *0*(1?4|8|10) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, CODE
+  1 \.data  *0*(8|10) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, DATA
+  2 \.rdata *0*0000.. .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
+  3 \.idata *0*1(4|8) .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA
+  4 \.reloc *0*00000c .*000  .*000  0*0000... .* CONTENTS, ALLOC, LOAD, READONLY, DATA



More information about the Binutils mailing list