From 0dbe725713da4bb5f5e8b2a67487828d34777d0e Mon Sep 17 00:00:00 2001
From: jameslahm <wangao.james@bytedance.com>
Date: Mon, 25 Apr 2022 14:27:26 +0800
Subject: [PATCH] [web-snapshot] Fix snapshot scope info in Context
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- In DeserializeContext, scope info local values
snapshot is in order of `name,value,name,value`,
and we should ReadValue after ReadString.

- Support non-inlined ScopeInfo locals, use
NameToIndexHashTable to serialize and deserialize
scope info local values when its local count is
more than kScopeInfoMaxInlinedLocalNamesSize.

Bug: v8:11525, v8:12820
Change-Id: I6ea2c498b594bed7ba8ca5be6af2ab9f0d39aa2b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3600531
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: 王澳 <wangao.james@bytedance.com>
Cr-Commit-Position: refs/heads/main@{#80130}
---
 src/web-snapshot/web-snapshot.cc            | 115 ++++---
 src/web-snapshot/web-snapshot.h             |   3 +-
 test/mjsunit/web-snapshot/web-snapshot-4.js | 337 ++++++++++++++++++++
 3 files changed, 411 insertions(+), 44 deletions(-)
 create mode 100644 test/mjsunit/web-snapshot/web-snapshot-4.js

diff --git a/src/web-snapshot/web-snapshot.cc b/src/web-snapshot/web-snapshot.cc
index 1b0a07e313..29ff1d648b 100644
--- a/src/web-snapshot/web-snapshot.cc
+++ b/src/web-snapshot/web-snapshot.cc
@@ -788,14 +788,10 @@ void WebSnapshotSerializer::DiscoverContext(Handle<Context> context) {
   contexts_ = ArrayList::Add(isolate_, contexts_, context);
 
   Handle<ScopeInfo> scope_info = handle(context->scope_info(), isolate_);
-  int count = scope_info->ContextLocalCount();
-
-  for (int i = 0; i < count; ++i) {
-    // TODO(v8:11525): support parameters
-    // TODO(v8:11525): distinguish variable modes
-    Handle<String> name(scope_info->context_local_names(i), isolate_);
-    DiscoverString(name);
-    Object value = context->get(scope_info->ContextHeaderLength() + i);
+  for (auto it : ScopeInfo::IterateLocalNames(scope_info)) {
+    DiscoverString(handle(it->name(), isolate_));
+    Object value =
+        context->get(scope_info->ContextHeaderLength() + it->index());
     if (!value.IsHeapObject()) continue;
     discovery_queue_.push(handle(HeapObject::cast(value), isolate_));
   }
@@ -807,6 +803,14 @@ void WebSnapshotSerializer::DiscoverContext(Handle<Context> context) {
 }
 
 void WebSnapshotSerializer::DiscoverSource(Handle<JSFunction> function) {
+  // Function may not have source code, e.g. we discover source for a builtin
+  // function. In SerializeFunctionInfo, we also check if the function has
+  // source code, and we throw the same error here if the function doesn't
+  // have source code to be consistent with SerializeFunctionInfo.
+  if (!function->shared().HasSourceCode()) {
+    Throw("Function without source code");
+    return;
+  }
   source_intervals_.emplace(function->shared().StartPosition(),
                             function->shared().EndPosition());
   Handle<String> function_script_source =
@@ -945,13 +949,13 @@ void WebSnapshotSerializer::SerializeContext(Handle<Context> context) {
   int count = scope_info->ContextLocalCount();
   context_serializer_.WriteUint32(count);
 
-  for (int i = 0; i < count; ++i) {
+  for (auto it : ScopeInfo::IterateLocalNames(scope_info)) {
     // TODO(v8:11525): support parameters
     // TODO(v8:11525): distinguish variable modes
-    Handle<String> name(scope_info->context_local_names(i), isolate_);
-    WriteStringId(name, context_serializer_);
-    Handle<Object> value(context->get(scope_info->ContextHeaderLength() + i),
-                         isolate_);
+    WriteStringId(handle(it->name(), isolate_), context_serializer_);
+    Handle<Object> value(
+        context->get(scope_info->ContextHeaderLength() + it->index()),
+        isolate_);
     WriteValue(value, context_serializer_);
   }
 }
@@ -1624,10 +1628,12 @@ void WebSnapshotDeserializer::DeserializeContexts() {
       Throw("Malformed context");
       return;
     }
+    const bool has_inlined_local_names =
+        variable_count < kScopeInfoMaxInlinedLocalNamesSize;
     // TODO(v8:11525): Enforce upper limit for variable count.
-    Handle<ScopeInfo> scope_info =
-        CreateScopeInfo(variable_count, parent_context_id > 0,
-                        static_cast<ContextType>(context_type));
+    Handle<ScopeInfo> scope_info = CreateScopeInfo(
+        variable_count, parent_context_id > 0,
+        static_cast<ContextType>(context_type), has_inlined_local_names);
 
     Handle<Context> parent_context;
     if (parent_context_id > 0) {
@@ -1638,29 +1644,6 @@ void WebSnapshotDeserializer::DeserializeContexts() {
       parent_context = handle(isolate_->context(), isolate_);
     }
 
-    const int context_local_base = ScopeInfo::kVariablePartIndex;
-    const int context_local_info_base = context_local_base + variable_count;
-    for (int variable_index = 0;
-         variable_index < static_cast<int>(variable_count); ++variable_index) {
-      {
-        String name = ReadString(true);
-        scope_info->set(context_local_base + variable_index, name);
-      }
-
-      // TODO(v8:11525): Support variable modes etc.
-      uint32_t info =
-          ScopeInfo::VariableModeBits::encode(VariableMode::kLet) |
-          ScopeInfo::InitFlagBit::encode(
-              InitializationFlag::kNeedsInitialization) |
-          ScopeInfo::MaybeAssignedFlagBit::encode(
-              MaybeAssignedFlag::kMaybeAssigned) |
-          ScopeInfo::ParameterNumberBits::encode(
-              ScopeInfo::ParameterNumberBits::kMax) |
-          ScopeInfo::IsStaticFlagBit::encode(IsStaticFlag::kNotStatic);
-      scope_info->set(context_local_info_base + variable_index,
-                      Smi::FromInt(info));
-    }
-
     // Allocate the FunctionContext after setting up the ScopeInfo to avoid
     // pointing to a ScopeInfo which is not set up yet.
     Handle<Context> context;
@@ -1675,10 +1658,46 @@ void WebSnapshotDeserializer::DeserializeContexts() {
         Throw("Unsupported context type");
         return;
     }
-    int context_header_length = scope_info->ContextHeaderLength();
+
+    const int local_names_container_size =
+        has_inlined_local_names ? variable_count : 1;
+    const int context_local_base = ScopeInfo::kVariablePartIndex;
+    const int context_local_info_base =
+        context_local_base + local_names_container_size;
+
     for (int variable_index = 0;
          variable_index < static_cast<int>(variable_count); ++variable_index) {
-      int context_index = context_header_length + variable_index;
+      {
+        String name = ReadString(true);
+        if (has_inlined_local_names) {
+          scope_info->set(context_local_base + variable_index, name);
+        } else {
+          Handle<NameToIndexHashTable> local_names_hashtable(
+              scope_info->context_local_names_hashtable(), isolate_);
+
+          Handle<NameToIndexHashTable> new_table =
+              NameToIndexHashTable::Add(isolate_, local_names_hashtable,
+                                        handle(name, isolate_), variable_index);
+          // The hash table didn't grow, since it was preallocated to
+          // be large enough in CreateScopeInfo.
+          DCHECK_EQ(*new_table, *local_names_hashtable);
+          USE(new_table);
+        }
+      }
+      // TODO(v8:11525): Support variable modes etc.
+      uint32_t info =
+          ScopeInfo::VariableModeBits::encode(VariableMode::kLet) |
+          ScopeInfo::InitFlagBit::encode(
+              InitializationFlag::kNeedsInitialization) |
+          ScopeInfo::MaybeAssignedFlagBit::encode(
+              MaybeAssignedFlag::kMaybeAssigned) |
+          ScopeInfo::ParameterNumberBits::encode(
+              ScopeInfo::ParameterNumberBits::kMax) |
+          ScopeInfo::IsStaticFlagBit::encode(IsStaticFlag::kNotStatic);
+      scope_info->set(context_local_info_base + variable_index,
+                      Smi::FromInt(info));
+
+      int context_index = scope_info->ContextHeaderLength() + variable_index;
       Object value = ReadValue(context, context_index);
       context->set(context_index, value);
     }
@@ -1687,7 +1706,8 @@ void WebSnapshotDeserializer::DeserializeContexts() {
 }
 
 Handle<ScopeInfo> WebSnapshotDeserializer::CreateScopeInfo(
-    uint32_t variable_count, bool has_parent, ContextType context_type) {
+    uint32_t variable_count, bool has_parent, ContextType context_type,
+    bool has_inlined_local_names) {
   // TODO(v8:11525): Decide how to handle language modes. (The code below sets
   // the language mode as strict.)
   // TODO(v8:11525): Support (context-allocating) receiver.
@@ -1732,12 +1752,16 @@ Handle<ScopeInfo> WebSnapshotDeserializer::CreateScopeInfo(
       Throw("Unsupported context type");
   }
   flags |= ScopeInfo::ScopeTypeBits::encode(scope_type);
+  const int local_names_container_size =
+      has_inlined_local_names ? variable_count : 1;
   const int length = ScopeInfo::kVariablePartIndex +
                      (ScopeInfo::NeedsPositionInfo(scope_type)
                           ? ScopeInfo::kPositionInfoEntries
                           : 0) +
-                     (has_parent ? 1 : 0) + 2 * variable_count;
+                     (has_parent ? 1 : 0) + local_names_container_size +
+                     variable_count;
   Handle<ScopeInfo> scope_info = factory()->NewScopeInfo(length);
+  Handle<NameToIndexHashTable> local_names_hashtable;
   {
     DisallowGarbageCollection no_gc;
     ScopeInfo raw = *scope_info;
@@ -1752,6 +1776,11 @@ Handle<ScopeInfo> WebSnapshotDeserializer::CreateScopeInfo(
       raw.SetPositionInfo(0, 0);
     }
   }
+  if (!has_inlined_local_names) {
+    local_names_hashtable = NameToIndexHashTable::New(isolate_, variable_count,
+                                                      AllocationType::kOld);
+    scope_info->set_context_local_names_hashtable(*local_names_hashtable);
+  }
   return scope_info;
 }
 
diff --git a/src/web-snapshot/web-snapshot.h b/src/web-snapshot/web-snapshot.h
index 0e18733e3a..9f380f2cd1 100644
--- a/src/web-snapshot/web-snapshot.h
+++ b/src/web-snapshot/web-snapshot.h
@@ -315,7 +315,8 @@ class V8_EXPORT WebSnapshotDeserializer
   void DeserializeMaps();
   void DeserializeContexts();
   Handle<ScopeInfo> CreateScopeInfo(uint32_t variable_count, bool has_parent,
-                                    ContextType context_type);
+                                    ContextType context_type,
+                                    bool has_inlined_local_names);
   Handle<JSFunction> CreateJSFunction(int index, uint32_t start,
                                       uint32_t length, uint32_t parameter_count,
                                       uint32_t flags, uint32_t context_id);
diff --git a/test/mjsunit/web-snapshot/web-snapshot-4.js b/test/mjsunit/web-snapshot/web-snapshot-4.js
new file mode 100644
index 0000000000..1efd40a338
--- /dev/null
+++ b/test/mjsunit/web-snapshot/web-snapshot-4.js
@@ -0,0 +1,337 @@
+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --experimental-d8-web-snapshot-api --allow-natives-syntax
+
+'use strict';
+
+d8.file.execute('test/mjsunit/web-snapshot/web-snapshot-helpers.js');
+
+(function TestNonInlinedScopeInfoInContext() {
+  function createObjects() {
+    globalThis.bar = (function() {
+      let a1 = 1;
+      let a2 = 1;
+      let a3 = 1;
+      let a4 = 1;
+      let a5 = 1;
+      let a6 = 1;
+      let a7 = 1;
+      let a8 = 1;
+      let a9 = 1;
+      let a10 = 1;
+      let a11 = 1;
+      let a12 = 1;
+      let a13 = 1;
+      let a14 = 1;
+      let a15 = 1;
+      let a16 = 1;
+      let a17 = 1;
+      let a18 = 1;
+      let a19 = 1;
+      let a20 = 1;
+      let a21 = 1;
+      let a22 = 1;
+      let a23 = 1;
+      let a24 = 1;
+      let a25 = 1;
+      let a26 = 1;
+      let a27 = 1;
+      let a28 = 1;
+      let a29 = 1;
+      let a30 = 1;
+      let a31 = 1;
+      let a32 = 1;
+      let a33 = 1;
+      let a34 = 1;
+      let a35 = 1;
+      let a36 = 1;
+      let a37 = 1;
+      let a38 = 1;
+      let a39 = 1;
+      let a40 = 1;
+      let a41 = 1;
+      let a42 = 1;
+      let a43 = 1;
+      let a44 = 1;
+      let a45 = 1;
+      let a46 = 1;
+      let a47 = 1;
+      let a48 = 1;
+      let a49 = 1;
+      let a50 = 1;
+      let a51 = 1;
+      let a52 = 1;
+      let a53 = 1;
+      let a54 = 1;
+      let a55 = 1;
+      let a56 = 1;
+      let a57 = 1;
+      let a58 = 1;
+      let a59 = 1;
+      let a60 = 1;
+      let a61 = 1;
+      let a62 = 1;
+      let a63 = 1;
+      let a64 = 1;
+      let a65 = 1;
+      let a66 = 1;
+      let a67 = 1;
+      let a68 = 1;
+      let a69 = 1;
+      let a70 = 1;
+      let a71 = 1;
+      let a72 = 1;
+      let a73 = 1;
+      let a74 = 1;
+      let a75 = 1;
+      function inner1() {
+        return a1;
+      }
+      function inner2() {
+        return a2;
+      }
+      function inner3() {
+        return a3;
+      }
+      function inner4() {
+        return a4;
+      }
+      function inner5() {
+        return a5;
+      }
+      function inner6() {
+        return a6;
+      }
+      function inner7() {
+        return a7;
+      }
+      function inner8() {
+        return a8;
+      }
+      function inner9() {
+        return a9;
+      }
+      function inner10() {
+        return a10;
+      }
+      function inner11() {
+        return a11;
+      }
+      function inner12() {
+        return a12;
+      }
+      function inner13() {
+        return a13;
+      }
+      function inner14() {
+        return a14;
+      }
+      function inner15() {
+        return a15;
+      }
+      function inner16() {
+        return a16;
+      }
+      function inner17() {
+        return a17;
+      }
+      function inner18() {
+        return a18;
+      }
+      function inner19() {
+        return a19;
+      }
+      function inner20() {
+        return a20;
+      }
+      function inner21() {
+        return a21;
+      }
+      function inner22() {
+        return a22;
+      }
+      function inner23() {
+        return a23;
+      }
+      function inner24() {
+        return a24;
+      }
+      function inner25() {
+        return a25;
+      }
+      function inner26() {
+        return a26;
+      }
+      function inner27() {
+        return a27;
+      }
+      function inner28() {
+        return a28;
+      }
+      function inner29() {
+        return a29;
+      }
+      function inner30() {
+        return a30;
+      }
+      function inner31() {
+        return a31;
+      }
+      function inner32() {
+        return a32;
+      }
+      function inner33() {
+        return a33;
+      }
+      function inner34() {
+        return a34;
+      }
+      function inner35() {
+        return a35;
+      }
+      function inner36() {
+        return a36;
+      }
+      function inner37() {
+        return a37;
+      }
+      function inner38() {
+        return a38;
+      }
+      function inner39() {
+        return a39;
+      }
+      function inner40() {
+        return a40;
+      }
+      function inner41() {
+        return a41;
+      }
+      function inner42() {
+        return a42;
+      }
+      function inner43() {
+        return a43;
+      }
+      function inner44() {
+        return a44;
+      }
+      function inner45() {
+        return a45;
+      }
+      function inner46() {
+        return a46;
+      }
+      function inner47() {
+        return a47;
+      }
+      function inner48() {
+        return a48;
+      }
+      function inner49() {
+        return a49;
+      }
+      function inner50() {
+        return a50;
+      }
+      function inner51() {
+        return a51;
+      }
+      function inner52() {
+        return a52;
+      }
+      function inner53() {
+        return a53;
+      }
+      function inner54() {
+        return a54;
+      }
+      function inner55() {
+        return a55;
+      }
+      function inner56() {
+        return a56;
+      }
+      function inner57() {
+        return a57;
+      }
+      function inner58() {
+        return a58;
+      }
+      function inner59() {
+        return a59;
+      }
+      function inner60() {
+        return a60;
+      }
+      function inner61() {
+        return a61;
+      }
+      function inner62() {
+        return a62;
+      }
+      function inner63() {
+        return a63;
+      }
+      function inner64() {
+        return a64;
+      }
+      function inner65() {
+        return a65;
+      }
+      function inner66() {
+        return a66;
+      }
+      function inner67() {
+        return a67;
+      }
+      function inner68() {
+        return a68;
+      }
+      function inner69() {
+        return a69;
+      }
+      function inner70() {
+        return a70;
+      }
+      function inner71() {
+        return a71;
+      }
+      function inner72() {
+        return a72;
+      }
+      function inner73() {
+        return a73;
+      }
+      function inner74() {
+        return a74;
+      }
+      function inner75() {
+        return a75;
+      }
+      return inner1;
+    })()
+  }
+  const {bar} = takeAndUseWebSnapshot(createObjects, ['bar']);
+  assertEquals(bar(), 1);
+})();
+
+(function TestMoreThanOneScopeLocalInContext() {
+  function createObjects() {
+    globalThis.foo = (function() {
+      let result = 'bar';
+      let a = '1';
+      function inner() {
+        return result;
+      }
+      function inner2() {
+        return a;
+      }
+      return inner;
+    })();
+  }
+  const {foo} = takeAndUseWebSnapshot(createObjects, ['foo']);
+  assertEquals('bar', foo());
+})();