[PATCH] Support 'exclude' in objcopy --set-section-flags

[PATCH] Support 'exclude' in objcopy --set-section-flags

[Fangrui Song via binutils]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]

https://sourceware.org/bugzilla/show_bug.cgi?id=25371

commit 18ae9cc1db45e2e7f6467b91d8abbc5eb45fbaa5 makes SHF_EXCLUDE
generic, not like other SHF_MASKPROC flags. So we do not preserve
SHF_EXCLUDE when setting sh_flags.

--set-section-flags .foo= => clear SHF_EXCLUDE
--set-section-flags .foo=exclude => set SHF_EXCLUDE

[-- Attachment #2: objcopy.patch --]
[-- Type: text/x-diff, Size: 3350 bytes --]

From cddb761804cf4e4bf3b67fef1ef0af3dbe07cd2b Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Thu, 16 Jan 2020 00:43:14 -0800
Subject: [PATCH] Support 'exclude' in objcopy --set-section-flags
To: binutils@sourceware.org

---
 bfd/elf.c                  | 2 +-
 binutils/ChangeLog         | 4 ++++
 binutils/doc/binutils.texi | 8 ++++----
 binutils/objcopy.c         | 3 ++-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 08aaab644a..9849d2d53c 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7766,7 +7766,7 @@ _bfd_elf_init_private_section_data (bfd *ibfd,
 
   /* FIXME: Is this correct for all OS/PROC specific flags?  */
   elf_section_flags (osec) |= (elf_section_flags (isec)
-			       & (SHF_MASKOS | SHF_MASKPROC));
+                              & (SHF_MASKOS | SHF_MASKPROC) & ~SHF_EXCLUDE);
 
   /* Copy sh_info from input for mbind section.  */
   if ((elf_tdata (ibfd)->has_gnu_osabi & elf_gnu_osabi_mbind) != 0
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 3737672832..398531ecb2 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-16  Fangrui Song  <maskray@google.com>
+
+	* objcopy.c (parse_flags): Handle "exclude".
+
 2020-01-13  Nick Clifton  <nickc@redhat.com>
 
 	* objdump.c (disassemble_bytes): Remove C99-ism.
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 669bee968f..289d7b14a3 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -1644,10 +1644,10 @@ 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{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
+@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.
 
 @item --set-section-alignment @var{sectionpattern}=@var{align}
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index ef3b693be4..3cee65de50 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -780,6 +780,7 @@ parse_flags (const char *s)
       PARSE_FLAG ("code", SEC_CODE);
       PARSE_FLAG ("data", SEC_DATA);
       PARSE_FLAG ("rom", SEC_ROM);
+      PARSE_FLAG ("exclude", SEC_EXCLUDE);
       PARSE_FLAG ("share", SEC_COFF_SHARED);
       PARSE_FLAG ("contents", SEC_HAS_CONTENTS);
       PARSE_FLAG ("merge", SEC_MERGE);
@@ -794,7 +795,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, share, contents, merge, strings");
+		 "alloc, load, noload, readonly, debug, code, data, rom, exclude, share, contents, merge, strings");
 	}
 
       s = snext;
-- 
2.25.0

Re: [PATCH] Support 'exclude' in objcopy --set-section-flags

[Fangrui Song]

On 2020-01-16, Fangrui Song via binutils wrote:
>https://sourceware.org/bugzilla/show_bug.cgi?id=25371
>
>commit 18ae9cc1db45e2e7f6467b91d8abbc5eb45fbaa5 makes SHF_EXCLUDE
>generic, not like other SHF_MASKPROC flags. So we do not preserve
>SHF_EXCLUDE when setting sh_flags.
>
>--set-section-flags .foo= => clear SHF_EXCLUDE
>--set-section-flags .foo=exclude => set SHF_EXCLUDE

Ping :)

Re: [PATCH] Support 'exclude' in objcopy --set-section-flags

[Nick Clifton]

Hi Fangrui,

>> --set-section-flags .foo= => clear SHF_EXCLUDE
>> --set-section-flags .foo=exclude => set SHF_EXCLUDE
> 
> Ping :)

Approved and applied - mostly.  You also had an undocumented
change to _bfd_elf_init_private_section_data() which would
remove the SHF_EXCLUDE bit from sections with OS or PROC
specific flags.  This breaks several linker tests, and seemed
to be unrelated to the main purpose of the patch, so I discarded
it.

Cheers
  Nick

Re: [PATCH] Support 'exclude' in objcopy --set-section-flags

[Fangrui Song]

On Mon, Feb 10, 2020 at 8:24 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Fangrui,
>
> >> --set-section-flags .foo= => clear SHF_EXCLUDE
> >> --set-section-flags .foo=exclude => set SHF_EXCLUDE
> >
> > Ping :)
>
> Approved and applied - mostly.  You also had an undocumented
> change to _bfd_elf_init_private_section_data() which would
> remove the SHF_EXCLUDE bit from sections with OS or PROC
> specific flags.  This breaks several linker tests, and seemed
> to be unrelated to the main purpose of the patch, so I discarded
> it.

Thanks! I'm curious to learn what SHF_MASKPROC tests on what archs are affected,
and how you made the test.

The main issue is that (SHF_EXCLUDE & SHF_MASKPROC) != 0.
SHF_EXCLUDE = SHF_ARM_COMDEF = SHF_MIPS_STRINGS = SHF_PARISC_SBP

If we don't drop the SHF_EXCLUDE bit,
https://sourceware.org/bugzilla/show_bug.cgi?id=25371#c1
exclude will behave differently from other flags.

% cat a.s
.section foo,"e"
% as a.s -o a.o
% llvm-objcopy --set-section-flags foo=readonly a.o b.o   #
LLVM>=10.0.0  , SHF_EXCLUDE is dropped
% objcopy --set-section-flags foo=readonly a.o b.o   # SHF_EXCLUDE is kept

This is probably not a problem because in practice no users will use
--set-section-flags on sections with such flags.

Re: [PATCH] Support 'exclude' in objcopy --set-section-flags

[Nick Clifton]

Hi Fangrui,

> Thanks! I'm curious to learn what SHF_MASKPROC tests on what archs are affected,
> and how you made the test.

Well I have a build-lots-of-toolchains script that I run to test patches.
These are the linker failures that show up with that change to elf.c applied:

  LD REGRESSION: ld-elf/exclude3c    
  LD REGRESSION: ld-elf/pr20528a    
  LD REGRESSION: ld-elf/pr20528b    

This is for lots of architectures (x86_64, arm, aarch64, ppc, etc).  Now it may
well be that these tests are inaccurate and need to be updated.  But that
should be done in sync with applying the patch, rather than afterwards.


> The main issue is that (SHF_EXCLUDE & SHF_MASKPROC) != 0.
> SHF_EXCLUDE = SHF_ARM_COMDEF = SHF_MIPS_STRINGS = SHF_PARISC_SBP

Which implies that the value being used for SHF_EXCLUDE is wrong.  It
ought to be a generic value.  I assume that it is too late to change 
this however.

So it seems to me that what we need to do is to provide a backend
hook to handle the copying of OS/PROC flag bits from an input bfd to 
an output bfd.  The default could be to strip the SHF_EXCLUDE, but
backends should be able to decide.

Cheers
  Nick