[llvm-dev] LLD COFF library: crashes when lld::coff::link is called twice

[Hans Wennborg via llvm-dev]

Oh, it landed before the branch so it's already there. Please ignore me :-)

On Wed, Aug 8, 2018 at 5:06 PM, Hans Wennborg <hans at chromium.org> wrote:
> Ah, I see it's r338042. I'll get that merged to 7.0.
>
> On Wed, Aug 8, 2018 at 4:58 PM, Andrew Kelley <superjoe30 at gmail.com> wrote:
>> Hans,
>>
>> Apologies for not updating the list. I went through the formal
>> reviews.llvm.org process, and this has already been merged into trunk, I
>> believe.
>>
>> Regards,
>> Andrew
>>
>> On Wed, Aug 8, 2018, 10:50 AM Hans Wennborg <hans at chromium.org> wrote:
>>>
>>> +Rui and Peter
>>>
>>> On Wed, Jul 25, 2018 at 8:34 AM, Andrew Kelley via llvm-dev
>>> <llvm-dev at lists.llvm.org> wrote:
>>> > Here's a fix:
>>> >
>>> > --- a/lld/COFF/Driver.cpp
>>> > +++ b/lld/COFF/Driver.cpp
>>> > @@ -72,6 +72,9 @@ bool link(ArrayRef<const char *> Args, bool
>>> > CanExitEarly,
>>> > raw_ostream &Diag) {
>>> >      exitLld(errorCount() ? 1 : 0);
>>> >
>>> >    freeArena();
>>> > +  ObjFile::Instances.clear();
>>> > +  ImportFile::Instances.clear();
>>> > +  BitcodeFile::Instances.clear();
>>> >    return !errorCount();
>>> >  }
>>> >
>>> > I don't know how to make a test for this, since it depends on running
>>> > LLD
>>> > twice in the same process.
>>> >
>>> > Can I get some assistance trying to get this fix upstreamed?
>>> >
>>> > On Wed, Jul 25, 2018 at 2:18 AM, Andrew Kelley <superjoe30 at gmail.com>
>>> > wrote:
>>> >>
>>> >> If you call lld::coff::link twice, the second time gives this
>>> >> backtrace:
>>> >>
>>> >>      msvcp140d.dll!00007ffc35830806()    Unknown
>>> >> >    zig.exe!std::_Debug_pointer<lld::coff::Chunk * __ptr64
>>> >> > const>(lld::coff::Chunk * const * _Ptr, const wchar_t * _File,
>>> >> > unsigned int
>>> >> > _Line) Line 926    C++
>>> >>      zig.exe!std::_Debug_range2<lld::coff::Chunk * __ptr64 const *
>>> >> __ptr64>(lld::coff::Chunk * const * _First, lld::coff::Chunk * const *
>>> >> _Last, const wchar_t * _File, unsigned int _Line,
>>> >> std::random_access_iterator_tag __formal) Line 958    C++
>>> >>      zig.exe!std::_Debug_range<lld::coff::Chunk * __ptr64 const *
>>> >> __ptr64>(lld::coff::Chunk * const * _First, lld::coff::Chunk * const *
>>> >> _Last, const wchar_t * _File, unsigned int _Line) Line 968    C++
>>> >>      zig.exe!std::vector<lld::coff::Chunk *
>>> >> __ptr64,std::allocator<lld::coff::Chunk * __ptr64>
>>> >> >::_Insert<lld::coff::Chunk * __ptr64 const *
>>> >>
>>> >> __ptr64>(std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<lld::coff::Chunk
>>> >> *> > > _Where, lld::coff::Chunk * const * _First, lld::coff::Chunk *
>>> >> const *
>>> >> _Last, std::forward_iterator_tag __formal) Line 1421    C++
>>> >>      zig.exe!std::vector<lld::coff::Chunk *
>>> >> __ptr64,std::allocator<lld::coff::Chunk * __ptr64>
>>> >> >::insert<lld::coff::Chunk * __ptr64 const *
>>> >>
>>> >> __ptr64>(std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<lld::coff::Chunk
>>> >> *> > > _Where, lld::coff::Chunk * const * _First, lld::coff::Chunk *
>>> >> const *
>>> >> _Last) Line 1376    C++
>>> >>      zig.exe!lld::coff::SymbolTable::getChunks() Line 311    C++
>>> >>      zig.exe!`anonymous namespace'::Writer::createSections() Line 340
>>> >> C++
>>> >>      zig.exe!`anonymous namespace'::Writer::run() Line 288    C++
>>> >>      zig.exe!lld::coff::writeResult() Line 166    C++
>>> >>      zig.exe!lld::coff::LinkerDriver::link(llvm::ArrayRef<char const *>
>>> >> ArgsArr) Line 1331    C++
>>> >>      zig.exe!lld::coff::link(llvm::ArrayRef<char const *> Args, bool
>>> >> CanExitEarly, llvm::raw_ostream & Diag) Line 71    C++
>>> >>      zig.exe!ZigLLDLink(ZigLLVM_ObjectFormatType oformat, const char *
>>> >> *
>>> >> args, unsigned __int64 arg_count, void(*)(void *, const char *,
>>> >> unsigned
>>> >> __int64) append_diagnostic, void * context) Line 837    C++
>>> >>      zig.exe!zig_lld_link(ZigLLVM_ObjectFormatType oformat, const char
>>> >> * *
>>> >> args, unsigned __int64 arg_count, Buf * diag) Line 435    C++
>>> >>      zig.exe!codegen_link(CodeGen * g, const char * out_file) Line 1020
>>> >> C++
>>> >>      zig.exe!main(int argc, char * * argv) Line 909    C++
>>> >>      [External Code]
>>> >>
>>> >> It appears that in this code, ObjFile::Instance is garbage data:
>>> >>
>>> >> std::vector<Chunk *> SymbolTable::getChunks() {
>>> >>   std::vector<Chunk *> Res;
>>> >>   for (ObjFile *File : ObjFile::Instances) {
>>> >>     ArrayRef<Chunk *> V = File->getChunks();
>>> >>     Res.insert(Res.end(), V.begin(), V.end());
>>> >>   }
>>> >>   return Res;
>>> >> }
>>> >>
>>> >> When I go to the definition of ObjFile::Instances, it appears to be
>>> >> static
>>> >> data:
>>> >>
>>> >>   static std::vector<ObjFile *> Instances;
>>> >>
>>> >> It appears that LLD is not resetting this data between calls. On the
>>> >> other
>>> >> hand, lld::elf::link and lld::macho::link work no problem when called
>>> >> multiple times in the same process.
>>> >>
>>> >> My understanding is that there is supposed to be an arena allocator,
>>> >> which
>>> >> is freed here:
>>> >>
>>> >> bool link(ArrayRef<const char *> Args, bool CanExitEarly, raw_ostream
>>> >> &Diag) {
>>> >>   // ...
>>> >>
>>> >>   Driver = make<LinkerDriver>();
>>> >>   Driver->link(Args);
>>> >>
>>> >>   // Call exit() if we can to avoid calling destructors.
>>> >>   if (CanExitEarly)
>>> >>     exitLld(errorCount() ? 1 : 0);
>>> >>
>>> >>   freeArena();  // <------ here
>>> >>   return !errorCount();
>>> >> }
>>> >>
>>> >> Is there a simple fix for this?
>>> >>
>>> >> Downstream issue reference: https://github.com/ziglang/zig/issues/1289
>>> >>
>>> >> Thanks,
>>> >> Andrew
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > LLVM Developers mailing list
>>> > llvm-dev at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>> >