[llvm] r208257 - [RuntimeDyld] Make RuntimeDyldImpl::resolveExternalSymbols preserve the

[Lang Hames]

Because StringMap's copy constructor body is:

 assert(RHS.empty() && "Copy ctor from non-empty stringmap not implemented
yet!");

:)


On Wed, May 7, 2014 at 4:16 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Wed, May 7, 2014 at 3:34 PM, Lang Hames <lhames at gmail.com> wrote:
> > Author: lhames
> > Date: Wed May  7 17:34:08 2014
> > New Revision: 208257
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=208257&view=rev
> > Log:
> > [RuntimeDyld] Make RuntimeDyldImpl::resolveExternalSymbols preserve the
> > relocation entries it applies.
> >
> > Prior to this patch, RuntimeDyldImpl::resolveExternalSymbols discarded
> > relocations for external symbols once they had been applied. This causes
> issues
> > if the client calls MCJIT::finalizeLoadedModules more than once, and
> updates the
> > location of any symbols in between (e.g. by calling
> MCJIT::mapSectionAddress).
> >
> > No test case yet: None of our in-tree memory managers support moving
> sections
> > around. I'll have to hack up a dummy memory manager before I can write a
> unit
> > test.
> >
> > Fixes <rdar://problem/16764378>
> >
> >
> > Modified:
> >     llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
> >
> > Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=208257&r1=208256&r2=208257&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)
> > +++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Wed May
>  7 17:34:08 2014
> > @@ -620,6 +620,8 @@ void RuntimeDyldImpl::resolveRelocationL
> >  }
> >
> >  void RuntimeDyldImpl::resolveExternalSymbols() {
> > +  StringMap<RelocationList> ProcessedSymbols;
> > +
> >    while (!ExternalSymbolRelocations.empty()) {
> >      StringMap<RelocationList>::iterator i =
> ExternalSymbolRelocations.begin();
> >
> > @@ -665,8 +667,20 @@ void RuntimeDyldImpl::resolveExternalSym
> >        resolveRelocationList(Relocs, Addr);
> >      }
> >
> > +    ProcessedSymbols[i->first()] = i->second;
> >      ExternalSymbolRelocations.erase(i);
> >    }
> > +
> > +  // Restore the relocation entries that were consumed in the loop
> above:
> > +  //
> > +  // FIXME: Replace the following loop with:
> > +  //           std::swap(ProcessedSymbols, ExternalSymbolRelocations)
>
> Presumably just: ExternalSymbolRelocations = std::move(ProcessedSymbols);
>
> swap was what you had to use for this back in C++98 when there were no
> move semantics.
>
> (but, yes, in either case, StringMap needs move semantic support to
> rewrite it that way)
>
> > +  //        once StringMap has copy and move construction.
> > +  for (StringMap<RelocationList>::iterator I = ProcessedSymbols.begin(),
> > +                                           E = ProcessedSymbols.end();
> > +       I != E; ++I) {
> > +    ExternalSymbolRelocations[I->first()] = I->second;
>
> But this code looks about as expensive as a copy anyway, so why not
> just write this as suggested above (Ext = std::move(Proc);) and then
> it'll just get 'better' if/when StringMap grows move semantics?
>
> > +  }
> >  }
> >
> >
>  //===----------------------------------------------------------------------===//
> >
> >
> > _______________________________________________
> > 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/20140507/e6f9327d/attachment.html>