[PATCH v6] Add retain attribute to place symbols in SHF_GNU_RETAIN section

H.J. Lu hjl.tools@gmail.com
Thu Feb 18 04:10:40 GMT 2021


On Wed, Feb 17, 2021 at 12:50 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Feb 17, 2021 at 12:14 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Wed, Feb 17, 2021 at 12:04:34PM -0800, H.J. Lu wrote:> >
> > > +  /* For -fretain-used-symbol, a "used" attribute also implies "retain".  */
> >
> > s/-symbol/-symbols/
>
> Fixed.
>
> > > +  if (flag_retain_used_symbols
> > > +      && attributes
> > > +      && (TREE_CODE (*node) == FUNCTION_DECL
> > > +       || (VAR_P (*node) && TREE_STATIC (*node))
> > > +       || (TREE_CODE (*node) == TYPE_DECL))
> >
> > What will "retain" do on TYPE_DECLs?  It only makes sense to me
> > for FUNCTION_DECL or non-automatic VAR_DECLs, unless for types
> > it means the types with that result in vars with those types automatically
> > getting "retain", but there is no code for that and I don't see "used"
> > handled that way.
>
> Fixed.
>
> > > +  if (SUPPORTS_SHF_GNU_RETAIN
> > > +      && (TREE_CODE (node) == FUNCTION_DECL
> > > +       || (VAR_P (node) && TREE_STATIC (node))
> > > +       || (TREE_CODE (node) == TYPE_DECL)))
> >
> > Likewise.
>
> Fixed.
>
> > > +    ;
> > > +  else
> > > +    {
> > > +      warning (OPT_Wattributes, "%qE attribute ignored", name);
> > > +      *no_add_attrs = true;
> > > +    }
> > > +
> > > +  return NULL_TREE;
> > > +}
> > > +
> > >  /* Handle a "externally_visible" attribute; arguments as in
> > >     struct attribute_spec.handler.  */
> > >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index c75dd36843e..70842481d4d 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -2404,6 +2404,10 @@ frerun-loop-opt
> > >  Common Ignore
> > >  Does nothing.  Preserved for backward compatibility.
> > >
> > > +fretain-used-symbols
> > > +Common Var(flag_retain_used_symbols)
> > > +Make the used attribute to imply the retain attribute if supported.
> >
> > English is not my native language, but I think the "to " doesn't belong
> > here.
>
> Fixed.
>
> > > +@item -fretain-used-symbols
> > > +@opindex fretain-used-symbols
> > > +On systems with recent GNU assembler and linker, the compiler makes
> >
> > I think we should avoid recent here, new/old/recent etc. are relative terms.
> > Either use exact version (is it 2.36 or later) or say GNU assembler and
> > linker that supports the @code{.retain} directive or something similar.
>
> Fixed.
>
> > >    flags |= SECTION_NAMED;
> > > -  if (SUPPORTS_SHF_GNU_RETAIN
> > > -      && decl != nullptr
> > > +  if (decl != nullptr
> > >        && DECL_P (decl)
> > > -      && DECL_PRESERVE_P (decl))
> > > +      && DECL_PRESERVE_P (decl)
> > > +      && lookup_attribute ("retain", DECL_ATTRIBUTES (decl)))
> > >      flags |= SECTION_RETAIN;
> > >    if (*slot == NULL)
> > >      {
> >
> > I think none of the varasm.c code should use DECL_PRESERVE_P when it means
> > retain, just lookup_attribute.
>
> Fixed.
>
> > > @@ -487,7 +487,8 @@ resolve_unique_section (tree decl, int reloc ATTRIBUTE_UNUSED,
> > >    if (DECL_SECTION_NAME (decl) == NULL
> > >        && targetm_common.have_named_sections
> > >        && (flag_function_or_data_sections
> > > -       || (SUPPORTS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl))
> > > +       || (DECL_PRESERVE_P (decl)
> > > +           && lookup_attribute ("retain", DECL_ATTRIBUTES (decl)))
> > >         || DECL_COMDAT_GROUP (decl)))
> > >      {
> > >        targetm.asm_out.unique_section (decl, reloc);
> > > @@ -1227,7 +1228,8 @@ get_variable_section (tree decl, bool prefer_noswitch_p)
> > >      vnode->get_constructor ();
> > >
> > >    if (DECL_COMMON (decl)
> > > -      && !(SUPPORTS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl)))
> > > +      && !(DECL_PRESERVE_P (decl)
> > > +        && lookup_attribute ("retain", DECL_ATTRIBUTES (decl))))
> > >      {
> > >        /* If the decl has been given an explicit section name, or it resides
> > >        in a non-generic address space, then it isn't common, and shouldn't
> > > @@ -7761,12 +7763,14 @@ switch_to_section (section *new_section, tree decl)
> > >  {
> > >    if (in_section == new_section)
> > >      {
> > > -      if (SUPPORTS_SHF_GNU_RETAIN
> > > -       && (new_section->common.flags & SECTION_NAMED)
> > > +      if ((new_section->common.flags & SECTION_NAMED)
> > >         && decl != nullptr
> > >         && DECL_P (decl)
> > >         && (!!DECL_PRESERVE_P (decl)
> > > -           != !!(new_section->common.flags & SECTION_RETAIN)))
> > > +           != !!(new_section->common.flags & SECTION_RETAIN))
> > > +       && (lookup_attribute ("retain",
> > > +                             DECL_ATTRIBUTES (new_section->named.decl))
> > > +           || lookup_attribute ("retain", DECL_ATTRIBUTES (decl))))
> > >       {
> > >         /* If the SECTION_RETAIN bit doesn't match, switch to a new
> > >            section.  */
> > > --
> > > 2.29.2
> > >
>
> Here is the updated patch.
>

Here is the updated patch.   Tested on Linux/x86-64 with binutils
master branch.   OK for master?

Thanks.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v6-0001-Add-retain-attribute-to-place-symbols-in-SHF_GNU_.patch
Type: text/x-patch
Size: 24703 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210217/7130b514/attachment-0001.bin>


More information about the Gcc-patches mailing list