[webkit-changes] [WebKit/WebKit] 010c6b: CloneSerializer/Deserializer's objectPool should m...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 010c6bdfb0cde0485d31f0260ab9a046fa9b8567
      https://github.com/WebKit/WebKit/commit/010c6bdfb0cde0485d31f0260ab9a046fa9b8567
  Author: Mark Lam <mark.lam at apple.com>
  Date:   2024-01-26 (Fri, 26 Jan 2024)

  Changed paths:
    A LayoutTests/js/structuredClone/structured-clone-validation-with-big-int-expected.txt
    A LayoutTests/js/structuredClone/structured-clone-validation-with-big-int.html
    M Source/JavaScriptCore/runtime/OptionsList.h
    M Source/WebCore/bindings/js/SerializedScriptValue.cpp

  Log Message:
  -----------
  CloneSerializer/Deserializer's objectPool should match.
https://bugs.webkit.org/show_bug.cgi?id=265975
rdar://118868470

Reviewed by Chris Dumez and Sihui Liu

CloneSerializer and CloneDeserializer uses m_gcBuffer for multiple purposes:
1. As an object pool that ObjectReferenceTag refers back to i.e. the order of its
   entries need to be consistent between CloneSerializer and CloneDeserializer.
2. As a keep alive buffer to protect some objects use in the serialization effort.

Purpose (2) conflicts with purpose (1), which can lead to bugs.  This patch disambiguates
between these 2 purposes by introducing m_objectPool for purpose (1), and m_keepAliveBuffer
for purpose (2).

Changes made:
1. Renamed m_objectPool to m_objectPoolMap.
2. Renamed m_gcBuffer to m_objectPool: for tracking the list of objects that ObjectReferenceTag
   can refer to.
3. Added m_keepAliveBuffer to CloneSerializer: for keeping miscellaneous objects alive from the GC.

4. Renamed some method names for clarity:
       CloneSerializer::checkForDuplicate --> CloneSerializer::writeObjectReferenceIfDupe
       CloneSerializer::recordObject --> CloneSerializer::addToObjectPool
       CloneSerializer::startObjectInternal --> CloneSerializer::addToObjectPoolIfNotDupe

5. Used CloneSerializer::addToObjectPoolIfNotDupe instead of the following:
       CloneSerializer::startObject
       CloneSerializer::startArray
       CloneSerializer::startSet
       CloneSerializer::startMap

   The clients of addToObjectPoolIfNotDupe now indicate what object types (indicated by their
   SerializationTags) they are adding.  This makes it easier to compare the serialization and
   deserialization code and make sure that they are equivalent.

   This enables us to audit the type of object being added and provide a sanity check that
   it's also added on the deserializer side.

6. Introduced CloneDeserializer::addToObjectPool() so that we can tag which object type (as
   indicated by its SerializationTag) we're adding to the m_objectPool (instead of calling
   appendWithCrashOnOverflow() on it directly to add objects).

   This enables us to audit the type of object being added and provide a sanity check that
   it's also added on the serializer side.

7. Removed 3 calls to m_gcBuffer.appendWithCrashOnOverflow in the BigIntTag case in
   CloneDeserializer::readBigInt().  This was a bug.

8. Removed the following calls to m_gcBuffer.appendWithCrashOnOverflow in CloneSerializer::serialize:
   a. redundant adding of the JSMap object.  It was already added by startMap(), now addToObjectPoolIfNotDupe().
   b. keep alive of the JSMapIterator object.  It does not need to be in m_objectPool.
   c. keep alive of a map entry value.
   d. redundant adding of the JSSet object.  It was already added by startSet(), now addToObjectPoolIfNotDupe().
   e. keep alive of the JSSetIterator object.  It does not need to be in m_objectPool.

   These were bugs.

9. Renamed the mapObjectStartState and setObjectStartState labels in the deserializer to match the
   mapStartState and setStartState labels in the serializer.  This makes it easier to check the
   equivalency of the operations in the two.

10. Added a validator (see validateSerializedResult()) in the serializer.

   The validator works by running a deserialization pass on the output of the serializer.
   After that, it compares the m_objectPoolTags of the 2 passes, and their entries should
   match.  This ensures that the serializer and deserializer will catch any bugs in the
   serialization / deserialization order of objects.

   a. The validator is only enabled on Debug builds (not built in on Release builds).
   b. The validator is only run when JSC::Options::validateSerializedValue() is true.
   c. The validator is only run when the object graph to be serialized and deserialized
      does not contain any complicated / complex objects.  "complex" here means that
      serialization of such objects cannot be validated this way.
   d. The validator is only run when both serialization and deserialization passes succeeds.

   With this validator, we can now fuzz the serializer / deserializer by creating HTML tests cases
   where we build an object graph and call structureClone() on it.  The HTML test will need to have
   the following comment on its 1st line:

       <!-- webkit-test-runner [ jscOptions=--validateSerializedValue=true ] -->

   This will enable the validator when structured cloning is done on that object graph.

* LayoutTests/js/structuredClone/structured-clone-validation-with-big-int-expected.txt: Added.
* LayoutTests/js/structuredClone/structured-clone-validation-with-big-int.html: Added.
* Source/JavaScriptCore/runtime/OptionsList.h:
* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::name):
(WTF::printInternal):
(WebCore::canBeAddedToObjectPool):
(WebCore::CloneBase::objectPoolTags const):
(WebCore::CloneBase::appendObjectPoolTag):
(WebCore::CloneSerializer::serialize):
(WebCore::CloneSerializer::sawComplexCase):
(WebCore::CloneSerializer::didSeeComplexCases const):
(WebCore::CloneSerializer::fillTransferMap):
(WebCore::CloneSerializer::writeObjectReferenceIfDupe):
(WebCore::CloneSerializer::addToObjectPool):
(WebCore::CloneSerializer::addToObjectPoolIfNotDupe):
(WebCore::CloneSerializer::dumpStringObject):
(WebCore::CloneSerializer::dumpArrayBufferView):
(WebCore::CloneSerializer::dumpIfTerminal):
(WebCore::CloneSerializer::writeObjectIndex):
(WebCore::CloneDeserializer::addToObjectPool):
(WebCore::CloneDeserializer::readBigInt):
(WebCore::CloneDeserializer::readTerminal):
(WebCore::CloneDeserializer::deserialize):
(WebCore::validateSerializedResult):
(WebCore::CloneSerializer::checkForDuplicate): Deleted.
(WebCore::CloneSerializer::recordObject): Deleted.
(WebCore::CloneSerializer::startObjectInternal): Deleted.
(WebCore::CloneSerializer::startObject): Deleted.
(WebCore::CloneSerializer::startArray): Deleted.
(WebCore::CloneSerializer::startSet): Deleted.
(WebCore::CloneSerializer::startMap): Deleted.

Originally-landed-as: 267815.623 at safari-7617-branch (430d474531c0). rdar://121480622
Canonical link: https://commits.webkit.org/273557@main