[lld] r352254 - lld-link: Store comdat selection in SectionChunk, reject more invalid associated comdats

Thu Jan 31 07:35:52 PST 2019

Sorry, I missed this mail, but I had fixed this a few days ago in 352304.

On Mon, Jan 28, 2019 at 2:09 PM Kostya Serebryany <kcc at google.com> wrote:

> msan bots are unhappy with this code.
> Nico or Rui, could you please check?
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/28760/steps/check-lld%20msan/logs/stdio
>
> lld-link: Reading /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/tools/lld/test/COFF/Output/associative-comdat-mingw.s.tmp1.obj
> ==62002==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0x7a0681 in lld::coff::ObjFile::readAssociativeDefinition(llvm::object::COFFSymbolRef, llvm::object::coff_aux_section_definition const*, unsigned int) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/lld/COFF/InputFiles.cpp:252:9
>     #1 0x7a12de in lld::coff::ObjFile::maybeAssociateSEHForMingw(llvm::object::COFFSymbolRef, llvm::object::coff_aux_section_definition const*, llvm::DenseMap<llvm::StringRef, unsigned int, llvm::DenseMapInfo<llvm::StringRef>, llvm::detail::DenseMapPair<llvm::StringRef, unsigned int> > const&) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/lld/COFF/InputFiles.cpp:292:7
>     #2 0x79da8c in lld::coff::ObjFile::initializeSymbols() /b/sanitizer-x86_64-linux-fast/build/llvm/tools/lld/COFF/InputFiles.cpp:364:9
>     #3 0x79ae9b in lld::coff::ObjFile::parse() /b/sanitizer-x86_64-linux-fast/build/llvm/tools/lld/COFF/InputFiles.cpp:126:3
>
>
> On Fri, Jan 25, 2019 at 4:14 PM Nico Weber via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: nico
>> Date: Fri Jan 25 16:14:52 2019
>> New Revision: 352254
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=352254&view=rev
>> Log:
>> lld-link: Store comdat selection in SectionChunk, reject more invalid
>> associated comdats
>>
>> I need the comdat selection for PR40094. To keep the patch for that
>> smaller,
>> I'm adding it here, and as a first application I'm using it to reject
>> associative comdats referring to earlier associative comdats. Depends on
>> D56929; together with that all associative comdats referring to other
>> associative comdats are now rejected.
>>
>> Differential Revision: https://reviews.llvm.org/D56931
>>
>> Modified:
>>     lld/trunk/COFF/Chunks.h
>>     lld/trunk/COFF/InputFiles.cpp
>>     lld/trunk/test/COFF/associative-comdat-order.test
>>
>> Modified: lld/trunk/COFF/Chunks.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=352254&r1=352253&r2=352254&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/COFF/Chunks.h (original)
>> +++ lld/trunk/COFF/Chunks.h Fri Jan 25 16:14:52 2019
>> @@ -223,6 +223,9 @@ public:
>>    // The COMDAT leader symbol if this is a COMDAT chunk.
>>    DefinedRegular *Sym = nullptr;
>>
>> +  // The COMDAT selection if this is a COMDAT chunk.
>> +  llvm::COFF::COMDATType Selection;
>> +
>>    ArrayRef<coff_relocation> Relocs;
>>
>>    // Used by the garbage collector.
>>
>> Modified: lld/trunk/COFF/InputFiles.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=352254&r1=352253&r2=352254&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/COFF/InputFiles.cpp (original)
>> +++ lld/trunk/COFF/InputFiles.cpp Fri Jan 25 16:14:52 2019
>> @@ -226,10 +226,7 @@ void ObjFile::readAssociativeDefinition(
>>                                          uint32_t ParentIndex) {
>>    SectionChunk *Parent = SparseChunks[ParentIndex];
>>
>> -  if (Parent == PendingComdat) {
>> -    // This can happen if an associative comdat refers to another
>> associative
>> -    // comdat that appears after it (invalid per COFF spec) or to a
>> section
>> -    // without any symbols.
>> +  auto Diag = [&]() {
>>      StringRef Name, ParentName;
>>      COFFObj->getSymbolName(Sym, Name);
>>
>> @@ -238,6 +235,13 @@ void ObjFile::readAssociativeDefinition(
>>      COFFObj->getSectionName(ParentSec, ParentName);
>>      error(toString(this) + ": associative comdat " + Name +
>>            " has invalid reference to section " + ParentName);
>> +  };
>> +
>> +  if (Parent == PendingComdat) {
>> +    // This can happen if an associative comdat refers to another
>> associative
>> +    // comdat that appears after it (invalid per COFF spec) or to a
>> section
>> +    // without any symbols.
>> +    Diag();
>>      return;
>>    }
>>
>> @@ -245,7 +249,14 @@ void ObjFile::readAssociativeDefinition(
>>    // the section; otherwise mark it as discarded.
>>    int32_t SectionNumber = Sym.getSectionNumber();
>>    if (Parent) {
>> -    SparseChunks[SectionNumber] = readSection(SectionNumber, Def, "");
>> +    if (Parent->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE) {
>> +      Diag();
>> +      return;
>> +    }
>> +
>> +    SectionChunk *C = readSection(SectionNumber, Def, "");
>> +    C->Selection = IMAGE_COMDAT_SELECT_ASSOCIATIVE;
>> +    SparseChunks[SectionNumber] = C;
>>      if (SparseChunks[SectionNumber])
>>        Parent->addAssociative(SparseChunks[SectionNumber]);
>>    } else {
>>
>> Modified: lld/trunk/test/COFF/associative-comdat-order.test
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/associative-comdat-order.test?rev=352254&r1=352253&r2=352254&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/test/COFF/associative-comdat-order.test (original)
>> +++ lld/trunk/test/COFF/associative-comdat-order.test Fri Jan 25 16:14:52
>> 2019
>> @@ -1,10 +1,16 @@
>> -# RUN: yaml2obj < %s > %t.obj
>> -# RUN: not lld-link /include:symbol /dll /noentry /nodefaultlib %t.obj
>> /out:%t.exe 2>&1 | FileCheck %s
>> -
>>  # Tests that an associative comdat being associated with another
>>  # associated comdat later in the file produces an error.
>> -# CHECK: lld-link: error: {{.*}}: associative comdat .text$ac1 has
>> invalid reference to section .text$ac2
>> -# CHECK-NOT: lld-link: error:
>> +# RUN: sed -e s/ASSOC1/2/ -e s/ASSOC2/3/ %s | yaml2obj > %t.obj
>> +# RUN: not lld-link /include:symbol /dll /noentry /nodefaultlib %t.obj
>> /out:%t.exe 2>&1 | FileCheck --check-prefix=FORWARD %s
>> +# FORWARD: lld-link: error: {{.*}}: associative comdat .text$ac1 has
>> invalid reference to section .text$ac2
>> +# FORWARD-NOT: lld-link: error:
>> +
>> +# Tests that an associative comdat being associated with another
>> +# associated comdat earlier in the file produces an error.
>> +# RUN: sed -e s/ASSOC1/3/ -e s/ASSOC2/1/ %s | yaml2obj > %t.obj
>> +# RUN: not lld-link /include:symbol /dll /noentry /nodefaultlib %t.obj
>> /out:%t.exe 2>&1 | FileCheck --check-prefix=BACKWARD %s
>> +# BACKWARD: lld-link: error: {{.*}}: associative comdat .text$ac2 has
>> invalid reference to section .text$ac1
>> +# BACKWARD-NOT: lld-link: error:
>>
>>  --- !COFF
>>  header:
>> @@ -35,7 +41,7 @@ symbols:
>>        NumberOfRelocations: 0
>>        NumberOfLinenumbers: 0
>>        CheckSum:        3099354981
>> -      Number:          2
>> +      Number:          ASSOC1
>>        Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE
>>    - Name:            '.text$ac2'
>>      Value:           0
>> @@ -48,7 +54,7 @@ symbols:
>>        NumberOfRelocations: 0
>>        NumberOfLinenumbers: 0
>>        CheckSum:        3099354981
>> -      Number:          3
>> +      Number:          ASSOC2
>>        Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE
>>    - Name:            '.text$nm'
>>      Value:           0
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190131/47027423/attachment.html>