[PATCH] [lld] Proposal and example for changing Atom attributes.

[Rui Ueyama]

That sounds fine to me. Even if we want to make it mutable (looks like we
do), it shouldn't be mixed with this patch.

On Tue, Aug 19, 2014 at 6:44 PM, Nick Kledzik <kledzik at apple.com> wrote:

>
> On Aug 19, 2014, at 6:20 PM, Michael Spencer <bigcheesegs at gmail.com>
> wrote:
>
> > On Tue, Aug 19, 2014 at 3:57 PM, Rui Ueyama <ruiu at google.com> wrote:
> >> On Tue, Aug 19, 2014 at 3:29 PM, Shankar Easwaran <
> shankare at codeaurora.org>
> >> wrote:
> >>>
> >>> On 8/19/2014 5:16 PM, Nick Kledzik wrote:
> >>>>
> >>>> ELF has the concept of bindiing and visibility.
> >>>>
> >>>> Binding can be STB_LOCAL, STB_GLOBAL, STB_WEAK which could be modeled
> >>>> using scope.
> >>>>
> >>>> Visibility plays a role mainly in dynamic linking (executables and
> >>>> dynamic libraries).  Common examples of visibility are STV_DEFAULT.
> There
> >>>> are other variants of visibility STV_HIDDEN, STV_PROTECTED,
> STV_INTERNAL.
> >>>>
> >>>> <snip from ELF abi for convenience>
> >>>>
> >>>> The ELF application binary interface (ABI) defines the visibility of
> >>>> symbols. Generally, it defines four classes, but in most cases, only
> two of
> >>>> them are more commonly used:
> >>>> STV_DEFAULT - Symbols defined with it will be exported. In other
> words,
> >>>> it declares that symbols are visible everywhere.
> >>>> STV_HIDDEN - Symbols defined with it will not be exported and cannot
> be
> >>>> used from other objects.
> >>>> STV_PROTECTED: The symbol is visible outside the current executable or
> >>>> shared object, but it may not be overridden. In other words, if a
> protected
> >>>> symbol in a shared library is referenced by an other code in the
> shared
> >>>> library, the other code will always reference the symbol in the shared
> >>>> library, even if the executable defines a symbol with the same name.
> >>>> STV_INTERNAL: The symbol is not accessible outside the current
> executable
> >>>> or shared library.
> >>>> </end snip>
> >>>>
> >>>> The main problem I see with the setScope function from when called
> from a
> >>>> callback function is, the atom loses the original property of how it
> >>>> appeared in the object file.
> >>>> Yes.  That is the big model change here.  Previously, the atoms were
> >>>> always read-only.  Now we are saying Passes and the LinkingContext may
> >>>> modify the atoms as the linking progresses.  The goal being that by
> the end
> >>>> of the link, the atoms reflect what the Writer needs.  If we don’t
> allow
> >>>> this, then we need to invent some sidecar somewhere which tracks the
> extra
> >>>> information Writers need.
> >>>
> >>> I think a sidecar model would be more beneficial here, Just my $0.02.
> >>>
> >>> If we need something in the future its easier to add to the sidecar,
> >>> rather than changing/looking for an attribute in the Atoms.
> >>>
> >>> References are one way to record that information, I was thinking if we
> >>> have a key value pair of recognized keys and values it might be easier
> to
> >>> look for as well.
> >>
> >>
> >> The reference is not a sidecar; it's a part of the graph (edge). Along
> with
> >> the atom (vertex) it consists the entire graph. By sidecar, I guess Nick
> >> suggested having a completely separate data structure than the graph. I
> >> basically agree with Nick that maintaining such information besides the
> >> graph is going to be complicated. It would annotate the graph (in that
> sense
> >> it's logically mutate the graph) while we wouldn't actually mutate the
> >> graph. Maybe we should aggregate the information in one place, which is
> the
> >> graph.
> >
> > I agree with not having a side data structure that would have to be
> > checked on every access due to performance concerns. Mutating the
> > graph is fine, and we already do it with references.
>
> I started down the path of mutating-the-graph because it seemed like an
> obvious solution to Simon’s issue where some symbols needed to be added to
> the .dynsym.  Seems like all we needed was a new method on DefinedAtom
> called setDynamicExport()...
>
> It also seemed like I could use the same mechanism to implement the darwin
> -exported_symbols_list linker option, so I worked up this patch.  But for
> -exported_symbols_list support, I don’t really need to modify the atoms.
> The driver is already passing the list of symbols to the
> MachOLinkingContext.  I could just leave it there (as a StringSet) and
> later in the MachOWriter, I could consult that StringSet to determine which
> bits to set in the mach-o symbol table (that is the MachOLinkingContext is
> the sidecar).
>
> So, I surveyed the existing ld64 linker for modifications of atoms and
> found five cases. All of them can be implemented without the need to modify
> any atoms, by just using the “notify” hooks to update a StringSet in the
> MachOLinkingContext.
>
> Given that, I’ll gut this patch to just add the “notify” methods.  For
> Simon’s bug, he can implement the notify methods in ELFLinkingContext and
> when a defined atom overrides a weak undefined atom, he can add its name to
> a new StringSet ivar (in ELFLinkingContext), and later in the ELF Writer
> consult that StringSet when building the .dynsym.
>
> -Nick
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140819/7226b2e8/attachment.html>