Bug 258608 - Generated IPC serializers for MoveOnlyBaseClass&& are incorrect
Summary: Generated IPC serializers for MoveOnlyBaseClass&& are incorrect
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Matt Woodrow
URL:
Keywords: InRadar
Depends on:
Blocks: 257580
  Show dependency treegraph
 
Reported: 2023-06-27 23:44 PDT by Kimmo Kinnunen
Modified: 2023-10-27 00:22 PDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2023-06-27 23:44:12 PDT
Generated IPC serializers forMoveOnlyBaseClass&& are incorrect

It appears that the code tries to implement move through base class rvalue reference. It is not possible in general case following c++ rvalue semantics, as rvalue constructors are not polymorphic.

Move makes no sense:

void ArgumentCoder<WebCore::MoveOnlyBaseClass>::encode(Encoder& encoder, WebCore::MoveOnlyBaseClass&& instance)
{
    if (auto* subclass = dynamicDowncast<WebCore::MoveOnlyDerivedClass>(instance)) {
        encoder << WebCore_MoveOnlyBaseClass_Subclass::MoveOnlyDerivedClass;
        encoder << WTFMove(*subclass);
        return;
    }
    ASSERT_NOT_REACHED();
}
Comment 1 Radar WebKit Bug Importer 2023-07-04 23:45:17 PDT
<rdar://problem/111768907>
Comment 2 Kimmo Kinnunen 2023-10-27 00:22:08 PDT
Maybe you're right that when using downcast, it is consistent with by-ref encoding.
I don't remember what was the idea why it's bad. The only thing I could re-comeup with is that MoveOnlyDerivedClass might not be the final class, so then moving parts of the class is not ok. Comparing to by-ref encoding, copying only part of the class is ok from the safety perspective. However, it's not probably intended so both would be equally buggy and as such rvalue ref code would be consistent with ref code.