Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@mkustermann
Copy link
Member

Flutter makes use of C++ multiple inheritence in it's C++ classes that have corresponding Dart objects attached. An example is e.g.

  class Canvas : public RefCountedDartWrappable<Canvas>, DisplayListOpFlags { }

  template <typename T>
  class RefCountedDartWrappable : public fml::RefCountedThreadSafe<T>,
                                  public tonic::DartWrappable { }

When a C++ class has multiple base classes, the C++ compiler has the liberty to decide the order in which they get layed out in memory. (It doesn't necessarily follow declaration order, in fact it has a preference for having classes with
vtables before classes without vtables.)

Two casts to consider (with multiple inheritance in mind):

  • upcasts: Those are done by C++ automatically but can modify the actual address (i.e. pointer value)

  • downcasts: Those have to be done with static_cast<>(), which can also modify the address (i.e. pointer value) - using reinterpret_cast<>() is incorrect (as it doesn't modify the address)

Now the Dart objects that refer to C++ objects have a native field in them. The value of that field is a tonic::DartWrappable*. That means whenever we convert between the C++ object pointer (e.g. flutter::Canvas*) and the value of the native field (tonic::Wrappable*) the actual address / pointer may need modification.

There were bugs in the C FFI based tonic layer: Pointer values coming from Dart (via C FFI) are dart::Wrappable* and not pointers to subtypes.

=> For methods we type the first parameter in tonic trampolines as
dart::Wrappable* and static_cast<Class*>() that value

=> Similarly for arguments, the parameter has to be typed as
dart::Wrappable* and static_cast<Class*>() that value

How did it ever work? Well it happens to be that the C++ compiler decided to layout tonic::DartWrappable before
fml::RefCountedThreadSafe, making the tonic::DartWrappable* and the flutter::Canvas* (and other classes) have the same pointer value.

=> This worked due to implementation choice of C++ compiler.
=> This breaks immediately if one decided to add another base class (with virtual methods) to RefCountedDartWrappable

Flutter makes use of C++ multiple inheritence in it's C++ classes that
have corresponding Dart objects attached. An example is e.g.

  class Canvas : public RefCountedDartWrappable<Canvas>, DisplayListOpFlags { }

  template <typename T>
  class RefCountedDartWrappable : public fml::RefCountedThreadSafe<T>,
                                  public tonic::DartWrappable { }

When a C++ class has multiple base classes, the C++ compiler has the
liberty to decide the order in which they get layed out in memory. (It
doesn't follow declaration order, in fact it has a preference for having
classes with vtables before classes without vtables.)

Two casts to consider (with multiple inheritance in mind):

* upcasts: Those are done by C++ automatically but can modify the
  actual address (i.e. pointer value)

* downcasts: Those have to be done with `static_cast<>()`, which can also
  modify the address (i.e. pointer value) - using `reinterpret_cast<>()`
  is incorrect (as it doesn't modify the address)

Now the Dart objects that refer to C++ objects have a native field in
them. The value of that field is a `tonic::DartWrappable*`. That means
whenever we convert between the C++ object pointer (e.g. `flutter::Canvas*`)
and the value of the native field (`tonic::Wrappable*`) the actual
address / pointer may need modification.

There were bugs in the C FFI based tonic layer: Pointer values coming
from Dart (via C FFI) are `dart::Wrappable*` and not pointers to
subtypes.

=> For methods we type the first parameter in tonic trampolines as
    `dart::Wrappable*` and `static_cast<Class*>()` that value

=> Similarly for arguments, the parameter has to be typed as
    `dart::Wrappable*` and `static_cast<Class*>()` that value

How did it ever work? Well it happens to be that the C++ compiler
decided to layout `tonic::DartWrappable` before
`fml::RefCountedThreadSafe`, making the `tonic::DartWrappable*` and the
`flutter::Canvas*` (and other classes) have the same pointer value.

=> This worked due to implementation choice of C++ compiler.
=> This breaks immediately if one decided to add another base class
(with virtual methods) to `RefCountedDartWrappable`
@mkustermann
Copy link
Member Author

@jason-simmons Thank you for taking a look!

@mkustermann mkustermann merged commit b9b3269 into flutter:main Nov 6, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants