-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement W^X-compatible JIT. Hopefully makes JIT work on iOS again #8937
Conversation
…iteProtect on codeblocks (to be redesigned)
…y iOS (untested). Disabled block linking in this mode, can re-enable with some more work later. To enable W^X on other platforms than iOS, simply change PlatformIsWXExclusive.
@hrydgard You forgot this line in main.mm, otherwise Xcode throws an error message: |
PanicAlert("Bad memory protect : W^X is in effect, can't both write and exec"); | ||
} | ||
// Note - both VirtualProtect and mprotect will affect the full pages containing the requested range, | ||
// so no need to round ptr and size down/up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's confusing to say this and then round anyway (which we're assuming is necessary on iOS?)
I wonder if this helps anything on the Galaxy S7 (if the other change is turned off.)
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First I thought it was, but turned out it wasn't, or at least it worked after I added the rounding...
@hrydgard That looks pretty good, though I cannot test right now.
|
@vit9696 The AlignCode16 are not really relevant for this pull request, but there is a recommendation to start functions at 16-byte boundaries on some AMD CPUs especially so why not. OK, I'll fix the comment and change to PAGE_SIZE. |
return 4096; | ||
// For reference, here's how to check: | ||
// if (sys_info.dwPageSize == 0) | ||
// GetSystemInfo(&sys_info); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, we're doing that already:
Line 43 in 1e4b77f
#define MEM_PAGE_MASK (uintptr_t)(sys_info.dwPageSize - 1) |
-[Unknown]
ow, @unknownbrackets are you sure this builds on iOS? Currently cloning and will check but I fear that not all the headers are included still. |
@vit9696 let's just fix it if there are problems. I'll be around for a few hours so just let me know |
|
I guess this is the right fix :) |
Pushed a fix |
Thanks =) |
@hrydgard I can't really read code that well so bear with me: if (( iOS < 9 && isJailbroken() ) || ( iOS == 9 && isJailbroken() && arch == armv7 )) { canUseJIT = true } How do we test if the device is jailbroken? There are several methods for that, did you find a reliable one? Also should we include a Also look at this PDF from the Black Hat 2016. Will the new security changes affect our approach in iOS 10? Changes to JSCore (Safari)
Thanks for listening. |
I don't think the W^X compatibility slowed it down much, so for safety and simplicity, we now always use W^X compatible JIT on iOS, even if we could do it the old style. Therefore, the check is now simply:
as W^X should work on all iOS devices. We'll see about iOS 10. About the BlackHat thing, we give ourselves debugger permissions so we don't need to concern ourselves with that. App Store apps can't do that but we're not on the app store. Including a W^X JIT option on other platforms doesn't make much sense IMO - while it's useful for developers for debugging, you can just change a single line of code to get into that mode anyway. |
@hrydgard Thanks for answering my questions! Do you have actual numbers on how the W^X JIT compares to the old one? I would really like to help and test it for you (iPhone 6 - 8.4) but I don't have access to a computer. I know this may be a lot to ask but if you could provide me with a compiled PPSSPP or even write a quick way that let's me choose both cores, and push it to the Cydia buildbot, I could quickly compare some games and tell you if the old JIT was even worth it (on iOS). But on the other hand, I'm pretty sure you're going to find a better tester than myself 😁 |
No numbers, but there should only be a difference when it's compiling new code, which mostly happens when you enter new levels or do new things. We have to do some extra memory operations to un-write-protect and then write protect the generated code in this case. I'd expect possibly very small stutters when you enter a new level and then no difference to the old JIT. |
Just to say, the 3GS couldn't even run most easy/simple games at full speed with frameskip iirc. Also, last I checked my iOS version was too low to run PPSSPP anymore, but not a huge loss. The performance impact is likely to vary by operating system. hrydgard could test on Windows, let's say, or Android, but it might not reflect the performance impact on iOS. So someone with an iOS device that doesn't need W^X would likely need to perform the tests, but this would likely require writing code to get a good sense of the compilation performance impact (since runtime won't be affected, and loading times are harder to measure.) -[Unknown] |
- Only required for 64bit OSs, it's expensive - hrydgard/ppsspp#8937
- Only required for 64bit OSs, it's expensive - hrydgard/ppsspp#8937
- Only required for 64bit OSs, it's expensive - hrydgard/ppsspp#8937
- Only required for 64bit OSs, it's expensive - hrydgard/ppsspp#8937
- Required on non-JB iOS and jailbroken 64bit iOS - hrydgard/ppsspp#8937
- Required on non-JB iOS and jailbroken 64bit iOS - hrydgard/ppsspp#8937
( Sorry for the GitHub noise regarding commit references... We've finally managed to implement W^X JIT on non-JB iOS as well on our project :) ) |
That's no problem, that kind of commit reference noise doesn't send me notifications :) Nice job. |
Will hopefully fix issue #8122, with a lot less of a speed hit than @vit9696's patch (thanks for that!)
For now, block linking is disabled in this mode. Would like confirmation that this works on iOS before doing the work to enable it.
To play around with this mode on other platforms, simply change PlatformIsWXExclusive in MemoryUtil.cpp to always return true. Speed hit appears to be minimal, disregarding the loss of block linking.
Please let me know if this works or not in the comments. I've enabled the mode and tested on Android and Windows, and those are fine.