facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.17k stars 24.32k forks source link

[Codegen] overloaded fromRawValue across modules for ArrayEnum props with type alias uint32_t cause runtime crash #43821

Closed netmaxt3r closed 5 months ago

netmaxt3r commented 7 months ago

Description

codegen generates duplicate fromRawValue for array enum type props across different modules in same namespace facebook::react with different type alias for uint32_t In the reproducer app i used react-native-webview as an example to crash with Modal component from react-native Modal generates node_modules/react-native/ReactCommon/react/renderer/components/rncore/Props.h from line 295 for parsing supportedOrientations prop at line 323

static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, ModalHostViewSupportedOrientationsMask &result)

WebView generates /ios/Pods/Headers/Private/React-Codegen/react/renderer/components/RNCWebViewSpec/Props.h from line 110 for parsing dataDetectorTypes prop at line 142

static inline void fromRawValue(const PropsParserContext& context, const RawValue &value, RNCWebViewDataDetectorTypesMask &result) {

both function only differ in result parameter but they same at runtime because of same type alias

using RNCWebViewDataDetectorTypesMask = uint32_t;
using ModalHostViewSupportedOrientationsMask = uint32_t;

In the stack trace of crash we can see it starts to parse ModalHostViewProps then call fromRawValue from RNCWebViewSpec/Props.h with method signature facebook::react::fromRawValue(facebook::react::PropsParserContext const&, facebook::react::RawValue const&, unsigned int&)

One possible solution i see is using strong warapper class/struct for alias types, Or is there any compiler flag we can use to fix function overloading?

Note:- I haven't used react-native-webview in the js side, it is just for the codegen. If i remove supportedOrientations prop completely app wont crash

Steps to reproduce

clone repo https://github.com/netmaxt3r/rn-codegen-runtime-crash

  1. yarn install
  2. bundle install
  3. RCT_NEW_ARCH_ENABLED=1 bundle exec pod install
  4. yarn start
  5. On another console yarn ios

React Native Version

0.73.6

Affected Platforms

Runtime - iOS

Areas

Codegen

Output of npx react-native info

System:
  OS: macOS 14.4.1
  CPU: (12) arm64 Apple M2 Max
  Memory: 86.72 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.11.1
    path: ~/.nvm/versions/node/v20.11.1/bin/node
  Yarn:
    version: 1.22.22
    path: /opt/homebrew/bin/yarn
  npm:
    version: 10.2.4
    path: ~/.nvm/versions/node/v20.11.1/bin/npm
  Watchman:
    version: 2024.04.01.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.4
      - iOS 17.4
      - macOS 14.4
      - tvOS 17.4
      - visionOS 1.1
      - watchOS 10.4
  Android SDK:
    API Levels:
      - "23"
      - "28"
      - "29"
      - "30"
      - "31"
      - "33"
      - "33"
      - "34"
    Build Tools:
      - 28.0.3
      - 29.0.2
      - 30.0.3
      - 33.0.0
      - 33.0.1
      - 33.0.2
      - 34.0.0
    System Images:
      - android-33 | Google Play ARM 64 v8a
      - android-34 | Android TV ARM 64 v8a
      - android-34 | Google TV ARM 64 v8a
      - android-34 | Google APIs ARM 64 v8a
    Android NDK: 22.1.7171670
IDEs:
  Android Studio: Not Found
  Xcode:
    version: 15.3/15E204a
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 21.0.1
    path: /usr/bin/javac
  Ruby:
    version: 2.7.6
    path: /Users/maxter/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.6
    wanted: 0.73.6
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

#2  0x00000001801655c0 in abort ()
#3  0x00000001046c6f74 in facebook::react::fromRawValue(facebook::react::PropsParserContext const&, facebook::react::RawValue const&, unsigned int&) at /Users/maxter/tmp/p/rn-codegen-runtime-crash/ReproducerApp/ios/Pods/Headers/Private/React-Codegen/react/renderer/components/RNCWebViewSpec/Props.h:181
#4  0x00000001046bece4 in unsigned int facebook::react::convertRawProp<unsigned int, unsigned int>(facebook::react::PropsParserContext const&, facebook::react::RawProps const&, char const*, unsigned int const&, unsigned int const&, char const*, char const*) at /Users/maxter/tmp/p/rn-codegen-runtime-crash/ReproducerApp/ios/Pods/Headers/Public/React-Fabric/react/renderer/core/propsConversions.h:132
#5  0x00000001049285fc in facebook::react::ModalHostViewProps::ModalHostViewProps(facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&) at /Users/maxter/tmp/p/rn-codegen-runtime-crash/ReproducerApp/node_modules/react-native/ReactCommon/react/renderer/components/rncore/Props.cpp:152
#6  0x0000000104b560b0 in facebook::react::ModalHostViewProps* std::__1::construct_at[abi:ue170006]<facebook::react::ModalHostViewProps, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const, facebook::react::RawProps const&, facebook::react::ModalHostViewProps*>(facebook::react::ModalHostViewProps*, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&&, facebook::react::RawProps const&) at /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator17.4.sdk/usr/include/c++/v1/__memory/construct_at.h:41

Reproducer

https://github.com/netmaxt3r/rn-codegen-runtime-crash

Screenshots and Videos

No response

cortinico commented 7 months ago

Thanks for the reproducer @netmaxt3r

One possible solution i see is using strong warapper class/struct for alias types, Or is there any compiler flag we can use to fix function overloading?

Type wrappers could be a solution here, or we could qualify the fromRawValue function with the module name so they won't clash at runtime.

Let me get back to you on potential solution here.

netmaxt3r commented 7 months ago

we could qualify the fromRawValue function with the module name

we need to consider single module having multiple array enum props as well , which could clash each other in this case

cortinico commented 7 months ago

we could qualify the fromRawValue function with the module name

we need to consider single module having multiple array enum props as well , which could clash each other in this case

Ah that's also a really valid scenario

Martin2037 commented 7 months ago

same problem, need help

netmaxt3r commented 6 months ago

@cortinico any news on the possible approach to fix this ? If i get direction i can start working on a PR

cortinico commented 6 months ago

One possible solution i see is using strong warapper class/struct for alias types

@netmaxt3r I believe the best solution would be the one you suggested, to wrap the types into a struct and use them so the overload resolution won't be ambiguous. If you could send over a PR to draft this fix, we can look into merging it (or see if there are any blockers)

netmaxt3r commented 6 months ago

@cortinico please see PR #44123 , I am pretty new to codegen, i need to figure out the call stack for static inline std::string toString(const ${enumMask}Wrapped &wrapped) function and check other platforms + update jest snapshots. Please review changes with internal team if i missed any scenario.

knro commented 5 months ago

Any ideas how to apply the PR to an existing 0.74.1 project? Seems the PR code is typescript while the code in node_modules/@react-native/codegen is javascript

I feel I'm missing a step here?

netmaxt3r commented 5 months ago

@knro try @react-native/codegen version 0.75.0-nightly-20240429-b7de91666 might work with 0.74

knro commented 5 months ago

Thank you @netmaxt3r. It's still crashing (with the same backtrace that you have). I noticed we have two codegens apparently.

node_modules/@react-native/codegen which is updated to 0.75 night above node_modules/react-native-codegen which is v0.71

I tried deleting the react-native-codegen but after yarn install it gets added again to node_modules, so not sure how to make it select the first one?

knro commented 5 months ago

Ok I was able to remove react-native-codegen from package.json so that's resolved. But I noticed the app is still crashing, then I noticed when I install the bundle, I get this:

Installing React (0.74.1)
Installing React-Codegen (0.74.1)
Installing React-Core (0.74.1)

It appears that it installing Pod for 0.74.1 ? Not sure how you can override them to 0.75 nightly if that's at all possible?

This is the backtrace:

Thread 9 Crashed:: com.facebook.react.JavaScript
0   libsystem_kernel.dylib                 0x1063c53b0 __pthread_kill + 8
1   libsystem_pthread.dylib                0x10644f124 pthread_kill + 256
2   libsystem_c.dylib                      0x1801655c0 abort + 104
3   stellarmate                            0x104881e58 facebook::react::fromRawValue(facebook::react::PropsParserContext const&, facebook::react::RawValue const&, unsigned int&) + 580 (Props.h:180)
4   stellarmate                            0x10487e974 unsigned int facebook::react::convertRawProp<unsigned int, unsigned int>(facebook::react::PropsParserContext const&, facebook::react::RawProps const&, char const*, unsigned int const&, unsigned int const&, char const*, char const*) + 116 (propsConversions.h:130)
5   stellarmate                            0x10497a3bc facebook::react::ModalHostViewProps::ModalHostViewProps(facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&) + 436 (Props.cpp:123)
6   stellarmate                            0x104a0a474 facebook::react::ModalHostViewProps* std::__1::construct_at[abi:ue170006]<facebook::react::ModalHostViewProps, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&, facebook::react::ModalHostViewProps*>(facebook::react::ModalHostViewProps*, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&) + 4 (construct_at.h:41) [inlined]
7   stellarmate                            0x104a0a474 void std::__1::allocator_traits<std::__1::allocator<facebook::react::ModalHostViewProps>>::construct[abi:ue170006]<facebook::react::ModalHostViewProps, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&, void, void>(std::__1::allocator<facebook::react::ModalHostViewProps>&, facebook::react::ModalHostViewProps*, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&) + 4 (allocator_traits.h:304) [inlined]
8   stellarmate                            0x104a0a474 std::__1::__shared_ptr_emplace<facebook::react::ModalHostViewProps, std::__1::allocator<facebook::react::ModalHostViewProps>>::__shared_ptr_emplace[abi:ue170006]<facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&>(std::__1::allocator<facebook::react::ModalHostViewProps>, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&) + 40 (shared_ptr.h:300)
9   stellarmate                            0x104a0a41c std::__1::__shared_ptr_emplace<facebook::react::ModalHostViewProps, std::__1::allocator<facebook::react::ModalHostViewProps>>::__shared_ptr_emplace[abi:ue170006]<facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&>(std::__1::allocator<facebook::react::ModalHostViewProps>, facebook::react::PropsParserContext const&, facebook::react::ModalHostViewProps const&, facebook::react::RawProps const&) + 16 (shared_ptr.h:292) [inlined]
netmaxt3r commented 5 months ago

@knro

I noticed we have two codegens apparently.

please add using yarn resolution example

I noticed when I install the bundle

that is native code , we just need to bump the js package for this fix

I noticed the app is still crashing

I tested with latest rn app see reproducer app. could you try rebuilding app from clean Pods

knro commented 5 months ago

@netmaxt3r Thank you!!! The resolutions field did the trick and now it no longer crashes.

cortinico commented 5 months ago

Closing as https://github.com/facebook/react-native/pull/44123 got merged which fixes this issue

netmaxt3r commented 5 months ago

@cortinico is it possible keep this open till fix land on release version or create fix release for 0.74.x (current) rn version?

cortinico commented 5 months ago

is it possible keep this open till fix land on release version or create fix release for 0.74.x (current) rn version?

We generally close issues once they're fixed to keep track of the remaining actionable work. As there is nothing actionable here, I'd rather keep it closed.

We'll be cutting the 0.75 release branch really soon, and this change will go out in 0.75.0-rc.0