[PATCH] D93369: [lld-macho] Add support for weak references

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/SymbolTable.cpp:120
       (isa<DylibSymbol>(s) && !isWeakDef && s->isWeakDef()))
-    replaceSymbol<DylibSymbol>(s, file, name, isWeakDef, isTlv);
+    replaceSymbol<DylibSymbol>(s, file, name, isWeakDef, isWeakRef, isTlv);
 
----------------
Can it happen that a dylib symbol is added before the first undefined? If so, then the initial dylib has isWeakRef set to false, and it can then never become true. If I read Driver.cpp, this can happen if a dylib is passed before the .o referring to it is passed. (In my local patch I gave DylibSymbol an

```
+  enum WeakState { Unknown, SawWeakRefOnly, SawStrongRef,  };
+  WeakState weakRefState;
```

and a

```
+  void sawUndefined(bool weakRef) {
+    switch (weakRefState) {
+    case Unknown:
+      weakRefState = weakRef ? SawWeakRefOnly : SawStrongRef;
+      break;
+    case SawWeakRefOnly:
+      if (!weakRef)
+        weakRefState = SawStrongRef;
+      break;
+    case SawStrongRef:
+      break;
+    }
+  }
```

and made DylibSymbol start with `Unknown` and then in `addUndefined` did

```
+  else if (auto *dylib = dyn_cast<DylibSymbol>(s))
+    dylib->sawUndefined(isWeakRef);
```

I only sketched it locally and hadn't written tests yet so maybe this is overkill, but my read of Driver is that this is necessary. (It's noticeably more annoying to write than making one weak ref turn the whole symbol weak.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93369