PR30592 objcopy: allow --set-section-flags to add or remove SHF_X86_64_LARGE

Message ID 20230628010634.1147958-1-maskray@google.com
State Accepted
Headers
Series PR30592 objcopy: allow --set-section-flags to add or remove SHF_X86_64_LARGE |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Fangrui Song June 28, 2023, 1:06 a.m. UTC
  For example, objcopy --set-section-flags .data=alloc,large will add
SHF_X86_64_LARGE to the .data section. Omitting "large" will drop the
SHF_X86_64_LARGE flag.

The bfd_section flag is named generically, SEC_ELF_LARGE, in case other
processors want to follow SHF_X86_64_LARGE.

bfd/
    * bfd-in2.h: Define SEC_ELF_LARGE.
    * elf.c (_bfd_elf_make_section_from_shdr): Check SHF_X86_64_LARGE.
    (elf_fake_sections): Check SEC_ELF_LARGE.
    (_bfd_elf_init_private_section_data): Drop SHF_X86_64_LARGE for
    x86-64.

binutils/
    * doc/binutils.texi: Mention "large".
    * objcopy.c (parse_flags): Parse "large".
    * testsuite/binutils-all/x86-64/large-sections.d: New.
    * testsuite/binutils-all/x86-64/large-sections.s: New.
    * testsuite/binutils-all/x86-64/large-sections-2.d: New.
    * testsuite/binutils-all/x86-64/large-sections-2.s: New.

include/
    * elf/common.h: Define SHF_X86_64_LARGE to be used by elf.c.
---
 bfd/bfd-in2.h                                     |  3 +++
 bfd/elf.c                                         |  9 +++++++++
 binutils/doc/binutils.texi                        | 15 ++++++++-------
 binutils/objcopy.c                                |  3 ++-
 .../binutils-all/x86-64/large-sections-2.d        | 15 +++++++++++++++
 .../binutils-all/x86-64/large-sections-2.s        |  4 ++++
 .../binutils-all/x86-64/large-sections.d          | 14 ++++++++++++++
 .../binutils-all/x86-64/large-sections.s          |  8 ++++++++
 include/elf/common.h                              |  2 ++
 9 files changed, 65 insertions(+), 8 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/x86-64/large-sections-2.d
 create mode 100644 binutils/testsuite/binutils-all/x86-64/large-sections-2.s
 create mode 100644 binutils/testsuite/binutils-all/x86-64/large-sections.d
 create mode 100644 binutils/testsuite/binutils-all/x86-64/large-sections.s
  

Comments

Andreas Schwab June 28, 2023, 7:29 a.m. UTC | #1
On Jun 27 2023, Fangrui Song via Binutils wrote:

> diff --git a/bfd/elf.c b/bfd/elf.c
> index 8f6d7d1adba..d6e21b11419 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -1034,6 +1034,10 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
>    if ((hdr->sh_flags & SHF_EXCLUDE) != 0)
>      flags |= SEC_EXCLUDE;
>  
> +  if (get_elf_backend_data (abfd)->elf_machine_code == EM_X86_64)
> +    if ((hdr->sh_flags & SHF_X86_64_LARGE) != 0)
> +      flags |= SEC_ELF_LARGE;
> +
>    switch (elf_elfheader (abfd)->e_ident[EI_OSABI])
>      {
>        /* FIXME: We should not recognize SHF_GNU_MBIND for ELFOSABI_NONE,
> @@ -3351,6 +3355,8 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
>      }
>    if ((asect->flags & (SEC_GROUP | SEC_EXCLUDE)) == SEC_EXCLUDE)
>      this_hdr->sh_flags |= SHF_EXCLUDE;
> +  if (asect->flags & SEC_ELF_LARGE)
> +    this_hdr->sh_flags |= SHF_X86_64_LARGE;

Doesn't that need to be conditional on EM_X86_64 as well?  I don't see
anything that blocks SEC_ELF_LARGE from being set on incompatible
objects.

> diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
> index 8c14d1121d9..4dd7b83aaa6 100644
> --- a/binutils/doc/binutils.texi
> +++ b/binutils/doc/binutils.texi
> @@ -1740,13 +1740,14 @@ Set the flags for any sections matching @var{sectionpattern}.  The
>  @var{flags} argument is a comma separated string of flag names.  The
>  recognized names are @samp{alloc}, @samp{contents}, @samp{load},
>  @samp{noload}, @samp{readonly}, @samp{code}, @samp{data}, @samp{rom},
> -@samp{exclude}, @samp{share}, and @samp{debug}.  You can set the
> -@samp{contents} flag for a section which does not have contents, but it
> -is not meaningful to clear the @samp{contents} flag of a section which
> -does have contents--just remove the section instead.  Not all flags are
> -meaningful for all object file formats.  In particular the
> -@samp{share} flag is only meaningful for COFF format files and not for
> -ELF format files.
> +@samp{exclude}, @samp{share}, @samp{debug}, and @samp{large}.
> +You can set the @samp{contents} flag for a section which does not have
> +contents, but it is not meaningful to clear the @samp{contents} flag of a
> +section which does have contents--just remove the section instead.  Not all
> +flags are meaningful for all object file formats.  In particular the
> +@samp{share} flag is only meaningful for COFF format files and not for ELF
> +format files.  @samp{large} is an ELF x86-64 specific flag that corresponds to
> +SHF_X86_64_LARGE.

The last sentence needs to be rearranged so that it doesn't start with a
lower case word.
  
Fangrui Song June 28, 2023, 11:17 p.m. UTC | #2
On Wed, Jun 28, 2023 at 12:29 AM Andreas Schwab <schwab@suse.de> wrote:
>
> On Jun 27 2023, Fangrui Song via Binutils wrote:
>
> > diff --git a/bfd/elf.c b/bfd/elf.c
> > index 8f6d7d1adba..d6e21b11419 100644
> > --- a/bfd/elf.c
> > +++ b/bfd/elf.c
> > @@ -1034,6 +1034,10 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
> >    if ((hdr->sh_flags & SHF_EXCLUDE) != 0)
> >      flags |= SEC_EXCLUDE;
> >
> > +  if (get_elf_backend_data (abfd)->elf_machine_code == EM_X86_64)
> > +    if ((hdr->sh_flags & SHF_X86_64_LARGE) != 0)
> > +      flags |= SEC_ELF_LARGE;
> > +
> >    switch (elf_elfheader (abfd)->e_ident[EI_OSABI])
> >      {
> >        /* FIXME: We should not recognize SHF_GNU_MBIND for ELFOSABI_NONE,
> > @@ -3351,6 +3355,8 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
> >      }
> >    if ((asect->flags & (SEC_GROUP | SEC_EXCLUDE)) == SEC_EXCLUDE)
> >      this_hdr->sh_flags |= SHF_EXCLUDE;
> > +  if (asect->flags & SEC_ELF_LARGE)
> > +    this_hdr->sh_flags |= SHF_X86_64_LARGE;
>
> Doesn't that need to be conditional on EM_X86_64 as well?  I don't see
> anything that blocks SEC_ELF_LARGE from being set on incompatible
> objects.
>
> > diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
> > index 8c14d1121d9..4dd7b83aaa6 100644
> > --- a/binutils/doc/binutils.texi
> > +++ b/binutils/doc/binutils.texi
> > @@ -1740,13 +1740,14 @@ Set the flags for any sections matching @var{sectionpattern}.  The
> >  @var{flags} argument is a comma separated string of flag names.  The
> >  recognized names are @samp{alloc}, @samp{contents}, @samp{load},
> >  @samp{noload}, @samp{readonly}, @samp{code}, @samp{data}, @samp{rom},
> > -@samp{exclude}, @samp{share}, and @samp{debug}.  You can set the
> > -@samp{contents} flag for a section which does not have contents, but it
> > -is not meaningful to clear the @samp{contents} flag of a section which
> > -does have contents--just remove the section instead.  Not all flags are
> > -meaningful for all object file formats.  In particular the
> > -@samp{share} flag is only meaningful for COFF format files and not for
> > -ELF format files.
> > +@samp{exclude}, @samp{share}, @samp{debug}, and @samp{large}.
> > +You can set the @samp{contents} flag for a section which does not have
> > +contents, but it is not meaningful to clear the @samp{contents} flag of a
> > +section which does have contents--just remove the section instead.  Not all
> > +flags are meaningful for all object file formats.  In particular the
> > +@samp{share} flag is only meaningful for COFF format files and not for ELF
> > +format files.  @samp{large} is an ELF x86-64 specific flag that corresponds to
> > +SHF_X86_64_LARGE.
>
> The last sentence needs to be rearranged so that it doesn't start with a
> lower case word.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Thanks for the comments. Uploaded v2
https://sourceware.org/pipermail/binutils/2023-June/128078.html
  

Patch

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 7399fb0fa60..2507716899c 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -633,6 +633,9 @@  typedef struct bfd_section
   /* This section contains vliw code.  This is for Toshiba MeP only.  */
 #define SEC_MEP_VLIW               0x20000000
 
+  /* This section has the SHF_X86_64_LARGE flag.  This is ELF x86-64 only.  */
+#define SEC_ELF_LARGE              0x20000000
+
   /* All symbols, sizes and relocations in this section are octets
      instead of bytes.  Required for DWARF debug sections as DWARF
      information is organized in octets, not bytes.  */
diff --git a/bfd/elf.c b/bfd/elf.c
index 8f6d7d1adba..d6e21b11419 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -1034,6 +1034,10 @@  _bfd_elf_make_section_from_shdr (bfd *abfd,
   if ((hdr->sh_flags & SHF_EXCLUDE) != 0)
     flags |= SEC_EXCLUDE;
 
+  if (get_elf_backend_data (abfd)->elf_machine_code == EM_X86_64)
+    if ((hdr->sh_flags & SHF_X86_64_LARGE) != 0)
+      flags |= SEC_ELF_LARGE;
+
   switch (elf_elfheader (abfd)->e_ident[EI_OSABI])
     {
       /* FIXME: We should not recognize SHF_GNU_MBIND for ELFOSABI_NONE,
@@ -3351,6 +3355,8 @@  elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
     }
   if ((asect->flags & (SEC_GROUP | SEC_EXCLUDE)) == SEC_EXCLUDE)
     this_hdr->sh_flags |= SHF_EXCLUDE;
+  if (asect->flags & SEC_ELF_LARGE)
+    this_hdr->sh_flags |= SHF_X86_64_LARGE;
 
   /* If the section has relocs, set up a section header for the
      SHT_REL[A] section.  If two relocation sections are required for
@@ -7940,6 +7946,9 @@  _bfd_elf_init_private_section_data (bfd *ibfd,
   elf_section_flags (osec) = (elf_section_flags (isec)
 			      & (SHF_MASKOS | SHF_MASKPROC));
 
+  if (get_elf_backend_data (ibfd)->elf_machine_code == EM_X86_64)
+    elf_section_flags (osec) = (elf_section_flags (isec) & ~SHF_X86_64_LARGE);
+
   /* Copy sh_info from input for mbind section.  */
   if ((elf_tdata (ibfd)->has_gnu_osabi & elf_gnu_osabi_mbind) != 0
       && elf_section_flags (isec) & SHF_GNU_MBIND)
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 8c14d1121d9..4dd7b83aaa6 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -1740,13 +1740,14 @@  Set the flags for any sections matching @var{sectionpattern}.  The
 @var{flags} argument is a comma separated string of flag names.  The
 recognized names are @samp{alloc}, @samp{contents}, @samp{load},
 @samp{noload}, @samp{readonly}, @samp{code}, @samp{data}, @samp{rom},
-@samp{exclude}, @samp{share}, and @samp{debug}.  You can set the
-@samp{contents} flag for a section which does not have contents, but it
-is not meaningful to clear the @samp{contents} flag of a section which
-does have contents--just remove the section instead.  Not all flags are
-meaningful for all object file formats.  In particular the
-@samp{share} flag is only meaningful for COFF format files and not for
-ELF format files.
+@samp{exclude}, @samp{share}, @samp{debug}, and @samp{large}.
+You can set the @samp{contents} flag for a section which does not have
+contents, but it is not meaningful to clear the @samp{contents} flag of a
+section which does have contents--just remove the section instead.  Not all
+flags are meaningful for all object file formats.  In particular the
+@samp{share} flag is only meaningful for COFF format files and not for ELF
+format files.  @samp{large} is an ELF x86-64 specific flag that corresponds to
+SHF_X86_64_LARGE.
 
 @item --set-section-alignment @var{sectionpattern}=@var{align}
 Set the alignment for any sections matching @var{sectionpattern}.
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 414007780a8..40496f8f792 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -797,6 +797,7 @@  parse_flags (const char *s)
       PARSE_FLAG ("contents", SEC_HAS_CONTENTS);
       PARSE_FLAG ("merge", SEC_MERGE);
       PARSE_FLAG ("strings", SEC_STRINGS);
+      PARSE_FLAG ("large", SEC_ELF_LARGE);
 #undef PARSE_FLAG
       else
 	{
@@ -807,7 +808,7 @@  parse_flags (const char *s)
 	  copy[len] = '\0';
 	  non_fatal (_("unrecognized section flag `%s'"), copy);
 	  fatal (_("supported flags: %s"),
-		 "alloc, load, noload, readonly, debug, code, data, rom, exclude, share, contents, merge, strings");
+		 "alloc, load, noload, readonly, debug, code, data, rom, exclude, share, contents, merge, strings, large");
 	}
 
       s = snext;
diff --git a/binutils/testsuite/binutils-all/x86-64/large-sections-2.d b/binutils/testsuite/binutils-all/x86-64/large-sections-2.d
new file mode 100644
index 00000000000..29ace42cc9e
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/large-sections-2.d
@@ -0,0 +1,15 @@ 
+#source: large-sections.s
+#PROG: objcopy
+#as: --64
+#objcopy: --set-section-flags .ldata=alloc
+#readelf: -S -W
+
+#...
+  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AX[ \t]+.*
+#...
+  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WA[ \t]+.*
+#...
+  \[[ 0-9]+\] \.ltext.*[ \t]+PROGBITS[ \t0-9a-f]+AXl[ \t]+.*
+#...
+  \[[ 0-9]+\] \.ldata.*[ \t]+PROGBITS[ \t0-9a-f]+WA[ \t]+.*
+#pass
diff --git a/binutils/testsuite/binutils-all/x86-64/large-sections-2.s b/binutils/testsuite/binutils-all/x86-64/large-sections-2.s
new file mode 100644
index 00000000000..6f31aa93701
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/large-sections-2.s
@@ -0,0 +1,4 @@ 
+	.section .text, "axl"
+	nop
+	.section .data, "awl"
+	.byte 1
diff --git a/binutils/testsuite/binutils-all/x86-64/large-sections.d b/binutils/testsuite/binutils-all/x86-64/large-sections.d
new file mode 100644
index 00000000000..5d945e46ba3
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/large-sections.d
@@ -0,0 +1,14 @@ 
+#PROG: objcopy
+#as: --64
+#objcopy: --set-section-flags .text=alloc,readonly,code,large --set-section-flags .data=alloc,large
+#readelf: -S -W
+
+#...
+  \[[ 0-9]+\] \.text.*[ \t]+PROGBITS[ \t0-9a-f]+AXl[ \t]+.*
+#...
+  \[[ 0-9]+\] \.data.*[ \t]+PROGBITS[ \t0-9a-f]+WAl[ \t]+.*
+#...
+  \[[ 0-9]+\] \.ltext.*[ \t]+PROGBITS[ \t0-9a-f]+AXl[ \t]+.*
+#...
+  \[[ 0-9]+\] \.ldata.*[ \t]+PROGBITS[ \t0-9a-f]+WAl[ \t]+.*
+#pass
diff --git a/binutils/testsuite/binutils-all/x86-64/large-sections.s b/binutils/testsuite/binutils-all/x86-64/large-sections.s
new file mode 100644
index 00000000000..072e456a1ed
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/large-sections.s
@@ -0,0 +1,8 @@ 
+	.section .text, "ax"
+	nop
+	.section .data, "aw"
+	.byte 1
+	.section .ltext, "axl"
+	nop
+	.section .ldata, "awl"
+	.byte 1
diff --git a/include/elf/common.h b/include/elf/common.h
index ffa6b60bd2b..1397d60402e 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -588,6 +588,8 @@ 
 
 #define SHF_GNU_MBIND	0x01000000	/* Mbind section.  */
 
+#define SHF_X86_64_LARGE 0x10000000
+
 /* Compression types.  */
 #define ELFCOMPRESS_ZLIB   1		/* Compressed with zlib.  */
 #define ELFCOMPRESS_ZSTD   2		/* Compressed with zstd  */