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

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/COFF/InputFiles.cpp:258
+    SectionChunk *C = readSection(SectionNumber, Def, "");
+    C->Selection = IMAGE_COMDAT_SELECT_ASSOCIATIVE;
+    SparseChunks[SectionNumber] = C;
----------------
pcc wrote:
> Maybe move this inside `readSection` and do `C->Selection = Def->Selection;`?
That's a good suggestion, but Def->Selection is an uint8_t while C->Selection is a COMDATType.

I could make readSection() do validation and error out if Def->Selection is something unknown, but in the upcoming patch for better Selection handling I need to do this validation long before I call readSection(), so I'd have to do it in two places then. So I figured setting it here is maybe easier?


================
Comment at: lld/test/COFF/associative-comdat-order.s:1
-# 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.
----------------
pcc wrote:
> (Sorry didn't notice this on your other patch)
> 
> This isn't an assembly file, so the extension should be `.test`.
Renamed in 352074.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56931/new/

https://reviews.llvm.org/D56931