[lld] [LLD] [COFF] Fix handling of comdat .drectve sections (PR #68116)

[Martin Storsjö via llvm-commits]

================
@@ -660,10 +660,15 @@ std::optional<Symbol *> ObjFile::createDefined(
 
     if (prevailing) {
       SectionChunk *c = readSection(sectionNumber, def, getName());
-      sparseChunks[sectionNumber] = c;
-      c->sym = cast<DefinedRegular>(leader);
-      c->selection = selection;
-      cast<DefinedRegular>(leader)->data = &c->repl;
+      if (c) {
+        sparseChunks[sectionNumber] = c;
+        c->sym = cast<DefinedRegular>(leader);
+        c->selection = selection;
+        cast<DefinedRegular>(leader)->data = &c->repl;
+      } else {
+        sparseChunks[sectionNumber] = nullptr;
+        return nullptr;
+      }
     } else {
       sparseChunks[sectionNumber] = nullptr;
----------------
mstorsjo wrote:

I guess @aganea's suggestion of just moving the `if (!c) return nullptr;` after assigning `sparseChunks[sectionNumber] = c;` would simplify it a bit as well.

We can't return `leader` (I tested that and spent an awful lot of time hunting down why things were screwed up); the `cast<DefinedRegular>(leader)->data = &c->repl;` assignment is essential - otherwise the `leader` symbol is left with a nullptr `data`, which is expected to be initialized before the end.

As we're returning differently (`leader` vs `nullptr`) in the two cases where we're assigning `nullptr` into `sparseChunks[sectionNumber]`, I don't think we very neatly can fold all the three cases together, but @aganea's suggestion might make it more palatable.

https://github.com/llvm/llvm-project/pull/68116