[llvm] r216071 - Quick fix for an use after free.
lhames at gmail.com
Tue Aug 26 14:07:30 PDT 2014
I appear to have pasted my (very belated) response to "Don't own the buffer
in object::Binary" in the wrong thead. I'm not sure what I was thinking
with the RuntimeDyld comment - that all looks good. Your comment on
makeLTOModule makes perfect sense too. So no problems there after all.
This change looks good too. Please go for it!
On Tue, Aug 26, 2014 at 8:51 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> On 22 August 2014 19:42, Lang Hames <lhames at gmail.com> wrote:
> > Hi Rafael,
> > Sorry about the delayed review.
> NP, sorry for the delay on my side too.
> > Does this patch need to touch LTOModule? In any case, I think there's a
> > here: LTOModule has a create method that takes a file name, which means
> > LTOModule needs to own its underlying memory. Either LTOModule should own
> > its MemoryBuffer, or we should remove the LTOModule::createFromFile
> > The former is consistent with the current behavior - it's probably
> easier if
> > we stick with that for now.
> What makes it possible for LTOModule to not own the buffer is the fact
> that it parses the entire module before LTOModule::makeLTOModule
> returns. It needs to do that to provide
> LTO_SYMBOL_SCOPE_DEFAULT_CAN_BE_HIDDEN to ld64. At some point I would
> have wanted to make that optional, but given that tools/gold doesn't
> use LTOModule anymore, it seems better to just uses the simpler
> > Regarding the RuntimeDyld changes - if we can avoid it at all, I'd really
> > like to keep the Object ownership up in the ExecutionEngine, rather than
> > RuntimeDyld. The future direction for the JIT is pushing ownership
> > responsibility up to clients (or convenience adapters) and out of the JIT
> > itself. We can't go all the way with that yet, but we should manage the
> > ownership at the highest level that we can in-tree, which is the
> > ExecutionEngine.
> Not sure I follow. With the patch the ownership ends up in
> SmallVector<std::unique_ptr<MemoryBuffer>, 2> Buffers;
> which I added to MCJIT.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits