[llvm] r216071 - Quick fix for an use after free.

[Lang Hames]

Hi Rafael,

The ExecutionEngine already owns the LL module, right? I think we should
actually make addObjectFile an owning operation too, for consistency.

Side note: In the long term I'd love to move to a world where
ExecutionEngine and its subclasses are non-owning and we provide
convenience classes to automatically manage ownership for clients with
simple use cases. A lot of JIT clients probably know better than
ExecutionEngine how they want their memory managed, and for them automatic
ownership is just a straightjacket.

Cheers,
Lang.


On Wed, Aug 20, 2014 at 8:40 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> Hi Lang,
>
> This was a quick fix for a fallout from the last part of my change
> that dropped the creation of dummy buffers for archive members. What
> is your long term view with regards to buffer/object ownership in the
> ExecutionEngine? One option would be to keep the existing
> addObjectFile for use with archive members (where there is one buffer
> for the archive, owned somewhere else) but also have a version that
> takes a OwningBinary for use in top level calls like this one. What do
> you think?
>
>
> On 20 August 2014 11:19, Rafael Espindola <rafael.espindola at gmail.com>
> wrote:
> > Author: rafael
> > Date: Wed Aug 20 10:19:37 2014
> > New Revision: 216071
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=216071&view=rev
> > Log:
> > Quick fix for an use after free.
> >
> > Modified:
> >     llvm/trunk/tools/lli/lli.cpp
> >
> > Modified: llvm/trunk/tools/lli/lli.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/lli.cpp?rev=216071&r1=216070&r2=216071&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/tools/lli/lli.cpp (original)
> > +++ llvm/trunk/tools/lli/lli.cpp Wed Aug 20 10:19:37 2014
> > @@ -529,6 +529,7 @@ int main(int argc, char **argv, char * c
> >      EE->addModule(std::move(XMod));
> >    }
> >
> > +  std::vector<std::unique_ptr<MemoryBuffer>> Buffers;
> >    for (unsigned i = 0, e = ExtraObjects.size(); i != e; ++i) {
> >      ErrorOr<object::OwningBinary<object::ObjectFile>> Obj =
> >          object::ObjectFile::createObjectFile(ExtraObjects[i]);
> > @@ -536,7 +537,9 @@ int main(int argc, char **argv, char * c
> >        Err.print(argv[0], errs());
> >        return 1;
> >      }
> > -    EE->addObjectFile(std::move(Obj.get().getBinary()));
> > +    object::OwningBinary<object::ObjectFile> &O = Obj.get();
> > +    EE->addObjectFile(std::move(O.getBinary()));
> > +    Buffers.push_back(std::move(O.getBuffer()));
> >    }
> >
> >    for (unsigned i = 0, e = ExtraArchives.size(); i != e; ++i) {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140820/c42b1746/attachment.html>