[PATCH] D112977: [lld-macho] Fix failed assertion in registerCompactUnwind

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

We can probably just extend weak-definition-gc.s instead of creating a new test. And we should verify that the associated contents are indeed deleted.

IMO once we cover that case, I don't think it's particularly important to test that the assert in `registerCompactUnwind` isn't fired. The "coalesced symbol => coalesced section contents" invariant is the more fundamental thing.

cc @thakis since he wrote the original weak symbol GC code



================
Comment at: lld/MachO/SymbolTable.cpp:69-74
+        defined->referencedDynamically |= isReferencedDynamically;
+        defined->noDeadStrip |= noDeadStrip;
+
+        if (auto concatIsec = dyn_cast_or_null<ConcatInputSection>(isec))
+          concatIsec->wasCoalesced = true;
         return defined;
----------------
why are we moving these outside the `if` block? we should be coalescing away the subsection associated with the weak symbol, but not the subsection associated with the non-weak symbol


================
Comment at: lld/MachO/SymbolTable.cpp:82
+      else
+        curIsec = defined->isec;
+
----------------
given that we only assign to `curIsec` in one place, we can just inline the block below here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112977