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

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - binutils list mailing list
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

objcopy.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Fangrui Song-2
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 :)
Reply | Threaded
Open this post in threaded view
|

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


Reply | Threaded
Open this post in threaded view
|

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

Fangrui Song-2
On Mon, Feb 10, 2020 at 8:24 AM Nick Clifton <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

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

Nick Clifton
In reply to this post by 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