llvm dev - Jun 2018 - [lld] ObjFile::createRegular is oblivious of PendingComdat

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180607/645f3677/attachment.html>

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180607/e83b9cd3/attachment-0001.html>

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>