Skip to content

Commit

Permalink
Fix unsafe downcasting in ShadowTree::emitLayoutEvents
Browse files Browse the repository at this point in the history
Summary:
## Changelog:
[Internal] - 

This was revealed when running an ASAN build on MacOS - we were doing an unsafe downcast from `LayoutableShadowNode->ViewShadowNode` inside `ShadowTree::emitLayoutEvents`.

Which, even though incidentally worked, is generally unsafe, as we may get e.g. `ImageShadowNode` there, which doesn't inherit from `ViewShadowNode`.

That downcast to `ViewShadowNode` wasn't even required, to begin with, as all the needed information can be already extracted from the `LayoutableShadowNode` itself.

UndefinedBehaviorSanitizer: undefined-behavior xplat/js/react-native-github/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp:579:9 in
Traceback (most recent call last):
  File "/Users/rshest/fbsource/xplat/ReactNative/react-native-cxx/./build_and_run.py", line 182, in <module>
    main()
  File "/Users/rshest/fbsource/xplat/ReactNative/react-native-cxx/./build_and_run.py", line 170, in main
    run_buck_build(
  File "/Users/rshest/fbsource/xplat/ReactNative/react-native-cxx/scripts/build.py", line 362, in run_buck_build
    subprocess.run(
  File "/usr/local/fbcode/platform010/Python3.10.framework/Versions/3.10/lib/python3.10/subprocess.py", line 526, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command 'buck2 run @//arvr/mode/mac-arm/dev-asan //xplat/ReactNative/react-native-cxx/samples/granite/mac:ReactNativeCxx --config react_native.force_cxx_platform=True --config react_native.force_enable_fusebox=True --config user.default_platform=macosx-arm64 --config igl.backend_enable_vulkan=false -- --test images' returned non-zero exit status 1.
```

Differential Revision: D56062334
  • Loading branch information
rshest authored and facebook-github-bot committed Apr 12, 2024
1 parent 7131f94 commit 2762508
Showing 1 changed file with 4 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -575,15 +575,14 @@ void ShadowTree::emitLayoutEvents(

for (const auto* layoutableNode : affectedLayoutableNodes) {
// Only instances of `ViewShadowNode` (and subclasses) are supported.
const auto& viewShadowNode =
static_cast<const ViewShadowNode&>(*layoutableNode);
const auto& viewEventEmitter =
static_cast<const ViewEventEmitter&>(*viewShadowNode.getEventEmitter());

const auto& viewEventEmitter = static_cast<const BaseViewEventEmitter&>(
*layoutableNode->getEventEmitter());

// Checking if the `onLayout` event was requested for the particular Shadow
// Node.
const auto& viewProps =
static_cast<const ViewProps&>(*viewShadowNode.getProps());
static_cast<const BaseViewProps&>(*layoutableNode->getProps());
if (!viewProps.onLayout) {
continue;
}
Expand Down

0 comments on commit 2762508

Please sign in to comment.