<div dir="ltr">Nico fixed it in r352304.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 25, 2019 at 4:14 PM Nico Weber via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: nico<br>
Date: Fri Jan 25 16:14:52 2019<br>
New Revision: 352254<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=352254&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=352254&view=rev</a><br>
Log:<br>
lld-link: Store comdat selection in SectionChunk, reject more invalid associated comdats<br>
<br>
I need the comdat selection for PR40094. To keep the patch for that smaller,<br>
I'm adding it here, and as a first application I'm using it to reject<br>
associative comdats referring to earlier associative comdats. Depends on<br>
D56929; together with that all associative comdats referring to other<br>
associative comdats are now rejected.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D56931" rel="noreferrer" target="_blank">https://reviews.llvm.org/D56931</a><br>
<br>
Modified:<br>
    lld/trunk/COFF/Chunks.h<br>
    lld/trunk/COFF/InputFiles.cpp<br>
    lld/trunk/test/COFF/associative-comdat-order.test<br>
<br>
Modified: lld/trunk/COFF/Chunks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=352254&r1=352253&r2=352254&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=352254&r1=352253&r2=352254&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/Chunks.h (original)<br>
+++ lld/trunk/COFF/Chunks.h Fri Jan 25 16:14:52 2019<br>
@@ -223,6 +223,9 @@ public:<br>
   // The COMDAT leader symbol if this is a COMDAT chunk.<br>
   DefinedRegular *Sym = nullptr;<br>
<br>
+  // The COMDAT selection if this is a COMDAT chunk.<br>
+  llvm::COFF::COMDATType Selection;<br>
+<br>
   ArrayRef<coff_relocation> Relocs;<br>
<br>
   // Used by the garbage collector.<br>
<br>
Modified: lld/trunk/COFF/InputFiles.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=352254&r1=352253&r2=352254&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=352254&r1=352253&r2=352254&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/InputFiles.cpp (original)<br>
+++ lld/trunk/COFF/InputFiles.cpp Fri Jan 25 16:14:52 2019<br>
@@ -226,10 +226,7 @@ void ObjFile::readAssociativeDefinition(<br>
                                         uint32_t ParentIndex) {<br>
   SectionChunk *Parent = SparseChunks[ParentIndex];<br>
<br>
-  if (Parent == PendingComdat) {<br>
-    // This can happen if an associative comdat refers to another associative<br>
-    // comdat that appears after it (invalid per COFF spec) or to a section<br>
-    // without any symbols.<br>
+  auto Diag = [&]() {<br>
     StringRef Name, ParentName;<br>
     COFFObj->getSymbolName(Sym, Name);<br>
<br>
@@ -238,6 +235,13 @@ void ObjFile::readAssociativeDefinition(<br>
     COFFObj->getSectionName(ParentSec, ParentName);<br>
     error(toString(this) + ": associative comdat " + Name +<br>
           " has invalid reference to section " + ParentName);<br>
+  };<br>
+<br>
+  if (Parent == PendingComdat) {<br>
+    // This can happen if an associative comdat refers to another associative<br>
+    // comdat that appears after it (invalid per COFF spec) or to a section<br>
+    // without any symbols.<br>
+    Diag();<br>
     return;<br>
   }<br>
<br>
@@ -245,7 +249,14 @@ void ObjFile::readAssociativeDefinition(<br>
   // the section; otherwise mark it as discarded.<br>
   int32_t SectionNumber = Sym.getSectionNumber();<br>
   if (Parent) {<br>
-    SparseChunks[SectionNumber] = readSection(SectionNumber, Def, "");<br>
+    if (Parent->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE) {<br>
+      Diag();<br>
+      return;<br>
+    }<br>
+<br>
+    SectionChunk *C = readSection(SectionNumber, Def, "");<br>
+    C->Selection = IMAGE_COMDAT_SELECT_ASSOCIATIVE;<br>
+    SparseChunks[SectionNumber] = C;<br>
     if (SparseChunks[SectionNumber])<br>
       Parent->addAssociative(SparseChunks[SectionNumber]);<br>
   } else {<br>
<br>
Modified: lld/trunk/test/COFF/associative-comdat-order.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/associative-comdat-order.test?rev=352254&r1=352253&r2=352254&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/associative-comdat-order.test?rev=352254&r1=352253&r2=352254&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/test/COFF/associative-comdat-order.test (original)<br>
+++ lld/trunk/test/COFF/associative-comdat-order.test Fri Jan 25 16:14:52 2019<br>
@@ -1,10 +1,16 @@<br>
-# RUN: yaml2obj < %s > %t.obj<br>
-# RUN: not lld-link /include:symbol /dll /noentry /nodefaultlib %t.obj /out:%t.exe 2>&1 | FileCheck %s<br>
-<br>
 # Tests that an associative comdat being associated with another<br>
 # associated comdat later in the file produces an error.<br>
-# CHECK: lld-link: error: {{.*}}: associative comdat .text$ac1 has invalid reference to section .text$ac2<br>
-# CHECK-NOT: lld-link: error:<br>
+# RUN: sed -e s/ASSOC1/2/ -e s/ASSOC2/3/ %s | yaml2obj > %t.obj<br>
+# RUN: not lld-link /include:symbol /dll /noentry /nodefaultlib %t.obj /out:%t.exe 2>&1 | FileCheck --check-prefix=FORWARD %s<br>
+# FORWARD: lld-link: error: {{.*}}: associative comdat .text$ac1 has invalid reference to section .text$ac2<br>
+# FORWARD-NOT: lld-link: error:<br>
+<br>
+# Tests that an associative comdat being associated with another<br>
+# associated comdat earlier in the file produces an error.<br>
+# RUN: sed -e s/ASSOC1/3/ -e s/ASSOC2/1/ %s | yaml2obj > %t.obj<br>
+# RUN: not lld-link /include:symbol /dll /noentry /nodefaultlib %t.obj /out:%t.exe 2>&1 | FileCheck --check-prefix=BACKWARD %s<br>
+# BACKWARD: lld-link: error: {{.*}}: associative comdat .text$ac2 has invalid reference to section .text$ac1<br>
+# BACKWARD-NOT: lld-link: error:<br>
<br>
 --- !COFF<br>
 header:          <br>
@@ -35,7 +41,7 @@ symbols:<br>
       NumberOfRelocations: 0<br>
       NumberOfLinenumbers: 0<br>
       CheckSum:        3099354981<br>
-      Number:          2<br>
+      Number:          ASSOC1<br>
       Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE<br>
   - Name:            '.text$ac2'<br>
     Value:           0<br>
@@ -48,7 +54,7 @@ symbols:<br>
       NumberOfRelocations: 0<br>
       NumberOfLinenumbers: 0<br>
       CheckSum:        3099354981<br>
-      Number:          3<br>
+      Number:          ASSOC2<br>
       Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE<br>
   - Name:            '.text$nm'<br>
     Value:           0<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>