[PATCH] D130725: [lld/mac] Add support for $ld$previous symbols with explicit symbol name

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:2077
+
+    // Just adding the symbol to the symtab works because dylibs contain their
+    // symbols in alphabetical order, guaranteeing $ld$ symbols to precede
----------------
thakis wrote:
> int3 wrote:
> > what do you mean by "works" here? what's the failure case that we have avoided?
> The tbd file usually contains the `$ld$previous` symbol for an old version, and then the symbol itself later, for newer deployment targets, like so:
> 
> ```
>     symbols:         [
>       '$ld$previous$/Another$$1$3.0$14.0$_zzz$',
>       _zzz,
>    ]
> ```
> 
> If they weren't sorted, then `_zzz` would be added with the current dylib first (and since it's added first, it would win over the `$ld$previous` symbol.
> 
> Suggestions on how to reword the comment?
How about

> The tbd file usually contains the $ld$previous symbol for an old version, and then the symbol itself later, for newer deployment targets, like so:
> <example>
> Since the symbols are sorted, adding them to the symtab in the given order means the `$ld$previous` version of `_zzz` will prevail, as desired.


================
Comment at: lld/test/MachO/special-symbol-ld-previous.s:33-34
+## Case 4: special symbol $ld$previous affects the install name / compatibility version
+## when the specified version 11.0.0 is within the affected range [3.0, 14.0) when a symbol
+## is part of $previous$ if and only if that symbol is referenced.
+
----------------
thakis wrote:
> int3 wrote:
> > > when a symbol is part of $previous$ if and only if that symbol is referenced.
> > 
> > kind of confused by this. Did you mean "only when the named symbol is referenced"?
> Yes, that's what I mean. Reworded it a bit.
I think it's confusing because it's a bit of a run-on sentence. How about
```
## Case 4: special symbol $ld$previous affects the install name / compatibility version if and only if:
##   * the specified version 11.0.0 is within the affected range [3.0, 14.0), and
##   * the symbol name after $previous$ points to a referenced symbol
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130725