[PATCH] D113167: [lld-macho]Allow exporting weak_def_can_be_hidden(AKA "autohide") symbols

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1478-1484
+              else
+                // weak_def_can_be_hidden symbols behaves similarly to
+                // private_extern symbols in most cases, except for when
+                // it is explicitly exported.
+                // LD64 allows exporting the former, but not the latter.
+                // So we do the same here.
+                defined->privateExtern = false;
----------------
oontvoo wrote:
> int3 wrote:
> > This is basically undoing the promotion work in InputFiles.cpp, right? Would it be simpler if we didn't attempt that promotion, and just treated these two booleans as independent values?
> > 
> > Also, what happens when a symbol is marked both autohide and private_extern?
> Well, it's a bit more convoluted than that (sorry!)
> It's only undoing the promotion IF the symbol is in the exported list. (if not, it needs to stay privateExtern so we dont put it in the dynamic table).
> However, if a symbol is both autohide and private-extern, then the autohide doesn't mean anything - it can't be exported either. (added the test)
> 
> We can keep the two flags independent in theory, but that means I'd have to go fix up all the places where privateExtern is treated to also take into account this extra flag, which (aside from being a bit of work) is unnecessary because as mentioned the symbol acts exactly like a privateExtern, only difference is that it can be exported.
I was going to say that I guess it's only a bit awkward but then I read the new code in InputFiles.cpp and I changed my mind :P

I think it's super confusing: for one, `isWeakDefCanBeHidden == true` means that a privateExtern symbol can be *un*hidden. Also, to prevent that un-hiding from happening when both are set, we flip `isWeakDefCanBeHidden` inside `createDefined` because it is only in that function that we know that `privateExtern` *really* means `.private_extern`. I.e. it has different semantics within that function vs everywhere else.


================
Comment at: lld/MachO/InputFiles.cpp:577-582
+    if (isWeakDefCanBeHidden) {
+      if (isPrivateExtern)
+        isWeakDefCanBeHidden = false;
+      else
+        isPrivateExtern = true;
+    }
----------------
what happens if there is an existing symbol that's `isWeakDefCanBeHidden` and another one that's `isPrivateExtern`? I.e. what if those two properties are defined on different symbols with the same name?

(should also add a test that covers this)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113167