This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit
ClosedPublic

Authored by ahatanak on Jun 22 2017, 1:21 PM.

Details

Summary

The following changes are made to the driver since 32-bit apps do not run on iOS 11 or later:

  • If the deployment target is set explicitly, either with a command-line option or an environment variable, the driver should report an error if the version is greater than iOS 10.
  • In the case where the deployment target is not set explicitly and the default is inferred from the target triple or SDK version, it should use a maximum default of iOS 10.

rdar://problem/32230613

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak created this revision.Jun 22 2017, 1:21 PM
bob.wilson edited edge metadata.Jun 29 2017, 10:25 PM

The proposed error message does not provide any information about why the version is invalid. That could be confusing. Your comment in the code is more clear: "iOS 10 is the maximum deployment target for 32-bit targets". Can you say something like that in the error message?

In the case where the version is inferred from the SDK, you're resetting Major=10 but leaving Minor and Micro unchanged. That seems wrong. Those should be set to the most recent release of iOS 10. Perhaps you could set those to big numbers, e.g., 99, so that they are sure to include any future iOS 10 releases.

ahatanak updated this revision to Diff 104906.Jun 30 2017, 12:22 PM

Address review comments.

  • Change the error message from "invalid iOS deployment version '%0', maximum allowable version is iOS 10" to "invalid iOS deployment version '%0', iOS 10 is the maximum deployment target for 32-bit targets"
  • If the inferred version is iOS 11 or later, reset it to 10.99.
bob.wilson added inline comments.Jun 30 2017, 1:09 PM
lib/Driver/ToolChains/Darwin.cpp
1350 ↗(On Diff #104906)

What about Micro = 99?

ahatanak updated this revision to Diff 104958.Jun 30 2017, 4:33 PM
ahatanak marked an inline comment as done.

Set Micro to 99 too.

This revision is now accepted and ready to land.Jun 30 2017, 4:45 PM
This revision was automatically updated to reflect the committed changes.