Skip to content
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

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

Open
netmaxt3r opened this issue Apr 3, 2024 · 7 comments
Labels
Impact: Bug The issue represents a bug somewhere Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@netmaxt3r
Copy link
Contributor

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

@netmaxt3r netmaxt3r added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Apr 3, 2024
@cortinico
Copy link
Contributor

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.

@cortinico cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Impact: Bug The issue represents a bug somewhere and removed Needs: Triage 🔍 labels Apr 4, 2024
@netmaxt3r
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link

same problem, need help

@netmaxt3r
Copy link
Contributor Author

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

@cortinico
Copy link
Contributor

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
Copy link
Contributor Author

@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.

facebook-github-bot pushed a commit that referenced this issue Apr 22, 2024
Summary:
codegen generates type alias for  array enum props with uint32_t which cause wrong overloaded fromRawValue to call at runtime eventually app to terminate
more detailed info at issue #43821

## Changelog:

[Internal] [Fixed] - Codegen for array enum props

Pull Request resolved: #44123

Test Plan: TODO

Reviewed By: cipolleschi

Differential Revision: D56414554

Pulled By: dmytrorykun

fbshipit-source-id: 0ec1b65951bc16ff58dd2b119c97a4e3fac2b161
kosmydel pushed a commit to kosmydel/react-native that referenced this issue May 6, 2024
Summary:
codegen generates type alias for  array enum props with uint32_t which cause wrong overloaded fromRawValue to call at runtime eventually app to terminate
more detailed info at issue facebook#43821

## Changelog:

[Internal] [Fixed] - Codegen for array enum props

Pull Request resolved: facebook#44123

Test Plan: TODO

Reviewed By: cipolleschi

Differential Revision: D56414554

Pulled By: dmytrorykun

fbshipit-source-id: 0ec1b65951bc16ff58dd2b119c97a4e3fac2b161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Bug The issue represents a bug somewhere Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

3 participants