V3 [PATCH 1/2] Switch to a new section if the SECTION_RETAIN bit doesn't match

[H.J. Lu]

On Mon, Dec 14, 2020 at 5:06 PM Jeff Law <law@redhat.com> wrote:
>
>
>
> On 12/8/20 5:51 AM, H.J. Lu wrote:
> > When definitions marked with used attribute and unmarked definitions are
> > placed in the section with the same name, switch to a new section if the
> > SECTION_RETAIN bit doesn't match.
> >
> > gcc/
> >
> >       PR target/98146
> >       * output.h (switch_to_section): Add a tree argument, default to
> >       nullptr.
> >       * varasm.c (get_section): If the SECTION_RETAIN bit doesn't match,
> >       return and switch to a new section later.
> >       (assemble_start_function): Pass decl to switch_to_section.
> >       (assemble_variable): Likewise.
> >       (switch_to_section): If the SECTION_RETAIN bit doesn't match,
> >       switch to a new section.
> >
> > gcc/testsuite/
> >
> >       PR target/98146
> >       * c-c++-common/attr-used-5.c: New test.
> >       * c-c++-common/attr-used-6.c: Likewise.
> >       * c-c++-common/attr-used-7.c: Likewise.
> >       * c-c++-common/attr-used-8.c: Likewise.
> >       * c-c++-common/attr-used-9.c: Likewise.
> > ---
> >  gcc/output.h                             |  2 +-
> >  gcc/testsuite/c-c++-common/attr-used-5.c | 26 ++++++++++++++++++++
> >  gcc/testsuite/c-c++-common/attr-used-6.c | 26 ++++++++++++++++++++
> >  gcc/testsuite/c-c++-common/attr-used-7.c |  8 +++++++
> >  gcc/testsuite/c-c++-common/attr-used-8.c |  8 +++++++
> >  gcc/testsuite/c-c++-common/attr-used-9.c | 28 ++++++++++++++++++++++
> >  gcc/varasm.c                             | 30 ++++++++++++++++++++----
> >  7 files changed, 123 insertions(+), 5 deletions(-)
> >  create mode 100644 gcc/testsuite/c-c++-common/attr-used-5.c
> >  create mode 100644 gcc/testsuite/c-c++-common/attr-used-6.c
> >  create mode 100644 gcc/testsuite/c-c++-common/attr-used-7.c
> >  create mode 100644 gcc/testsuite/c-c++-common/attr-used-8.c
> >  create mode 100644 gcc/testsuite/c-c++-common/attr-used-9.c
> >
> >
> > diff --git a/gcc/varasm.c b/gcc/varasm.c
> > index 0fac3688828..c5ea29c4e4c 100644
> > --- a/gcc/varasm.c
> > +++ b/gcc/varasm.c
> > @@ -342,6 +342,11 @@ get_section (const char *name, unsigned int flags, tree decl,
> >             sect->common.flags |= (SECTION_WRITE | SECTION_RELRO);
> >             return sect;
> >           }
> > +       /* If the SECTION_RETAIN bit doesn't match, return and switch
> > +          to a new section later.  */
> > +       if ((sect->common.flags & SECTION_RETAIN)
> > +           != (flags & SECTION_RETAIN))
> > +         return sect;
> At the least you need to fix the function comment as it would no longer
> match the behavior in this case.  It also seems to me that having
> SECTION_RETAIN be a special case like this just seems wrong.   In my
> mind there isn't anything fundamentally different with SECTION_RETAIN vs
> other symbols that it deserves to be special cased like this.

Any symbols with SHF_XXX can be special, for example SHF_TLS.

> Clearly we need to fix something as these bits are causing significant
> fallout, but I'm just not sure what the right fix ought to be.
>
> jeff
>


-- 
H.J.