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

Tue Nov 2 19:23:09 PDT 2021

int3 added inline comments.

================
Comment at: lld/MachO/SymbolTable.cpp:71-72

+        defined->referencedDynamically |= isReferencedDynamically;
+        defined->noDeadStrip |= noDeadStrip;
+
----------------
oontvoo wrote:
> int3 wrote:
> > is this what ld64 does for these two attributes too? (can we test it?)
> No, this is NOT what LD64 does. (In fact this was one of the differences I was trying to note in the draft doc a couple weeks ago).
> LD64 picks one of the two (based on things like scope,  alignment, whether symbols are auto-hide ) and doesn't merge anything.
> I guess we could imitate that here ... didn't want to change that since this current approach seems to be working fine...
> 
> 
gotcha. Nonetheless, this is changing our behavior slightly, since we are now setting `referencedDynamically` and `noDeadStrip` when coalescing a weak with a non-weak, whereas before we would only do it when both symbols were weak. Can we keep the previous behavior for now?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112977