Skip to content

Commit

Permalink
Generated IPC decoders check the decode success twice
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258604
rdar://problem/111434133

Reviewed by Alex Christensen.

The check would be like:
  decode property
  check success
  decode property
  check success

On first decode error, the subsequent decodes will fail as per design.
Thus we can omit the redundant caller checks and check only once after
series of decodes, before using the decoded properties.

Instead of checking each decode operation result, check as follows:

  decode properties
  if (!decoder.isValid())
    return std::nullopt;
  use properties
  decode more properties
  if (!decoder.isValid())
    return std::nullopt;
  use properties

This should, very slightly, optimize for the standard case of all
decodes succeeding.

* Source/WebKit/Scripts/generate-serializers.py:
(decode_type):
(generate_impl):
* Source/WebKit/Scripts/webkit/tests/GeneratedSerializers.cpp:
(IPC::ArgumentCoder<Namespace::Subnamespace::StructName>::decode):
(IPC::ArgumentCoder<Namespace::OtherClass>::decode):
(IPC::ArgumentCoder<Namespace::ReturnRefClass>::decode):
(IPC::ArgumentCoder<Namespace::EmptyConstructorStruct>::decode):
(IPC::ArgumentCoder<Namespace::EmptyConstructorNullable>::decode):
(IPC::ArgumentCoder<WithoutNamespace>::decode):
(IPC::ArgumentCoder<WithoutNamespaceWithAttributes>::decode):
(IPC::ArgumentCoder<WebCore::InheritsFrom>::decode):
(IPC::ArgumentCoder<WebCore::InheritanceGrandchild>::decode):
(IPC::ArgumentCoder<WTF::Seconds>::decode):
(IPC::ArgumentCoder<WTF::CreateUsingClass>::decode):
(IPC::ArgumentCoder<WebCore::FloatBoxExtent>::decode):
(IPC::ArgumentCoder<NullableSoftLinkedMember>::decode):
(IPC::ArgumentCoder<WebCore::TimingFunction>::decode):
(IPC::ArgumentCoder<Namespace::ConditionalCommonClass>::decode):
(IPC::ArgumentCoder<Namespace::CommonClass>::decode):
(IPC::ArgumentCoder<Namespace::AnotherCommonClass>::decode):
(IPC::ArgumentCoder<WebCore::MoveOnlyBaseClass>::decode):
(IPC::ArgumentCoder<WebCore::MoveOnlyDerivedClass>::decode):

Canonical link: https://commits.webkit.org/265717@main
  • Loading branch information
kkinnunen-apple committed Jul 3, 2023
1 parent 44564f3 commit 6006aa6
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 112 deletions.
21 changes: 8 additions & 13 deletions Source/WebKit/Scripts/generate-serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def decode_type(type):

if type.members_are_subclasses:
result.append(' auto type = decoder.decode<' + type.subclass_enum_name() + '>();')
result.append(' if (!type)')
result.append(' if (UNLIKELY(!decoder.isValid()))')
result.append(' return std::nullopt;')
result.append('')

Expand All @@ -433,13 +433,11 @@ def decode_type(type):
match = re.search("RetainPtr<(.*)>", member.type)
assert match
result.append(' auto ' + sanitized_variable_name + ' = IPC::decode<' + match.groups()[0] + '>(decoder, @[ ' + decodable_classes[0] + ' ]);')
result.append(' if (!' + sanitized_variable_name + ')')
result.append(' return std::nullopt;')
elif member.is_subclass:
result.append(' if (type == ' + type.subclass_enum_name() + "::" + member.name + ') {')
typename = member.namespace + "::" + member.name
result.append(' auto result = decoder.decode<Ref<' + typename + '>>();')
result.append(' if (!result)')
result.append(' if (UNLIKELY(!decoder.isValid()))')
result.append(' return std::nullopt;')
result.append(' return WTFMove(*result);')
result.append(' }')
Expand All @@ -454,38 +452,34 @@ def decode_type(type):
if 'Nullable' in member.attributes:
indentation = 2
result.append(' auto has' + sanitized_variable_name + ' = decoder.decode<bool>();')
result.append(' if (!has' + sanitized_variable_name + ')')
result.append(' if (UNLIKELY(!decoder.isValid()))')
result.append(' return std::nullopt;')
result.append(' std::optional<' + member.type + '> ' + sanitized_variable_name + ';')
result.append(' if (*has' + sanitized_variable_name + ') {')
auto_specifier = '' if 'Nullable' in member.attributes else 'auto '
result.append(indent(indentation) + auto_specifier + sanitized_variable_name + ' = IPC::decode<' + match.groups()[0] + '>(decoder, ' + soft_linked_classes[0] + ');')
result.append(indent(indentation) + 'if (!' + sanitized_variable_name + ')')
result.append(indent(indentation) + ' return std::nullopt;')
if 'Nullable' in member.attributes:
result.append(' } else')
result.append(' ' + sanitized_variable_name + ' = std::optional<' + member.type + '> { ' + member.type + ' { } };')
elif 'Nullable' in member.attributes:
result.append(' auto has' + sanitized_variable_name + ' = decoder.decode<bool>();')
result.append(' if (!has' + sanitized_variable_name + ')')
result.append(' if (UNLIKELY(!decoder.isValid()))')
result.append(' return std::nullopt;')
result.append(' std::optional<' + member.type + '> ' + sanitized_variable_name + ';')
result.append(' if (*has' + sanitized_variable_name + ') {')
# FIXME: This should be below
result.append(' ' + sanitized_variable_name + ' = decoder.decode<' + member.type + '>();')
result.append(' if (!' + sanitized_variable_name + ')')
result.append(' return std::nullopt;')
result.append(' } else')
result.append(' ' + sanitized_variable_name + ' = std::optional<' + member.type + '> { ' + member.type + ' { } };')
else:
assert len(soft_linked_classes) == 0
result.append(' auto ' + sanitized_variable_name + ' = decoder.decode<' + member.type + '>();')
result.append(' if (!' + sanitized_variable_name + ')')
result.append(' return std::nullopt;')
for attribute in member.attributes:
match = re.search(r'Validator=\'(.*)\'', attribute)
if match:
validator, = match.groups()
result.append(' if (UNLIKELY(!decoder.isValid()))')
result.append(' return std::nullopt;')
result.append('')
result.append(' if (!(' + validator + '))')
result.append(' return std::nullopt;')
Expand All @@ -495,7 +489,6 @@ def decode_type(type):
assert not match
if member.condition is not None:
result.append('#endif')
result.append('')
return result


Expand Down Expand Up @@ -624,6 +617,8 @@ def generate_impl(serialized_types, serialized_enums, headers):
result.append('{')
result = result + decode_type(type)
if not type.members_are_subclasses:
result.append(' if (UNLIKELY(!decoder.isValid()))')
result.append(' return std::nullopt;')
if type.populate_from_empty_constructor:
result.append(' ' + type.namespace_and_name() + ' result;')
for member in type.serialized_members():
Expand Down
Loading

0 comments on commit 6006aa6

Please sign in to comment.