[PATCH] D101855: [lld-macho] Check simulator platforms to avoid issuing false positive errors.

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:162-163
+  if (config->platform() != platformInfo->target.Platform &&
+      !checkSimulatedPlatform(config->platform(),
+                              platformInfo->target.Platform)) {
     error(toString(input) + " has platform " +
----------------
int3 wrote:
> int3 wrote:
> > how about having a function that just canonicalizes the platform name, removing the simulator part? i.e. something like
> > 
> > `removeSimulator(config->platform()) != removeSimulator(platformInfo->target.Platform)`
> > 
> > pros: the map will only need half the entries, plus it's a bit more symmetric
> > 
> > either is fine though
> another advantage of `removeSimulator()` is that we can have just one check here, i.e. no need for a separate `config->platform() != platformInfo->target.Platform`
> 
> With that in mind I think it's significantly nicer actually... could we do that? :)
done


================
Comment at: lld/test/MachO/invalid/incompatible-arch.s:45
+# RUN:	-o /dev/null 2>&1 | FileCheck %s --check-prefix=CROSS-SIM2
+# CROSS-SIM2: {{.*}}test_x86_ios_sim.o has platform iOS, which is different from target platform watchOS Simulator
+	
----------------
int3 wrote:
> oontvoo wrote:
> > int3 wrote:
> > > why doesn't this say "iOS simulator"?
> > It seems the "simulator" part was removed by llvm-mc. (Running `otool` on test_x86_ios_sim.oshows that it's only "IOS" and not IOS-simulator
> Ah, that seems to happen when the specified iOS version is old enough to require LC_MIN_VERSION instead of LC_BUILD_VERSION. Using a modern iOS version should preserve the 'simulator' part:
> 
> ```
> > echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-ios14.0-simulator -o test_x86_ios_sim.o
> > llvm-objdump --macho --all-headers test_x86_ios_sim.o
> ...
> Load command 1
>        cmd LC_BUILD_VERSION
>    cmdsize 24
>   platform iossimulator
>        sdk n/a
>      minos 14.0
>     ntools 0
> ```
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101855