[lld] r248102 - COFF: Fix thread-safety bug.

[David Blaikie via llvm-commits]

On Mon, Oct 5, 2015 at 6:15 AM, Peter Collingbourne <peter at pcc.me.uk> wrote:

> LTOModule owns a Module under the covers,


Does it need to/does it (need to) use it for LLD's uses? Perhaps we could
expose that as an actual stateful object rather than global mutable/shared
state?

Ah, I see - it gets the "global" LLVMContext - it doesn't even matter about
the Module that LTOModule might have.

Rui: Does this code need to create the Modules (that it's creating in
parallel) all in the same LLVMContext? I imagine it probably does, in which
case I guess there's currently nothing to do but to lock/only build one
Module at a time. Probably.

- David


> so any client that creates multiple
> LTOModules in the same LLVMContext will be affected.
>
> Maybe it's time for something like an LLVMContext-wide mutex, but that
> would
> present its own design issues (e.g. we would need to define exactly where
> guards should go) so I'd prefer to let clients worry about this until we
> have a good sense of the right design.
>
> Peter
>
> On Sat, Sep 19, 2015 at 06:22:47PM -0700, Rui Ueyama via llvm-commits
> wrote:
> > I have no idea. CC'ing Peter since he may know about that.
> >
> > On Sat, Sep 19, 2015 at 5:31 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> > >
> > >
> > > On Sat, Sep 19, 2015 at 4:14 PM, Rui Ueyama via llvm-commits <
> > > llvm-commits at lists.llvm.org> wrote:
> > >
> > >> Author: ruiu
> > >> Date: Sat Sep 19 18:14:51 2015
> > >> New Revision: 248102
> > >>
> > >> URL: http://llvm.org/viewvc/llvm-project?rev=248102&view=rev
> > >> Log:
> > >> COFF: Fix thread-safety bug.
> > >>
> > >> LTOModule doesn't seem to be thread-safe, so guard that with mutex.
> > >>
> > >
> > > Yeah, that seems unfortunate.
> > >
> > > Any idea where the thread unsafety comes from & whether we can/should
> fix
> > > it down the stack rather than in lld?
> > >
> > >
> > >>
> > >> Modified:
> > >>     lld/trunk/COFF/InputFiles.cpp
> > >>     lld/trunk/COFF/InputFiles.h
> > >>
> > >> Modified: lld/trunk/COFF/InputFiles.cpp
> > >> URL:
> > >>
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=248102&r1=248101&r2=248102&view=diff
> > >>
> > >>
> ==============================================================================
> > >> --- lld/trunk/COFF/InputFiles.cpp (original)
> > >> +++ lld/trunk/COFF/InputFiles.cpp Sat Sep 19 18:14:51 2015
> > >> @@ -310,6 +310,9 @@ void ImportFile::parse() {
> > >>  }
> > >>
> > >>  void BitcodeFile::parse() {
> > >> +  // Usually parse() is thread-safe, but bitcode file is an
> exception.
> > >> +  std::lock_guard<std::mutex> Lock(Mu);
> > >> +
> > >>    std::string Err;
> > >>    M.reset(LTOModule::createFromBuffer(MB.getBufferStart(),
> > >>                                        MB.getBufferSize(),
> > >> @@ -356,5 +359,7 @@ MachineTypes BitcodeFile::getMachineType
> > >>    }
> > >>  }
> > >>
> > >> +std::mutex BitcodeFile::Mu;
> > >> +
> > >>  } // namespace coff
> > >>  } // namespace lld
> > >>
> > >> Modified: lld/trunk/COFF/InputFiles.h
> > >> URL:
> > >>
> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=248102&r1=248101&r2=248102&view=diff
> > >>
> > >>
> ==============================================================================
> > >> --- lld/trunk/COFF/InputFiles.h (original)
> > >> +++ lld/trunk/COFF/InputFiles.h Sat Sep 19 18:14:51 2015
> > >> @@ -17,6 +17,7 @@
> > >>  #include "llvm/Object/COFF.h"
> > >>  #include "llvm/Support/StringSaver.h"
> > >>  #include <memory>
> > >> +#include <mutex>
> > >>  #include <set>
> > >>  #include <vector>
> > >>
> > >> @@ -213,6 +214,7 @@ private:
> > >>    std::vector<SymbolBody *> SymbolBodies;
> > >>    llvm::BumpPtrAllocator Alloc;
> > >>    std::unique_ptr<LTOModule> M;
> > >> +  static std::mutex Mu;
> > >>  };
> > >>
> > >>  } // namespace coff
> > >>
> > >>
> > >> _______________________________________________
> > >> llvm-commits mailing list
> > >> llvm-commits at lists.llvm.org
> > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > >>
> > >
> > >
>
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
> --
> Peter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151005/2671a994/attachment.html>