Verify non-serialized members of serialized types are not accidentall…

…y forgotten https://bugs.webkit.org/show_bug.cgi?id=255551 rdar://108161764 Reviewed by Chris Dumez. I introduce the NotSerialized attribute for members that are ... not serialized. Until we have https://wg21.link/P1240 in the language, we can't iterate the members at compile time and make sure we got them all. We already use MembersInCorrectOrder to make sure we got them in the correct order, but we didn't have a check that we got them all until now. This makes a struct that has the same members in the same order and static_asserts that it has the same size as the actual struct. To reduce memory use and make this new sizeof trick work, I made ReportBody::type() a virtual function call instead of using another byte to store the type. * Source/WebCore/Modules/reporting/DeprecationReportBody.cpp: (WebCore::DeprecationReportBody::DeprecationReportBody): * Source/WebCore/Modules/reporting/DeprecationReportBody.h: * Source/WebCore/Modules/reporting/ReportBody.cpp: (WebCore::ReportBody::ReportBody): Deleted. (WebCore::ReportBody::reportBodyType const): Deleted. * Source/WebCore/Modules/reporting/ReportBody.h: * Source/WebCore/Modules/reporting/TestReportBody.cpp: (WebCore::TestReportBody::TestReportBody): * Source/WebCore/Modules/reporting/TestReportBody.h: * Source/WebCore/loader/COEPInheritenceViolationReportBody.cpp: (WebCore::COEPInheritenceViolationReportBody::COEPInheritenceViolationReportBody): * Source/WebCore/loader/COEPInheritenceViolationReportBody.h: * Source/WebCore/loader/CORPViolationReportBody.cpp: (WebCore::CORPViolationReportBody::CORPViolationReportBody): * Source/WebCore/loader/CORPViolationReportBody.h: * Source/WebCore/page/csp/CSPViolationReportBody.cpp: (WebCore::CSPViolationReportBody::CSPViolationReportBody): * Source/WebCore/page/csp/CSPViolationReportBody.h: * Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.serialization.in: * Source/WebKit/Scripts/generate-serializers.py: (SerializedType.__init__): (SerializedType.has_bit_field): (SerializedType): (SerializedType.serialized_members): (check_type_members): (check_type_members.is): (check_type_members.is.is): (encode_type): (decode_type): (construct_type): (generate_impl): (generate_serialized_type_info): * Source/WebKit/Scripts/webkit/tests/GeneratedSerializers.cpp: (IPC::ArgumentCoder<Namespace::Subnamespace::StructName>::encode): (IPC::ArgumentCoder<Namespace::EmptyConstructorNullable>::encode): (IPC::ArgumentCoder<WithoutNamespace>::encode): (IPC::ArgumentCoder<WithoutNamespaceWithAttributes>::encode): (IPC::ArgumentCoder<WTF::CreateUsingClass>::encode): (IPC::ArgumentCoder<NullableSoftLinkedMember>::encode): (IPC::ArgumentCoder<Namespace::ConditionalCommonClass>::encode): (IPC::ArgumentCoder<Namesapce::CommonClass>::encode): (IPC::ArgumentCoder<Namesapce::AnotherCommonClass>::encode): (IPC::ArgumentCoder<Namesapce::AnotherCommonClass>::decode): * Source/WebKit/Scripts/webkit/tests/GeneratedSerializers.h: * Source/WebKit/Scripts/webkit/tests/SerializedTypeInfo.cpp: * Source/WebKit/Scripts/webkit/tests/TestSerializedType.serialization.in: * Source/WebKit/Shared/SessionState.serialization.in: * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in: Canonical link: https://commits.webkit.org/263133@main
WebKit · Apr 19, 2023 · be8e1cb · be8e1cb
1 parent aa24abe
commit be8e1cb
Show file tree

Hide file tree

Showing 20 changed files with 225 additions and 72 deletions.
diff --git a/Source/WebCore/Modules/reporting/DeprecationReportBody.cpp b/Source/WebCore/Modules/reporting/DeprecationReportBody.cpp
@@ -37,8 +37,7 @@ namespace WebCore {
 WTF_MAKE_ISO_ALLOCATED_IMPL(DeprecationReportBody);
 
 DeprecationReportBody::DeprecationReportBody(String&& id, WallTime anticipatedRemoval, String&& message, String&& sourceFile, std::optional<unsigned> lineNumber, std::optional<unsigned> columnNumber)
-    : ReportBody(ViolationReportType::Deprecation)
-    , m_id(WTFMove(id))
+    : m_id(WTFMove(id))
     , m_anticipatedRemoval(anticipatedRemoval)
     , m_message(WTFMove(message))
     , m_sourceFile(WTFMove(sourceFile))

diff --git a/Source/WebCore/Modules/reporting/DeprecationReportBody.h b/Source/WebCore/Modules/reporting/DeprecationReportBody.h
@@ -53,6 +53,8 @@ class DeprecationReportBody final : public ReportBody {
 private:
     DeprecationReportBody(String&& id, WallTime anticipatedRemoval, String&& message, String&& sourceFile, std::optional<unsigned> lineNumber, std::optional<unsigned> columnNumber);
 
+    ViolationReportType reportBodyType() const final { return ViolationReportType::Deprecation; }
+
     const String m_id;
     const WallTime m_anticipatedRemoval;
     const String m_message;

diff --git a/Source/WebCore/Modules/reporting/ReportBody.cpp b/Source/WebCore/Modules/reporting/ReportBody.cpp
@@ -33,15 +33,6 @@ namespace WebCore {
 
 WTF_MAKE_ISO_ALLOCATED_IMPL(ReportBody);
 
-ReportBody::ReportBody(ViolationReportType type)
-    : m_reportBodyType(type)
-{ }
-
 ReportBody::~ReportBody() = default;
 
-ViolationReportType ReportBody::reportBodyType() const
-{
-    return m_reportBodyType;
-}
-
 } // namespace WebCore
diff --git a/Source/WebCore/Modules/reporting/ReportBody.h b/Source/WebCore/Modules/reporting/ReportBody.h
@@ -38,13 +38,7 @@ class WEBCORE_EXPORT ReportBody : public RefCounted<ReportBody> {
     virtual ~ReportBody();
 
     virtual const String& type() const = 0;
-    ViolationReportType reportBodyType() const;
-
-protected:
-    ReportBody(ViolationReportType);
-
-private:
-    ViolationReportType m_reportBodyType;
+    virtual ViolationReportType reportBodyType() const = 0;
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/Modules/reporting/TestReportBody.cpp b/Source/WebCore/Modules/reporting/TestReportBody.cpp
@@ -34,8 +34,7 @@ namespace WebCore {
 WTF_MAKE_ISO_ALLOCATED_IMPL(TestReportBody);
 
 TestReportBody::TestReportBody(String&& message)
-    : ReportBody(ViolationReportType::Test)
-    , m_bodyMessage(WTFMove(message))
+    : m_bodyMessage(WTFMove(message))
 {
 }
 

diff --git a/Source/WebCore/Modules/reporting/TestReportBody.h b/Source/WebCore/Modules/reporting/TestReportBody.h
@@ -45,6 +45,8 @@ class TestReportBody final : public ReportBody {
 private:
     TestReportBody(String&& message);
 
+    ViolationReportType reportBodyType() const final { return ViolationReportType::Test; }
+
     const String m_bodyMessage;
 };
 

diff --git a/Source/WebCore/loader/COEPInheritenceViolationReportBody.cpp b/Source/WebCore/loader/COEPInheritenceViolationReportBody.cpp
@@ -39,8 +39,7 @@ Ref<COEPInheritenceViolationReportBody> COEPInheritenceViolationReportBody::crea
 }
 
 COEPInheritenceViolationReportBody::COEPInheritenceViolationReportBody(COEPDisposition disposition, const URL& blockedURL, const String& type)
-    : ReportBody(ViolationReportType::COEPInheritenceViolation)
-    , m_disposition(disposition)
+    : m_disposition(disposition)
     , m_blockedURL(blockedURL)
     , m_type(type)
 {

diff --git a/Source/WebCore/loader/COEPInheritenceViolationReportBody.h b/Source/WebCore/loader/COEPInheritenceViolationReportBody.h
@@ -45,6 +45,8 @@ class COEPInheritenceViolationReportBody : public ReportBody {
     friend struct IPC::ArgumentCoder<COEPInheritenceViolationReportBody, void>;
     COEPInheritenceViolationReportBody(COEPDisposition, const URL& blockedURL, const String& type);
 
+    ViolationReportType reportBodyType() const final { return ViolationReportType::COEPInheritenceViolation; }
+
     COEPDisposition m_disposition;
     URL m_blockedURL;
     String m_type;

diff --git a/Source/WebCore/loader/CORPViolationReportBody.cpp b/Source/WebCore/loader/CORPViolationReportBody.cpp
@@ -38,8 +38,7 @@ Ref<CORPViolationReportBody> CORPViolationReportBody::create(COEPDisposition dis
 }
 
 CORPViolationReportBody::CORPViolationReportBody(COEPDisposition disposition, const URL& blockedURL, FetchOptions::Destination destination)
-    : ReportBody(ViolationReportType::CORPViolation)
-    , m_disposition(disposition)
+    : m_disposition(disposition)
     , m_blockedURL(blockedURL)
     , m_destination(destination)
 {

diff --git a/Source/WebCore/loader/CORPViolationReportBody.h b/Source/WebCore/loader/CORPViolationReportBody.h
@@ -47,6 +47,8 @@ class CORPViolationReportBody : public ReportBody {
     friend struct IPC::ArgumentCoder<CORPViolationReportBody, void>;
     CORPViolationReportBody(COEPDisposition, const URL& blockedURL, FetchOptions::Destination);
 
+    ViolationReportType reportBodyType() const final { return ViolationReportType::CORPViolation; }
+
     COEPDisposition m_disposition;
     URL m_blockedURL;
     FetchOptions::Destination m_destination;

diff --git a/Source/WebCore/page/csp/CSPViolationReportBody.cpp b/Source/WebCore/page/csp/CSPViolationReportBody.cpp
@@ -39,8 +39,7 @@ using Init = SecurityPolicyViolationEventInit;
 WTF_MAKE_ISO_ALLOCATED_IMPL(CSPViolationReportBody);
 
 CSPViolationReportBody::CSPViolationReportBody(Init&& init)
-    : ReportBody(ViolationReportType::ContentSecurityPolicy)
-    , m_documentURL(WTFMove(init.documentURI))
+    : m_documentURL(WTFMove(init.documentURI))
     , m_referrer(init.referrer.isNull() ? emptyString() : WTFMove(init.referrer))
     , m_blockedURL(WTFMove(init.blockedURI))
     , m_effectiveDirective(WTFMove(init.effectiveDirective))
@@ -55,8 +54,7 @@ CSPViolationReportBody::CSPViolationReportBody(Init&& init)
 }
 
 CSPViolationReportBody::CSPViolationReportBody(String&& documentURL, String&& referrer, String&& blockedURL, String&& effectiveDirective, String&& originalPolicy, String&& sourceFile, String&& sample, SecurityPolicyViolationEventDisposition disposition, unsigned short statusCode, unsigned long lineNumber, unsigned long columnNumber)
-    : ReportBody(ViolationReportType::ContentSecurityPolicy)
-    , m_documentURL(WTFMove(documentURL))
+    : m_documentURL(WTFMove(documentURL))
     , m_referrer(WTFMove(referrer))
     , m_blockedURL(WTFMove(blockedURL))
     , m_effectiveDirective(WTFMove(effectiveDirective))

diff --git a/Source/WebCore/page/csp/CSPViolationReportBody.h b/Source/WebCore/page/csp/CSPViolationReportBody.h
@@ -64,6 +64,8 @@ class CSPViolationReportBody final : public ReportBody {
     CSPViolationReportBody(Init&&);
     CSPViolationReportBody(String&& documentURL, String&& referrer, String&& blockedURL, String&& effectiveDirective, String&& originalPolicy, String&& sourceFile, String&& sample, SecurityPolicyViolationEventDisposition, unsigned short statusCode, unsigned long lineNumber, unsigned long columnNumber);
 
+    ViolationReportType reportBodyType() const final { return ViolationReportType::ContentSecurityPolicy; }
+
     const String m_documentURL;
     const String m_referrer;
     const String m_blockedURL;

diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.serialization.in b/Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.serialization.in
@@ -31,6 +31,9 @@ class WebKit::NetworkLoadParameters {
     RefPtr<WebCore::SecurityOrigin> topOrigin;
     RefPtr<WebCore::SecurityOrigin> sourceOrigin;
     WTF::ProcessID parentPID;
+#if HAVE(AUDIT_TOKEN)
+    [NotSerialized] std::optional<audit_token_t> networkProcessAuditToken;
+#endif
     WebCore::ResourceRequest request;
 
     WebCore::ContentSniffingPolicy contentSniffingPolicy;
@@ -43,8 +46,9 @@ class WebKit::NetworkLoadParameters {
     bool isMainFrameNavigation;
     bool isMainResourceNavigationForAnyFrame;
     WebCore::ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking;
-
+    [NotSerialized] Vector<RefPtr<WebCore::BlobDataFileReference>> blobFileReferences;
     WebKit::PreconnectOnly shouldPreconnectOnly;
+    [NotSerialized] std::optional<WebKit::NetworkActivityTracker> networkActivityTracker;
     std::optional<WebKit::NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain;
     bool hadMainFrameMainResourcePrivateRelayed;
     bool allowPrivacyProxy;

diff --git a/Source/WebKit/Scripts/generate-serializers.py b/Source/WebKit/Scripts/generate-serializers.py
@@ -43,6 +43,7 @@
 # BitField - work around the need for http://wg21.link/P0572 and don't check that the serialization order matches the memory layout.
 # Nullable - check if the member is truthy before serializing.
 # Validator - additional C++ to validate the value when decoding
+# NotSerialized - member is present in structure but intentionally not serialized.
 
 class SerializedType(object):
     def __init__(self, struct_or_class, namespace, name, parent_class_name, members, condition, attributes, other_metadata=None):
@@ -117,6 +118,9 @@ def can_assert_member_order_is_correct(self):
                 return False
         return True
 
+    def serialized_members(self):
+        return list(filter(lambda member: 'NotSerialized' not in member.attributes, self.members))
+
 
 class SerializedEnum(object):
     def __init__(self, namespace, name, underlying_type, valid_values, condition, attributes):
@@ -348,20 +352,30 @@ def resolve_inheritance(serialized_types):
     return result
 
 
-def check_type_members(type):
+def check_type_members(type, checking_parent_class):
     result = []
     if type.parent_class is not None:
-        result = result + check_type_members(type.parent_class)
+        result = check_type_members(type.parent_class, True)
     for member in type.members:
         if member.condition is not None:
             result.append('#if ' + member.condition)
         result.append('    static_assert(std::is_same_v<std::remove_cvref_t<decltype(instance.' + member.name + ')>, ' + member.type + '>);')
         if member.condition is not None:
             result.append('#endif')
     if type.can_assert_member_order_is_correct():
+        # FIXME: Add this check for types with parent classes, too.
+        if type.parent_class is None and not checking_parent_class:
+            result.append('    struct ShouldBeSameSizeAs' + type.name + ' : public VirtualTableAndRefCountOverhead<std::is_polymorphic_v<' + type.namespace_and_name() + '>, ' + ('true' if type.return_ref else 'false') + '> {')
+            for member in type.members:
+                if member.condition is not None:
+                    result.append('#if ' + member.condition)
+                result.append('        ' + member.type + ' ' + member.name + (' : 1' if 'BitField' in member.attributes else '') + ';')
+                if member.condition is not None:
+                    result.append('#endif')
+            result.append('    };')
+            result.append('    static_assert(sizeof(ShouldBeSameSizeAs' + type.name + ') == sizeof(' + type.namespace_and_name() + '));')
         result.append('    static_assert(MembersInCorrectOrder<0')
-        for i in range(len(type.members)):
-            member = type.members[i]
+        for member in type.members:
             if 'BitField' in member.attributes:
                 continue
             if member.condition is not None:
@@ -377,7 +391,7 @@ def encode_type(type):
     result = []
     if type.parent_class is not None:
         result = result + encode_type(type.parent_class)
-    for member in type.members:
+    for member in type.serialized_members():
         if member.condition is not None:
             result.append('#if ' + member.condition)
         if 'Nullable' in member.attributes:
@@ -409,7 +423,7 @@ def decode_type(type):
         result.append('        return std::nullopt;')
         result.append('')
 
-    for member in type.members:
+    for member in type.serialized_members():
         if member.condition is not None:
             result.append('#if ' + member.condition)
         sanitized_variable_name = sanitize_string_for_variable_name(member.name)
@@ -502,11 +516,12 @@ def construct_type(type, indentation):
         result = result + construct_type(type.parent_class, indentation + 1)
         if len(type.members) != 0:
             result[-1] += ','
-    for i in range(len(type.members)):
-        member = type.members[i]
-        if type.members[i].condition is not None:
+    serialized_members = type.serialized_members()
+    for i in range(len(serialized_members)):
+        member = serialized_members[i]
+        if member.condition is not None:
             result.append('#if ' + member.condition)
-        result.append(indent(indentation + 1) + 'WTFMove(*' + sanitize_string_for_variable_name(member.name) + ')' + ('' if i == len(type.members) - 1 else ','))
+        result.append(indent(indentation + 1) + 'WTFMove(*' + sanitize_string_for_variable_name(member.name) + ')' + ('' if i == len(serialized_members) - 1 else ','))
         if member.condition is not None:
             result.append('#endif')
     if type.create_using or type.return_ref:
@@ -529,6 +544,35 @@ def generate_impl(serialized_types, serialized_enums, headers):
     result.append('    static constexpr bool value = firstOffset > secondOffset ? false : MembersInCorrectOrder<secondOffset, remainingOffsets...>::value;')
     result.append('};')
     result.append('')
+    result.append('template<bool, bool> struct VirtualTableAndRefCountOverhead;')
+    result.append('template<> struct VirtualTableAndRefCountOverhead<true, true> {')
+    result.append('    virtual ~VirtualTableAndRefCountOverhead() { }')
+    result.append('    unsigned refCount;')
+    result.append('#if ASSERT_ENABLED')
+    result.append('    bool m_isOwnedByMainThread;')
+    result.append('    bool m_areThreadingChecksEnabled;')
+    result.append('#endif')
+    result.append('#if CHECK_REF_COUNTED_LIFECYCLE')
+    result.append('    bool m_deletionHasBegun;')
+    result.append('    bool m_adoptionIsRequired;')
+    result.append('#endif')
+    result.append('};')
+    result.append('template<> struct VirtualTableAndRefCountOverhead<false, true> {')
+    result.append('    unsigned refCount;')
+    result.append('#if ASSERT_ENABLED')
+    result.append('    bool m_isOwnedByMainThread;')
+    result.append('    bool m_areThreadingChecksEnabled;')
+    result.append('#endif')
+    result.append('#if CHECK_REF_COUNTED_LIFECYCLE')
+    result.append('    bool m_deletionHasBegun;')
+    result.append('    bool m_adoptionIsRequired;')
+    result.append('#endif')
+    result.append('};')
+    result.append('template<> struct VirtualTableAndRefCountOverhead<true, false> {')
+    result.append('    virtual ~VirtualTableAndRefCountOverhead() { }')
+    result.append('};')
+    result.append('template<> struct VirtualTableAndRefCountOverhead<false, false> { };')
+    result.append('')
     # GCC is less generous with its interpretation of "Use of the offsetof macro with a
     # type other than a standard-layout class is conditionally-supported".
     result.append('#if COMPILER(GCC)')
@@ -564,7 +608,7 @@ def generate_impl(serialized_types, serialized_enums, headers):
             result.append('void ArgumentCoder<' + type.namespace_and_name() + '>::encode(' + encoder + '& encoder, const ' + type.namespace_and_name() + '& instance)')
             result.append('{')
             if not type.members_are_subclasses:
-                result = result + check_type_members(type)
+                result = result + check_type_members(type, False)
             result = result + encode_type(type)
             result.append('}')
         result.append('')
@@ -577,7 +621,7 @@ def generate_impl(serialized_types, serialized_enums, headers):
         if not type.members_are_subclasses:
             if type.populate_from_empty_constructor:
                 result.append('    ' + type.namespace_and_name() + ' result;')
-                for member in type.members:
+                for member in type.serialized_members():
                     if member.condition is not None:
                         result.append('#if ' + member.condition)
                     result.append('    result.' + member.name + ' = WTFMove(*' + member.name + ');')
@@ -692,15 +736,18 @@ def generate_serialized_type_info(serialized_types, serialized_enums, headers, t
             result.append('            { "std::variant<' + ', '.join([member.namespace + '::' + member.name for member in type.members]) + '>"_s, "subclasses"_s }')
             result.append('        } },')
             continue
-        for i in range(len(type.members)):
+
+        serialized_members = type.serialized_members()
+        for i in range(len(serialized_members)):
+            member = type.members[i]
             if i == 0:
                 result.append('            {')
-            if 'Nullable' in type.members[i].attributes:
-                result.append('                "std::optional<' + type.members[i].type + '>"_s,')
+            if 'Nullable' in member.attributes:
+                result.append('                "std::optional<' + member.type + '>"_s,')
             else:
-                result.append('                "' + type.members[i].type + '"_s,')
-            result.append('                "' + type.members[i].name + '"_s')
-            if i == len(type.members) - 1:
+                result.append('                "' + member.type + '"_s,')
+            result.append('                "' + member.name + '"_s')
+            if i == len(serialized_members) - 1:
                 result.append('            }')
             else:
                 result.append('            }, {')