[lld] r226883 - Make access to LinkingContext::getNode safe.

[Jean-Daniel Dupas]

In that line:

if (FileNode *node = dyn_cast<FileNode>(nodes[i].get())) 

What happen if, between the call to nodes[i] and the call to ‘get()’, the vector is resized (by a call to parse() performed on a spawned task) ? Couldn’t the resizing invalidate the unique_ptr reference ?

Moreover, AFAIK std::vector member access is not thread safe, so even nodes[i] may fail if the vector can be resized concurrently by background parsing thread.

> Le 23 janv. 2015 à 19:58, Rui Ueyama <ruiu at google.com> a écrit :
> 
> The list of nodes the context object has is a shared resource -- by parsing a file, the parser may add more files to the input list. (That doesn't occur for ELF but may occur for COFF, since a COFF object may contain extra command line parameters which let the linker to read more files.)
> 
> On Fri, Jan 23, 2015 at 8:17 AM, Jean-Daniel Dupas <dev at xenonium.com <mailto:dev at xenonium.com>> wrote:
> Does it not reveal a race condition here ?
> An action preformed in background by the thread pool should not affect the behavior of the current thread.
> 
> > Le 23 janv. 2015 à 01:09, Rui Ueyama <ruiu at google.com <mailto:ruiu at google.com>> a écrit :
> >
> > Author: ruiu
> > Date: Thu Jan 22 18:09:05 2015
> > New Revision: 226883
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=226883&view=rev <http://llvm.org/viewvc/llvm-project?rev=226883&view=rev>
> > Log:
> > Make access to LinkingContext::getNode safe.
> >
> > Modified:
> >    lld/trunk/lib/Driver/Driver.cpp
> >
> > Modified: lld/trunk/lib/Driver/Driver.cpp
> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/Driver.cpp?rev=226883&r1=226882&r2=226883&view=diff <http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/Driver.cpp?rev=226883&r1=226882&r2=226883&view=diff>
> > ==============================================================================
> > --- lld/trunk/lib/Driver/Driver.cpp (original)
> > +++ lld/trunk/lib/Driver/Driver.cpp Thu Jan 22 18:09:05 2015
> > @@ -77,8 +77,11 @@ bool Driver::link(LinkingContext &contex
> >   if (context.getNodes().empty())
> >     return false;
> >
> > -  for (std::unique_ptr<Node> &ie : context.getNodes())
> > -    if (FileNode *node = dyn_cast<FileNode>(ie.get()))
> > +  // File::parse may add items to the node list which may invalidate
> > +  // existing iterators. Avoid using iterator to access elements.
> > +  std::vector<std::unique_ptr<Node>> &nodes = context.getNodes();
> > +  for (size_t i = 0; i < nodes.size(); ++i)
> > +    if (FileNode *node = dyn_cast<FileNode>(nodes[i].get()))
> >       context.getTaskGroup().spawn([node] { node->getFile()->parse(); });
> >
> >   std::vector<std::unique_ptr<File>> internalFiles;
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 
> 
> _______________________________________________
> 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/20150124/14d7d532/attachment.html>