Peter Collingbourne via llvm-dev
2018-Jun-07 21:52 UTC
[llvm-dev] [lld] ObjFile::createRegular is oblivious of PendingComdat
Can you upload a reproducer? You can create it using the /linkrepro flag. Peter On Thu, Jun 7, 2018 at 2:50 PM, Reid Kleckner via llvm-dev < llvm-dev at lists.llvm.org> wrote:> GCC does comdats completely differently from the spec. Since you contacted > me about this off of the mailing list, I started investigating what they > do, and it is completely different. It's basically an alternative comdat > ABI. I guess because GCC never considered it important to implement C++ ABI > compatibility, they just did their own thing with comdats, since they > aren't really part of the C ABI. > > Anyway, if we want to be C++ ABI compatible with GCC+binutils on Windows, > there will be a lot of work to do. It's not a small patch here or there, it > requires more research. We should discuss it in full. > > I support recovering from invalid COFF files to avoid crashes in any way > we can, though, so this patch might still be good. > > On Thu, Jun 7, 2018 at 2:44 PM Pirama Arumuga Nainar via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> I encountered a segfault when using lld to cross-compile for Windows >> (+MinGW) from Linux. The problem happens with objects built by gcc. The >> problem is that ObjFile::CreateRegular considers a PendingComdat to be >> valid (and later causes an illegal pointer dereference). The following >> patch fixes the crash: >> >> diff --git a/COFF/InputFiles.cpp b/COFF/InputFiles.cpp >> index 9e2345b0a..f47d612df 100644 >> --- a/COFF/InputFiles.cpp >> +++ b/COFF/InputFiles.cpp >> @@ -229,11 +229,11 @@ Symbol *ObjFile::createRegular(COFFSymbolRef Sym) { >> if (Sym.isExternal()) { >> StringRef Name; >> COFFObj->getSymbolName(Sym, Name); >> - if (SC) >> + if (SC && SC != PendingComdat) >> return Symtab->addRegular(this, Name, Sym.getGeneric(), SC); >> return Symtab->addUndefined(Name, this, false); >> } >> - if (SC) >> + if (SC && SC != PendingComdat) >> return make<DefinedRegular>(this, /*Name*/ "", false, >> /*IsExternal*/ false, Sym.getGeneric(), >> SC); >> return nullptr; >> >> I can upload this to Phabricator in case this is the right fix. If not, >> I'll upload a reproducer for further investigation. (I omitted it to avoid >> manually packaging a whole set of object files - I think >> --reproduce=<repro> doesn't work on Windows yet). >> >> For reference, here's the segfault: >> #0 0x0000000000500999 llvm::sys::PrintStackTrace(llvm::raw_ostream&) >> /ssd2/pirama/ll$ >> m-upstream/llvm/lib/Support/Unix/Signals.inc:488:11 >> >> #1 0x0000000000500b49 PrintStackTraceSignalHandler(void*) >> /ssd2/pirama/llvm-upstream$ >> llvm/lib/Support/Unix/Signals.inc:552:1 >> #2 0x00000000004ff1d6 llvm::sys::RunSignalHandlers() >> /ssd2/pirama/llvm-upstream/llvm$ >> lib/Support/Signals.cpp:66:5 >> #3 0x0000000000500eb7 SignalHandler(int) /ssd2/pirama/llvm-upstream/ >> llvm/lib/Support$ >> Unix/Signals.inc:351:1 >> #4 0x00007fc7bfaff0c0 __restore_rt (/lib/x86_64-linux-gnu/ >> libpthread.so.0+0x110c0) >> #5 0x00000000005fe370 lld::coff::DefinedRegular::getChunk() const >> /ssd2/pirama/llvm-$ >> pstream/llvm/tools/lld/COFF/Symbols.h:169:43 >> >> #6 0x00000000005fe082 lld::coff::Defined::getChunk() >> /ssd2/pirama/llvm-upstream/llvm$ >> tools/lld/COFF/Symbols.h:381:5 >> >> #7 0x0000000000656114 (anonymous namespace)::Writer:: >> createSymbol(lld::coff::Defined$ >> ) /ssd2/pirama/llvm-upstream/llvm/tools/lld/COFF/Writer.cpp:617:42 >> >> #8 0x000000000064c7df (anonymous namespace)::Writer::createSymbolAndStringTable() >> /s$ >> d2/pirama/llvm-upstream/llvm/tools/lld/COFF/Writer.cpp:0:43 >> >> #9 0x00000000006499f9 (anonymous namespace)::Writer::run() >> /ssd2/pirama/llvm-upstrea$ >> /llvm/tools/lld/COFF/Writer.cpp:342:3 >> >> #10 0x0000000000649843 lld::coff::writeResult() >> /ssd2/pirama/llvm-upstream/llvm/tool$ >> /lld/COFF/Writer.cpp:232:31 >> >> #11 0x00000000005d1268 lld::coff::LinkerDriver::link(llvm::ArrayRef<char >> const*>) /s$ >> d2/pirama/llvm-upstream/llvm/tools/lld/COFF/Driver.cpp:1536:3 >> #12 0x00000000005c8687 lld::coff::link(llvm::ArrayRef<char const*>, >> bool, llvm::raw_$ >> stream&) /ssd2/pirama/llvm-upstream/llvm/tools/lld/COFF/Driver.cpp:75:7 >> #13 0x0000000000946db0 lld::mingw::link(llvm::ArrayRef<char const*>, >> llvm::raw_ostrea >> m&) /ssd2/pirama/llvm-upstream/llvm/tools/lld/MinGW/Driver.cpp:255:10 >> #14 0x00000000004d02a8 main /ssd2/pirama/llvm-upstream/ >> llvm/tools/lld/tools/lld/lld.c >> pp:121:14 >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >-- -- Peter -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180607/8ece928e/attachment.html>
Pirama Arumuga Nainar via llvm-dev
2018-Jun-07 22:20 UTC
[llvm-dev] [lld] ObjFile::createRegular is oblivious of PendingComdat
Hi Reid, Thanks for the heads up. I'll try to compile everything with Clang + LLD. Peter, I am using a MinGW target, so Clang invokes ld.lld instead of lld-link. But the MinGW driver doesn't seem to support --reproduce. On Thu, Jun 7, 2018 at 2:52 PM Peter Collingbourne <peter at pcc.me.uk> wrote:> Can you upload a reproducer? You can create it using the /linkrepro flag. > > Peter > > On Thu, Jun 7, 2018 at 2:50 PM, Reid Kleckner via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> GCC does comdats completely differently from the spec. Since you >> contacted me about this off of the mailing list, I started investigating >> what they do, and it is completely different. It's basically an alternative >> comdat ABI. I guess because GCC never considered it important to implement >> C++ ABI compatibility, they just did their own thing with comdats, since >> they aren't really part of the C ABI. >> >> Anyway, if we want to be C++ ABI compatible with GCC+binutils on Windows, >> there will be a lot of work to do. It's not a small patch here or there, it >> requires more research. We should discuss it in full. >> >> I support recovering from invalid COFF files to avoid crashes in any way >> we can, though, so this patch might still be good. >> >> On Thu, Jun 7, 2018 at 2:44 PM Pirama Arumuga Nainar via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> I encountered a segfault when using lld to cross-compile for Windows >>> (+MinGW) from Linux. The problem happens with objects built by gcc. The >>> problem is that ObjFile::CreateRegular considers a PendingComdat to be >>> valid (and later causes an illegal pointer dereference). The following >>> patch fixes the crash: >>> >>> diff --git a/COFF/InputFiles.cpp b/COFF/InputFiles.cpp >>> index 9e2345b0a..f47d612df 100644 >>> --- a/COFF/InputFiles.cpp >>> +++ b/COFF/InputFiles.cpp >>> @@ -229,11 +229,11 @@ Symbol *ObjFile::createRegular(COFFSymbolRef Sym) { >>> if (Sym.isExternal()) { >>> StringRef Name; >>> COFFObj->getSymbolName(Sym, Name); >>> - if (SC) >>> + if (SC && SC != PendingComdat) >>> return Symtab->addRegular(this, Name, Sym.getGeneric(), SC); >>> return Symtab->addUndefined(Name, this, false); >>> } >>> - if (SC) >>> + if (SC && SC != PendingComdat) >>> return make<DefinedRegular>(this, /*Name*/ "", false, >>> /*IsExternal*/ false, Sym.getGeneric(), >>> SC); >>> return nullptr; >>> >>> I can upload this to Phabricator in case this is the right fix. If not, >>> I'll upload a reproducer for further investigation. (I omitted it to avoid >>> manually packaging a whole set of object files - I think >>> --reproduce=<repro> doesn't work on Windows yet). >>> >>> For reference, here's the segfault: >>> #0 0x0000000000500999 llvm::sys::PrintStackTrace(llvm::raw_ostream&) >>> /ssd2/pirama/ll$ >>> m-upstream/llvm/lib/Support/Unix/Signals.inc:488:11 >>> >>> #1 0x0000000000500b49 PrintStackTraceSignalHandler(void*) >>> /ssd2/pirama/llvm-upstream$ >>> llvm/lib/Support/Unix/Signals.inc:552:1 >>> #2 0x00000000004ff1d6 llvm::sys::RunSignalHandlers() >>> /ssd2/pirama/llvm-upstream/llvm$ >>> lib/Support/Signals.cpp:66:5 >>> #3 0x0000000000500eb7 SignalHandler(int) >>> /ssd2/pirama/llvm-upstream/llvm/lib/Support$ >>> Unix/Signals.inc:351:1 >>> #4 0x00007fc7bfaff0c0 __restore_rt >>> (/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0) >>> #5 0x00000000005fe370 lld::coff::DefinedRegular::getChunk() const >>> /ssd2/pirama/llvm-$ >>> pstream/llvm/tools/lld/COFF/Symbols.h:169:43 >>> >>> #6 0x00000000005fe082 lld::coff::Defined::getChunk() >>> /ssd2/pirama/llvm-upstream/llvm$ >>> tools/lld/COFF/Symbols.h:381:5 >>> >>> #7 0x0000000000656114 (anonymous >>> namespace)::Writer::createSymbol(lld::coff::Defined$ >>> ) /ssd2/pirama/llvm-upstream/llvm/tools/lld/COFF/Writer.cpp:617:42 >>> >>> #8 0x000000000064c7df (anonymous >>> namespace)::Writer::createSymbolAndStringTable() /s$ >>> d2/pirama/llvm-upstream/llvm/tools/lld/COFF/Writer.cpp:0:43 >>> >>> #9 0x00000000006499f9 (anonymous namespace)::Writer::run() >>> /ssd2/pirama/llvm-upstrea$ >>> /llvm/tools/lld/COFF/Writer.cpp:342:3 >>> >>> #10 0x0000000000649843 lld::coff::writeResult() >>> /ssd2/pirama/llvm-upstream/llvm/tool$ >>> /lld/COFF/Writer.cpp:232:31 >>> >>> #11 0x00000000005d1268 lld::coff::LinkerDriver::link(llvm::ArrayRef<char >>> const*>) /s$ >>> d2/pirama/llvm-upstream/llvm/tools/lld/COFF/Driver.cpp:1536:3 >>> #12 0x00000000005c8687 lld::coff::link(llvm::ArrayRef<char const*>, >>> bool, llvm::raw_$ >>> stream&) /ssd2/pirama/llvm-upstream/llvm/tools/lld/COFF/Driver.cpp:75:7 >>> #13 0x0000000000946db0 lld::mingw::link(llvm::ArrayRef<char const*>, >>> llvm::raw_ostrea >>> m&) /ssd2/pirama/llvm-upstream/llvm/tools/lld/MinGW/Driver.cpp:255:10 >>> #14 0x00000000004d02a8 main >>> /ssd2/pirama/llvm-upstream/llvm/tools/lld/tools/lld/lld.c >>> pp:121:14 >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> > > > -- > -- > Peter >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180607/d61005ac/attachment.html>
Pirama Arumuga Nainar via llvm-dev
2018-Jun-07 22:51 UTC
[llvm-dev] [lld] ObjFile::createRegular is oblivious of PendingComdat
https://reviews.llvm.org/F6327463 has a reproducer with all the necessary files. repro.sh has the command that fails. Thanks, -Pirama On Thu, Jun 7, 2018 at 3:20 PM Pirama Arumuga Nainar <pirama at google.com> wrote:> Hi Reid, Thanks for the heads up. I'll try to compile everything with > Clang + LLD. > > Peter, I am using a MinGW target, so Clang invokes ld.lld instead of > lld-link. But the MinGW driver doesn't seem to support --reproduce. > > On Thu, Jun 7, 2018 at 2:52 PM Peter Collingbourne <peter at pcc.me.uk> > wrote: > >> Can you upload a reproducer? You can create it using the /linkrepro flag. >> >> Peter >> >> On Thu, Jun 7, 2018 at 2:50 PM, Reid Kleckner via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> GCC does comdats completely differently from the spec. Since you >>> contacted me about this off of the mailing list, I started investigating >>> what they do, and it is completely different. It's basically an alternative >>> comdat ABI. I guess because GCC never considered it important to implement >>> C++ ABI compatibility, they just did their own thing with comdats, since >>> they aren't really part of the C ABI. >>> >>> Anyway, if we want to be C++ ABI compatible with GCC+binutils on >>> Windows, there will be a lot of work to do. It's not a small patch here or >>> there, it requires more research. We should discuss it in full. >>> >>> I support recovering from invalid COFF files to avoid crashes in any way >>> we can, though, so this patch might still be good. >>> >>> On Thu, Jun 7, 2018 at 2:44 PM Pirama Arumuga Nainar via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>>> I encountered a segfault when using lld to cross-compile for Windows >>>> (+MinGW) from Linux. The problem happens with objects built by gcc. The >>>> problem is that ObjFile::CreateRegular considers a PendingComdat to be >>>> valid (and later causes an illegal pointer dereference). The following >>>> patch fixes the crash: >>>> >>>> diff --git a/COFF/InputFiles.cpp b/COFF/InputFiles.cpp >>>> index 9e2345b0a..f47d612df 100644 >>>> --- a/COFF/InputFiles.cpp >>>> +++ b/COFF/InputFiles.cpp >>>> @@ -229,11 +229,11 @@ Symbol *ObjFile::createRegular(COFFSymbolRef Sym) >>>> { >>>> if (Sym.isExternal()) { >>>> StringRef Name; >>>> COFFObj->getSymbolName(Sym, Name); >>>> - if (SC) >>>> + if (SC && SC != PendingComdat) >>>> return Symtab->addRegular(this, Name, Sym.getGeneric(), SC); >>>> return Symtab->addUndefined(Name, this, false); >>>> } >>>> - if (SC) >>>> + if (SC && SC != PendingComdat) >>>> return make<DefinedRegular>(this, /*Name*/ "", false, >>>> /*IsExternal*/ false, >>>> Sym.getGeneric(), SC); >>>> return nullptr; >>>> >>>> I can upload this to Phabricator in case this is the right fix. If >>>> not, I'll upload a reproducer for further investigation. (I omitted it to >>>> avoid manually packaging a whole set of object files - I think >>>> --reproduce=<repro> doesn't work on Windows yet). >>>> >>>> For reference, here's the segfault: >>>> #0 0x0000000000500999 llvm::sys::PrintStackTrace(llvm::raw_ostream&) >>>> /ssd2/pirama/ll$ >>>> m-upstream/llvm/lib/Support/Unix/Signals.inc:488:11 >>>> >>>> #1 0x0000000000500b49 PrintStackTraceSignalHandler(void*) >>>> /ssd2/pirama/llvm-upstream$ >>>> llvm/lib/Support/Unix/Signals.inc:552:1 >>>> #2 0x00000000004ff1d6 llvm::sys::RunSignalHandlers() >>>> /ssd2/pirama/llvm-upstream/llvm$ >>>> lib/Support/Signals.cpp:66:5 >>>> #3 0x0000000000500eb7 SignalHandler(int) >>>> /ssd2/pirama/llvm-upstream/llvm/lib/Support$ >>>> Unix/Signals.inc:351:1 >>>> #4 0x00007fc7bfaff0c0 __restore_rt >>>> (/lib/x86_64-linux-gnu/libpthread.so.0+0x110c0) >>>> #5 0x00000000005fe370 lld::coff::DefinedRegular::getChunk() const >>>> /ssd2/pirama/llvm-$ >>>> pstream/llvm/tools/lld/COFF/Symbols.h:169:43 >>>> >>>> #6 0x00000000005fe082 lld::coff::Defined::getChunk() >>>> /ssd2/pirama/llvm-upstream/llvm$ >>>> tools/lld/COFF/Symbols.h:381:5 >>>> >>>> #7 0x0000000000656114 (anonymous >>>> namespace)::Writer::createSymbol(lld::coff::Defined$ >>>> ) /ssd2/pirama/llvm-upstream/llvm/tools/lld/COFF/Writer.cpp:617:42 >>>> >>>> #8 0x000000000064c7df (anonymous >>>> namespace)::Writer::createSymbolAndStringTable() /s$ >>>> d2/pirama/llvm-upstream/llvm/tools/lld/COFF/Writer.cpp:0:43 >>>> >>>> #9 0x00000000006499f9 (anonymous namespace)::Writer::run() >>>> /ssd2/pirama/llvm-upstrea$ >>>> /llvm/tools/lld/COFF/Writer.cpp:342:3 >>>> >>>> #10 0x0000000000649843 lld::coff::writeResult() >>>> /ssd2/pirama/llvm-upstream/llvm/tool$ >>>> /lld/COFF/Writer.cpp:232:31 >>>> >>>> #11 0x00000000005d1268 >>>> lld::coff::LinkerDriver::link(llvm::ArrayRef<char const*>) /s$ >>>> d2/pirama/llvm-upstream/llvm/tools/lld/COFF/Driver.cpp:1536:3 >>>> #12 0x00000000005c8687 lld::coff::link(llvm::ArrayRef<char const*>, >>>> bool, llvm::raw_$ >>>> stream&) /ssd2/pirama/llvm-upstream/llvm/tools/lld/COFF/Driver.cpp:75:7 >>>> #13 0x0000000000946db0 lld::mingw::link(llvm::ArrayRef<char const*>, >>>> llvm::raw_ostrea >>>> m&) /ssd2/pirama/llvm-upstream/llvm/tools/lld/MinGW/Driver.cpp:255:10 >>>> #14 0x00000000004d02a8 main >>>> /ssd2/pirama/llvm-upstream/llvm/tools/lld/tools/lld/lld.c >>>> pp:121:14 >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> >> >> >> -- >> -- >> Peter >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180607/f9753637/attachment.html>