[PATCH] D30458: [ELF] - Make Bss and BssRelRo sections to be synthetic by nature.

[Rui Ueyama via llvm-commits]

On Tue, Feb 28, 2017 at 4:49 PM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> >    // Dynamic section must be the last one in this list and dynamic
> >    // symbol table section (DynSymTab) must be the first one.
> >    finalizeSynthetic<ELFT>(
> > -      {In<ELFT>::DynSymTab, In<ELFT>::GnuHashTab, In<ELFT>::HashTab,
> > -       In<ELFT>::SymTab,    In<ELFT>::ShStrTab,   In<ELFT>::StrTab,
> > -       In<ELFT>::VerDef,    In<ELFT>::DynStrTab,  In<ELFT>::GdbIndex,
> > -       In<ELFT>::Got,       In<ELFT>::MipsGot,    In<ELFT>::IgotPlt,
> > -       In<ELFT>::GotPlt,    In<ELFT>::RelaDyn,    In<ELFT>::RelaIplt,
> > -       In<ELFT>::RelaPlt,   In<ELFT>::Plt,        In<ELFT>::Iplt,
> > -       In<ELFT>::Plt,       In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,
> > -       In<ELFT>::VerNeed,   In<ELFT>::Dynamic});
> > +      {In<ELFT>::Bss,        In<ELFT>::BssRelRo, In<ELFT>::DynSymTab,
> > +       In<ELFT>::GnuHashTab, In<ELFT>::HashTab,  In<ELFT>::SymTab,
> > +       In<ELFT>::ShStrTab,   In<ELFT>::StrTab,   In<ELFT>::VerDef,
> > +       In<ELFT>::DynStrTab,  In<ELFT>::GdbIndex, In<ELFT>::Got,
> > +       In<ELFT>::MipsGot,    In<ELFT>::IgotPlt,  In<ELFT>::GotPlt,
> > +       In<ELFT>::RelaDyn,    In<ELFT>::RelaIplt, In<ELFT>::RelaPlt,
> > +       In<ELFT>::Plt,        In<ELFT>::Iplt,     In<ELFT>::Plt,
> > +       In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,   In<ELFT>::VerNeed,
> > +       In<ELFT>::Dynamic});
>
> The comment is now out of date.
>
> > Index: ELF/Relocations.cpp
> > ===================================================================
> > --- ELF/Relocations.cpp
> > +++ ELF/Relocations.cpp
> > @@ -479,8 +479,8 @@
> >    // See if this symbol is in a read-only segment. If so, preserve the
> symbol's
> >    // memory protection by reserving space in the .bss.rel.ro section.
> >    bool IsReadOnly = isReadOnly<ELFT>(SS);
> > -  OutputSection *OSec = IsReadOnly ? Out::BssRelRo : Out::Bss;
> > -
> > +  OutputSection *OSec =
> > +      IsReadOnly ? In<ELFT>::BssRelRo->OutSec : In<ELFT>::Bss->OutSec;
>
> This is a bit odd. We create the In<ELFT>::BssRelRo and In<ELFT>::Bss,
> but they never hold anything. They are just a placeholder for output
> sections.
>
> I guess that instead of CopyRelSection it could be another
> SyntheticSection type that is explicitly just a placeholder for an
> output section.
>
> Not sure if that is worth it, but it should at least have a comment if
> we are to use CopyRelSection. Rui, what do you think?
>

I think the current code works satisfactory, and this patch doesn't seems
an improvement over it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170228/8044dbb7/attachment.html>