Fix Symbol not found: (_JSGlobalContextSetInspectable) #44185
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D56425834 |
Summary: This change will fix a symbol not found for JSC Runtime. The `if` check was not a compile time check, therefore the symbol ended up in the binary even if it is not available. Following this post on [Apple forum](https://forums.developer.apple.com/forums/thread/749534), this changes should do the trick. ## Changelog [iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable) Differential Revision: D56425834
cipolleschi
force-pushed
the
export-D56425834
branch
from
fe44f60 to
5fb18fb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D56425834 |
Base commit: f453d7a |
|
This pull request has been merged in 224ac21. |
|
This pull request was successfully merged by @cipolleschi in 224ac21. When will my fix make it into a release? | How to file a pick request? |
Saadnajmi
reviewed
Apr 23, 2024
Comment on lines
379
to
+380
| #ifdef _JSC_HAS_INSPECTABLE | ||
| #if (__OSX_AVAILABLE_STARTING(MAC_NA, IPHONE_16_4)) |
There was a problem hiding this comment.
@cipolleschi Original author of _JSC_HAS_INSPECTABLE, should we move this check to the macros' definition (AKA, line 303)? I can do so in my already open PR where I refactor some of it #39549
There was a problem hiding this comment.
Hi @Saadnajmi, yes, we can. I just saw we had a check on the iOS version at line 303, and I think that that's what triggered the issue... The version is set to 160400, which is... 160.4!!!
I think it should be 16400 if we want to specify 16.4.
- #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 160400
+ #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 16400for example, the check for iOS 17, is 17000.
There was a problem hiding this comment.
Damn. I remember double checking that. Ok, I guess we don't need the double check then, and I'll fix the versions.
There was a problem hiding this comment.
Also closing this thread so we can keep discussion in #39549 (comment) , but as noted there, I think the existing macro is correct.
EDIT: I can't actually resolve this thread.. so.. just go comment on the other one :D
hurali97
pushed a commit
that referenced
this pull request
Apr 29, 2024
Summary: Pull Request resolved: #44185 This change will fix a symbol not found for JSC Runtime. The `if` check was not a compile time check, therefore the symbol ended up in the binary even if it is not available. Following this post on [Apple forum](https://forums.developer.apple.com/forums/thread/749534), this changes should do the trick. ## Changelog [iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable) Reviewed By: hash3r Differential Revision: D56425834 fbshipit-source-id: a37af51b078bd47a938e6b65d9d8e0f7506e746f
Titozzz
pushed a commit
that referenced
this pull request
Apr 29, 2024
Summary: Pull Request resolved: #44185 This change will fix a symbol not found for JSC Runtime. The `if` check was not a compile time check, therefore the symbol ended up in the binary even if it is not available. Following this post on [Apple forum](https://forums.developer.apple.com/forums/thread/749534), this changes should do the trick. ## Changelog [iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable) Reviewed By: hash3r Differential Revision: D56425834 fbshipit-source-id: a37af51b078bd47a938e6b65d9d8e0f7506e746f
cipolleschi
added a commit
that referenced
this pull request
May 1, 2024
Summary: Pull Request resolved: #44185 This change will fix a symbol not found for JSC Runtime. The `if` check was not a compile time check, therefore the symbol ended up in the binary even if it is not available. Following this post on [Apple forum](https://forums.developer.apple.com/forums/thread/749534), this changes should do the trick. ## Changelog [iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable) Reviewed By: hash3r Differential Revision: D56425834 fbshipit-source-id: a37af51b078bd47a938e6b65d9d8e0f7506e746f
kosmydel
pushed a commit
to kosmydel/react-native
that referenced
this pull request
May 6, 2024
Summary: Pull Request resolved: facebook#44185 This change will fix a symbol not found for JSC Runtime. The `if` check was not a compile time check, therefore the symbol ended up in the binary even if it is not available. Following this post on [Apple forum](https://forums.developer.apple.com/forums/thread/749534), this changes should do the trick. ## Changelog [iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable) Reviewed By: hash3r Differential Revision: D56425834 fbshipit-source-id: a37af51b078bd47a938e6b65d9d8e0f7506e746f
Saadnajmi
added a commit
to microsoft/react-native-macos
that referenced
this pull request
May 21, 2024
## Summary: Merge up to RN Core's `0.73.8` release, with one notable change: - Instead of picking up facebook#44185 , let's instead port facebook#39549 to React Native macOS, so that we have direct debugging with JSC working. ## Test Plan: CI should pass.
kosmydel
pushed a commit
to kosmydel/react-native
that referenced
this pull request
Jun 11, 2024
Summary: Pull Request resolved: facebook#44185 This change will fix a symbol not found for JSC Runtime. The `if` check was not a compile time check, therefore the symbol ended up in the binary even if it is not available. Following this post on [Apple forum](https://forums.developer.apple.com/forums/thread/749534), this changes should do the trick. ## Changelog [iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable) Reviewed By: hash3r Differential Revision: D56425834 fbshipit-source-id: a37af51b078bd47a938e6b65d9d8e0f7506e746f
This was referenced Jun 28, 2024
Merged
facebook-github-bot
pushed a commit
that referenced
this pull request
Dec 18, 2024
Summary: Originally with 5cf8f43 , we added a call to a new Apple API `JSGlobalContextSetInspectable` to ensure that our Javascript running with JSC is debuggable. That change was guarded with a `__builtin_available(macOS 13.3, iOS 16.4, tvOS 16.4, *)` check to make sure it only ran on OS'es where to function existed. Later, in 3eeee11 we did an extra guard in the way of a macro to check we were compiling against a new enough version of Xcode (so that Xcode knows about the symbol). Between the runtime check and the compile time check, we should be good right? Wrong! As it turns out, this bit of code still caused crashes on iOS 15 devices (See this [Apple Forum Thread](https://forums.developer.apple.com/forums/thread/749534)). To address this, #44185 was added which added a new compiler guard (`__OSX_AVAILABLE_STARTING(MAC_NA, IPHONE_16_4)` was added. Unfortunately, this guard is incorrect: It is basically checking if our minimum iOS deployment target is 16.4 (It's not, as of writing it is iOS 13.4), which effectively means this code is never compiled and one can never direct debug with JSC on iOS 16.4+ 😨! So what went wrong, and why were the first two guards not good enough? Three main reasons.. Firstly, this is a device only crash, and not reproducible on simulator. This is probably why the crash was not caught earlier. Secondly, It's because system frameworks (like JavascriptCore) are _dynamically_ linked: the linker doesn't look for the symbol till runtime (and crashes when doing so). Thirdly, It's because we are _strongly_ linking the framework, so every symbol must be present and the macros / guard Apple provides with `AvailabilityMacros.h` don't work. What we want to do is link JavascriptCore as a `weak_framework`, more info here: https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html From that link: > One challenge faced by developers is that of taking advantage of new features introduced in new versions of OS X while still supporting older versions of the system. Normally, if an application uses a new feature in a framework, it is unable to run on earlier versions of the framework that do not support that feature. Such applications would either fail to launch or crash when an attempt to use the feature was made. Apple has solved this problem by adding support for weakly-linked symbols. >When a symbol in a framework is defined as weakly linked, the symbol does not have to be present at runtime for a process to continue running. The static linker identifies a weakly linked symbol as such in any code module that references the symbol. The dynamic linker uses this same information at runtime to determine whether a process can continue running. If a weakly linked symbol is not present in the framework, the code module can continue to run as long as it does not reference the symbol. However, if the symbol is present, the code can use it normally. This seems to be exactly what we want, and the Apple provided method for using new APIs in system frameworks! Let's update our podspecs so we link JavascriptCore weakly. As a bonus (and admittedly, the original purpose of this PR) let's add macOS support to the `JSC_HAS_INSPECTABLE` macro (This file `JSCRuntime.cpp` used to have more explicit macOS support in it's macros, but I had removed it with fb30fca). ## Changelog: [IOS] [FIXED] - Fix Direct Debugging with JSC Pull Request resolved: #39549 Test Plan: Tested that RNTester doesn't crash on boot running on an iPad Air 2 running iOS 15.8., and that an iOS 17.2 simulator is debuggable. Built RN-Tester and RN-Tester-macOS and verified both show up in Safari Web Inspectors' debug menu: <img width="1316" alt="Screenshot 2023-09-19 at 10 48 43 PM" src="https://github.com/facebook/react-native/assets/6722175/c642e6e0-36af-4c9f-845a-7e491489f419"> macOS screenshot small bc I got some internal stuff I gotta crop 😅 <img width="347" alt="Screenshot 2023-09-19 at 10 53 46 PM" src="https://github.com/facebook/react-native/assets/6722175/1e802c88-02b8-49e1-8fd2-d91726ca1e93"> Reviewed By: huntie Differential Revision: D67338150 Pulled By: cipolleschi fbshipit-source-id: 620c3b3cc1e37e54de7fa4dc9956a02c8f3c09f8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This change will fix a symbol not found for JSC Runtime.
The
ifcheck was not a compile time check, therefore the symbol ended up in the binary even if it is not available.Following this post on Apple forum, this changes should do the trick.
Changelog
[iOS][Fixed] - Fix Symbol not found: (_JSGlobalContextSetInspectable)
Differential Revision: D56425834