[MCJIT][DebugInfo] [PATCH] Remove object-ownership from RuntimeDyld.

[Eric Christopher]

Hi Lang,

In general the DI changes look ok (modulo Dave's comments). It's a
really large patch so splitting it up would be appreciated for a
closer look.

The symbol lookup via the passed in function feels awkward (no better
idea offhand so I'm not rejecting it, just feels awkward), can we get
that part of the patch especially split out and we can look at it? :)

-eric

On Fri, Oct 10, 2014 at 2:49 PM, Lang Hames <lhames at gmail.com> wrote:
> Hi All,
>
> The attached patch removes object file ownership from RuntimeDyld. Key
> changes are:
>
> (1) Instead of a taking a unique_ptr<ObjectFile>, the
> RuntimeDyld::loadObject method will now take a const ObjectFile reference.
> Instead of returning an owning wrapper (ObjectImage) for the loaded object,
> RuntimeDyld::loadObject now returns a "LoadedObjectInfo" instance, which can
> be queried for information about where the object's sections were loaded.
>
> (2) The classes that are currently used to manage object file ownership
> within RuntimeDyld (ObjectBuffer and ObjectImage) are removed from the API
> and turned into implementation details in MCJIT and RuntimeDyldELF
> respectively.
>
> (3) Minor changes are made to the DIContext and DWARFContext classes to
> support debugging objects loaded by RuntimeDyld.
>
> (4) All use-sites of loadObject are updated.
>
> Background:
>
> RuntimeDyld currently takes ownership of object files to be loaded/linked.
> I'm not sure there was ever a strong reason for this, but these days the
> only reason to have ownership in RuntimeDyld is to support the flawed
> debugging registration system used by RuntimeDyldELF. This system relies on
> rewriting objects in place to update address fields of the loaded
> symbols/sections. The problem is that ObjectFile instances are backed by
> MemoryBuffer instances, which (according to the MemoryBuffer contract)
> represent constant memory. RuntimeDyldELF casts away the constness of the
> underlying memory to make modifications. Usually the memory backing the
> MemoryBuffer is writable, in which case this trick works. Sometimes the
> memory is not writable (e.g. when mmaping files from disk), and you crash
> the JIT. See http://llvm.org/PR20722 .
>
> ELF debugging is important to a number of clients, and we haven't yet
> fleshed out a safe, generic system of debugger registration for the JIT, so
> I don't want to remove it. In lieu of that, this patch opts to clean up the
> interface to RuntimeDyld, and contain the existing craziness within
> RuntimeDyldELF. ELF objects will be cloned to writable memory before being
> modified (still a violation of the MemoryBuffer contract, but safe in
> practice, and no worse than what we had). MachO support is unaffected. Once
> we have a better debugging registration system for the JIT (which is in the
> works) we can go back and clean the rest of the nastiness out of
> RuntimeDyldELF.
>
> General feedback and style comments welcome. I could also particularly use
> feedback from debugger experts (Dave, Eric, Adrian) on the changes to
> DIContext/DWARFContext. I think they're sane, but I'm not intimately
> familiar with the DebugInfo library.
>
> Hopefully another step towards a cleaner LLVM JIT.
>
> Cheers,
> Lang.