On Fri, Dec 4, 2020 at 7:29 AM Jozef Lawrynowicz
<jozef.l@mittosystems.com> wrote:
>
> On Fri, Dec 04, 2020 at 05:52:54AM -0800, H.J. Lu wrote:
> > For
> >       .globl  foo2
> >       .section        .data.foo,"aR"
> >       .align 4
> >       .type   foo2, @object
> >       .size   foo2, 4
> > foo2:
> >       .long   2
> >       .globl  foo1
> >       .section        .data.foo
> >       .align 4
> >       .type   foo1, @object
> >       .size   foo1, 4
> > foo1:
> >       .long   1
> >
> > generate a new section if the SHF_GNU_RETAIN bit doesn't match.
> >
> >       * config/obj-elf.c (SEC_ASSEMBLER_SHF_MASK): New.
> >       (get_section_by_match): Also check if SEC_ASSEMBLER_SHF_MASK of
> >       sh_flags matches.  Rename info to sh_info.
> >       (obj_elf_change_section): Rename info to sh_info.
> >       (obj_elf_section): Rename info to sh_info.  Set sh_flags for
> >       SHF_GNU_RETAIN.
> >       * config/obj-elf.h (elf_section_match): Rename info to sh_info.
> >       Add sh_flags.
> >       * testsuite/gas/elf/elf.exp: Run section27.
> >       * testsuite/gas/elf/section24b.d: Updated.
> >       * testsuite/gas/elf/section27.d: New file.
> >       * testsuite/gas/elf/section27.s: Likewise.
> > ---
> >  gas/config/obj-elf.c               | 21 ++++++++++++------
> >  gas/config/obj-elf.h               |  3 ++-
> >  gas/testsuite/gas/elf/elf.exp      |  1 +
> >  gas/testsuite/gas/elf/section24b.d | 10 ++++++---
> >  gas/testsuite/gas/elf/section27.d  | 14 ++++++++++++
> >  gas/testsuite/gas/elf/section27.s  | 34 ++++++++++++++++++++++++++++++
> >  6 files changed, 73 insertions(+), 10 deletions(-)
> >  create mode 100644 gas/testsuite/gas/elf/section27.d
> >  create mode 100644 gas/testsuite/gas/elf/section27.s
>
> I just want to check that we are OK with the fact that a .section
> directive without any flags will always use the default flags, even if
> the section previously had the 'R' flag set. I suppose this is just
> standard behavior and how the .section directive has always behaved.

Correct.

> $ cat asm-tester.s
> .section .data.foo,"awR"
> .word 0
> .section .data.foo
> .word 0
> $ as asm-tester.s -o tester.o
> $ readelf --wide -S tester.o
> ...
>   [ 4] .data.foo         PROGBITS        0000000000000000 000040 000002 00 WAR  0   0  1
>   [ 5] .data.foo         PROGBITS        0000000000000000 000042 000002 00  WA  0   0  1
> ...
>
> Also, I think the following change should be added, since the OR'ing of
> SHF_GNU_RETAIN flag between sections, from the original patch, is now
> redundant.
>
> diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
> index 54d42d9ecb..9ac53e4d0e 100644
> --- a/gas/config/obj-elf.c
> +++ b/gas/config/obj-elf.c
> @@ -806,17 +806,9 @@ obj_elf_change_section (const char *name,
>                 as_bad (_("changed section attributes for %s"), name);
>             }
>           else
> -           {
> -             /* Don't overwrite a previously set SHF_GNU_RETAIN flag for the
> -                section.  The entire section must be marked retained.  */
> -             if ((elf_tdata (stdoutput)->has_gnu_osabi & elf_gnu_osabi_retain)
> -                 && ((elf_section_flags (old_sec) & SHF_GNU_RETAIN)))
> -               attr |= SHF_GNU_RETAIN;
> -
> -             /* FIXME: Maybe we should consider removing a previously set
> -                processor or application specific attribute as suspicious ?  */
> -             elf_section_flags (sec) = attr;
> -           }
> +           /* FIXME: Maybe we should consider removing a previously set
> +              processor or application specific attribute as suspicious ?  */
> +           elf_section_flags (sec) = attr;
>
>           if ((flags & SEC_MERGE) && old_sec->entsize != (unsigned) entsize)
>             as_bad (_("changed section entity size for %s"), name);
>

Fixed.

Here is the updated patch.  OK for master?

Thanks.

-- 
H.J.