[PATCH] D114397: [lld-macho] Mark dylib symbols coming from -weak_framework as weak-def.

Tue Nov 23 19:05:44 PST 2021

oontvoo marked an inline comment as done.
oontvoo added inline comments.

================
Comment at: lld/MachO/SymbolTable.cpp:170
+      !wasForceWeak) {
+    replaceSymbol<DylibSymbol>(s, file, name, isWeakDef, refState, isTlv);
+  } else if (wasInserted || isa<Undefined>(s) ||
----------------
oontvoo wrote:
> thevinster wrote:
> > Hmm... I still don't understand why this change is needed. This code never gets run after the first framework gets loaded because it would short circuit and return the cached file (https://github.com/llvm/llvm-project/blob/main/lld/MachO/DriverUtils.cpp#L212). I think the `-weak_framework` is just meant to set the the `forceWeakImport` flag which is being accounted for in `Symbols.h`. 
> > 
> > I pulled the diff in, removed all the changes in `SymbolTable` and only kept `Symbols.h` changes, and the test still passed. 
> hmmm .... Didn't work for me . (or rather this was needed to fix the missing symbol crash at runtime )
> maybe I over simplified the test 🤔 
P.S : ignore that - PEBCAK 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114397