[PATCH] D81413: [lld-macho] improve handling of -platform_version

[Greg McGary via Phabricator via llvm-commits]

gkm added inline comments.


================
Comment at: lld/test/MachO/platform-version.s:10
+# RUN:        -platform_version \
+# RUN:    | FileCheck --check-prefix=FAIL_MISSING %s
+# RUN: not lld -flavor darwinnew -Z -o %t %t.o 2>&1 \
----------------
int3 wrote:
> nit: it's more common to use hyphens instead of underscores for check prefixes, i.e. `FAIL-MISSING`
> 
> Also, I was thinking it might make sense to split this test into two parts, with all the `FAIL-` checks going under `invalid/`, and the passing ones remaining here. Once we support `LC_BUILD_VERSION`, we'll probably want to expand the test for passing platform strings to include checks that the `LC_BUILD_VERSION` command contains the expected output.
For now, I would rather keep all tests in one place.


================
Comment at: lld/test/MachO/platform-version.s:41
+# RUN: lld -flavor darwinnew -Z -o %t %t.o 2>&1 \
+# RUN:        -platform_version ios-sim 1.2.3 5.6.7
+# RUN: lld -flavor darwinnew -Z -o %t %t.o 2>&1 \
----------------
int3 wrote:
> even though `man ld` indicates that `ios-sim` is a valid platform string, I couldn't get it to work on ld64 :/ it seems like `ios-simulator` is expected instead. (I tried that string out after finding it in rG25ce33a6e).
Indeed, I got the `*-sim` names from the `ld(1)` manual page in Xcode 11. Obviously it is wrong.

I looked at `ld64/src/ld/PlatformSupport.cpp` the newest ld64 source I have (v512), and found that `-platform_version` matches are case insensitive, with words separated by either dash or space. Here are the official names:
```
"unknown"
"macOS"
"iOS"
"tvOS"
"watchOS"
"bridgeOS"
"Mac Catalyst"
"iOS Simulator"
"tvOS Simulator"
"watchOS Simulator"
"DriverKit"
"free standing"
```
I cross-checked against the binary for Xcode 11.3 (v530) via strings(1). I update to make the matches case insensitive and to tolerate either space or dash for word separators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81413