[runtime] Key template object cache on Script

Use Script as the key for the template object cache, instead of the
SharedFunctionInfo. This is because SharedFunctionInfos can be garbage
collected and then later recompiled, which would mean that we break the
spec's expectation that the template object stays constant.

Now the association of cached template object with SharedFunctionInfo is
via the function_literal_id of the SharedFunctionInfo, stored on the
CachedTemplateObject. These are linearly searched, similar to the linear
search over slot ids.

Bug: v8:13190
Change-Id: I3f67811c16ea4cd39c99b2fa034aa7e1f03c171e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3892787Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83195}

src/objects/objects-inl.h

@@ -1135,6 +1135,9 @@ Object Object::GetSimpleHash(Object object) {
   } else if (InstanceTypeChecker::IsSharedFunctionInfo(instance_type)) {
     uint32_t hash = SharedFunctionInfo::cast(object).Hash();
     return Smi::FromInt(hash & Smi::kMaxValue);
+  } else if (InstanceTypeChecker::IsScript(instance_type)) {
+    int id = Script::cast(object).id();
+    return Smi::FromInt(ComputeUnseededHash(id) & Smi::kMaxValue);
   }
   DCHECK(object.IsJSReceiver());
   return object;

src/objects/template-objects.cc

@@ -23,6 +23,7 @@ Handle<JSArray> TemplateObjectDescription::GetTemplateObject(
 
   // Check the template weakmap to see if the template object already exists.
   Handle<EphemeronHashTable> template_weakmap;
+  Handle<Script> script(Script::cast(shared_info->script(isolate)), isolate);
 
   if (native_context->template_weakmap().IsUndefined(isolate)) {
     template_weakmap = EphemeronHashTable::New(isolate, 1);
@@ -31,11 +32,14 @@ Handle<JSArray> TemplateObjectDescription::GetTemplateObject(
     ReadOnlyRoots roots(isolate);
     template_weakmap = handle(
         EphemeronHashTable::cast(native_context->template_weakmap()), isolate);
-    Object maybe_cached_template = template_weakmap->Lookup(shared_info, hash);
+    Object maybe_cached_template =
+        template_weakmap->Lookup(isolate, script, hash);
+    int function_literal_id = shared_info->function_literal_id();
     while (!maybe_cached_template.IsTheHole(roots)) {
       CachedTemplateObject cached_template =
           CachedTemplateObject::cast(maybe_cached_template);
-      if (cached_template.slot_id() == slot_id) {
+      if (cached_template.function_literal_id() == function_literal_id &&
+          cached_template.slot_id() == slot_id) {
         return handle(cached_template.template_object(), isolate);
       }
       maybe_cached_template = cached_template.next();
@@ -73,20 +77,21 @@ Handle<JSArray> TemplateObjectDescription::GetTemplateObject(
       .ToChecked();
 
   // Insert the template object into the template weakmap.
-  Handle<HeapObject> previous_cached_templates = handle(
-      HeapObject::cast(template_weakmap->Lookup(shared_info, hash)), isolate);
+  Handle<HeapObject> previous_cached_templates =
+      handle(HeapObject::cast(template_weakmap->Lookup(script, hash)), isolate);
   Handle<CachedTemplateObject> cached_template = CachedTemplateObject::New(
-      isolate, slot_id, template_object, previous_cached_templates);
-  template_weakmap = EphemeronHashTable::Put(
-      isolate, template_weakmap, shared_info, cached_template, hash);
+      isolate, shared_info->function_literal_id(), slot_id, template_object,
+      previous_cached_templates);
+  template_weakmap = EphemeronHashTable::Put(isolate, template_weakmap, script,
+                                             cached_template, hash);
   native_context->set_template_weakmap(*template_weakmap);
 
   return template_object;
 }
 
 Handle<CachedTemplateObject> CachedTemplateObject::New(
-    Isolate* isolate, int slot_id, Handle<JSArray> template_object,
-    Handle<HeapObject> next) {
+    Isolate* isolate, int function_literal_id, int slot_id,
+    Handle<JSArray> template_object, Handle<HeapObject> next) {
   DCHECK(next->IsCachedTemplateObject() || next->IsTheHole());
   Handle<CachedTemplateObject> result_handle =
       Handle<CachedTemplateObject>::cast(isolate->factory()->NewStruct(
@@ -94,6 +99,7 @@ Handle<CachedTemplateObject> CachedTemplateObject::New(
   {
     DisallowGarbageCollection no_gc;
     auto result = *result_handle;
+    result.set_function_literal_id(function_literal_id);
     result.set_slot_id(slot_id);
     result.set_template_object(*template_object);
     result.set_next(*next);

src/objects/template-objects.h

@@ -25,7 +25,8 @@ class StructBodyDescriptor;
 class CachedTemplateObject final
     : public TorqueGeneratedCachedTemplateObject<CachedTemplateObject, Struct> {
  public:
-  static Handle<CachedTemplateObject> New(Isolate* isolate, int slot_id,
+  static Handle<CachedTemplateObject> New(Isolate* isolate,
+                                          int function_literal_id, int slot_id,
                                           Handle<JSArray> template_object,
                                           Handle<HeapObject> next);
 

src/objects/template-objects.tq

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 extern class CachedTemplateObject extends Struct {
+  function_literal_id: Smi;
   slot_id: Smi;
   template_object: JSArray;
   next: CachedTemplateObject|TheHole;

src/runtime/runtime-test.cc

@@ -846,6 +846,9 @@ RUNTIME_FUNCTION(Runtime_GetOptimizationStatus) {
   if (function->ActiveTierIsIgnition()) {
     status |= static_cast<int>(OptimizationStatus::kInterpreted);
   }
+  if (!function->is_compiled()) {
+    status |= static_cast<int>(OptimizationStatus::kIsLazy);
+  }
 
   // Additionally, detect activations of this frame on the stack, and report the
   // status of the topmost frame.
@@ -906,6 +909,20 @@ RUNTIME_FUNCTION(Runtime_FinalizeOptimization) {
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
+RUNTIME_FUNCTION(Runtime_ForceFlush) {
+  HandleScope scope(isolate);
+  if (args.length() != 1) return CrashUnlessFuzzing(isolate);
+
+  Handle<Object> function_object = args.at(0);
+  if (!function_object->IsJSFunction()) return CrashUnlessFuzzing(isolate);
+  Handle<JSFunction> function = Handle<JSFunction>::cast(function_object);
+
+  SharedFunctionInfo::DiscardCompiled(
+      isolate, handle(function->shared(isolate), isolate));
+  function->ResetIfCodeFlushed();
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
 static void ReturnNull(const v8::FunctionCallbackInfo<v8::Value>& args) {
   args.GetReturnValue().SetNull();
 }

src/runtime/runtime.h

@@ -508,6 +508,7 @@ namespace internal {
   F(EnableCodeLoggingForTesting, 0, 1)        \
   F(EnsureFeedbackVectorForFunction, 1, 1)    \
   F(FinalizeOptimization, 0, 1)               \
+  F(ForceFlush, 1, 1)                         \
   F(GetCallable, 0, 1)                        \
   F(GetInitializerFunction, 1, 1)             \
   F(GetOptimizationStatus, 1, 1)              \
@@ -939,6 +940,7 @@ enum class OptimizationStatus {
   kBaseline = 1 << 15,
   kTopmostFrameIsInterpreted = 1 << 16,
   kTopmostFrameIsBaseline = 1 << 17,
+  kIsLazy = 1 << 18,
 };
 
 }  // namespace internal

src/snapshot/context-serializer.cc

@@ -203,12 +203,11 @@ void ContextSerializer::SerializeObjectImpl(Handle<HeapObject> obj) {
 }
 
 bool ContextSerializer::ShouldBeInTheStartupObjectCache(HeapObject o) {
-  // Scripts should be referred only through shared function infos.  We can't
-  // allow them to be part of the context snapshot because they contain a
-  // unique ID, and deserializing several context snapshots containing script
-  // would cause dupes.
-  DCHECK(!o.IsScript());
-  return o.IsName() || o.IsSharedFunctionInfo() || o.IsHeapNumber() ||
+  // We can't allow scripts to be part of the context snapshot because they
+  // contain a unique ID, and deserializing several context snapshots containing
+  // script would cause dupes.
+  return o.IsName() || o.IsScript() || o.IsSharedFunctionInfo() ||
+         o.IsHeapNumber() ||
          (V8_EXTERNAL_CODE_SPACE_BOOL && o.IsCodeDataContainer()) ||
          o.IsCode() || o.IsScopeInfo() || o.IsAccessorInfo() ||
          o.IsTemplateInfo() || o.IsClassPositions() ||

test/mjsunit/mjsunit.js

@@ -188,6 +188,7 @@ var V8OptimizationStatus = {
   kBaseline: 1 << 15,
   kTopmostFrameIsInterpreted: 1 << 16,
   kTopmostFrameIsBaseline: 1 << 17,
+  kIsLazy: 1 << 18,
 };
 
 // Returns true if --lite-mode is on and we can't ever turn on optimization.
@@ -199,6 +200,9 @@ var isNeverOptimize;
 // Returns true if --always-turbofan mode is on.
 var isAlwaysOptimize;
 
+// Returns true if given function in lazily compiled.
+var isLazy;
+
 // Returns true if given function in interpreted.
 var isInterpreted;
 
@@ -762,6 +766,13 @@ var prettyPrinted;
     return (opt_status & V8OptimizationStatus.kAlwaysOptimize) !== 0;
   }
 
+  isLazy = function isLazy(fun) {
+    var opt_status = OptimizationStatus(fun, '');
+    assertTrue((opt_status & V8OptimizationStatus.kIsFunction) !== 0,
+               "not a function");
+    return (opt_status & V8OptimizationStatus.kIsLazy) !== 0;
+  }
+
   isInterpreted = function isInterpreted(fun) {
     var opt_status = OptimizationStatus(fun, "");
     assertTrue((opt_status & V8OptimizationStatus.kIsFunction) !== 0,

test/mjsunit/regress/regress-v8-13190.js

+// 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: --allow-natives-syntax --expose-gc
+// Flags: --no-sparkplug --no-maglev --no-turbofan
+// Flags: --lazy --flush-bytecode
+
+// Helper that, given a template object, simply returns it.
+function get_template_object(obj) {
+  return obj;
+}
+
+function foo_factory() {
+  // The SFI for foo is held by the bytecode array of foo_factory, so will be
+  // collected if foo_factory's bytecode is flushed.
+  return function foo() {
+    return get_template_object``
+  }
+}
+
+// Create foo in another function to avoid it leaking on the global object or
+// top-level script locals, and accidentally being kept alive.
+function get_foo_template_object() {
+  let foo = foo_factory();
+  return foo();
+}
+
+assertTrue(isLazy(foo_factory));
+
+let inital_template_object = get_foo_template_object();
+assertEquals(inital_template_object, get_foo_template_object(),
+             "Template object identity should be preserved");
+
+// Force a flush of foo_factory, so that the SharedFunctionInfo of foo can be
+// collected.
+%ForceFlush(foo_factory);
+assertTrue(isLazy(foo_factory));
+
+// Do a few more GCs to allow weak maps to be cleared.
+gc();
+gc();
+gc();
+
+// Flushing foo_factory and GCing foo should not affect the persisted reference
+// identity of the template object inside foo.
+assertSame(inital_template_object, get_foo_template_object(),
+           "Template object identity should be preserved");