[compiler] Make ContextRef never-serialized

Also delete undefined ContextRef methods and make
Context::set_previous private (it is only used when
creating a new context).

Bug: v8:7790
Change-Id: I25a701f317f0f4e82432f7537eec1d63c5ef63f4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2886860
Auto-Submit: Georg Neis <neis@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74521}

src/compiler/heap-refs.cc

@@ -819,7 +819,7 @@ ObjectData* ContextData::previous(JSHeapBroker* broker,
 
 ObjectData* ContextData::GetSlot(JSHeapBroker* broker, int index,
                                  SerializationPolicy policy) {
-  CHECK_GE(index, 0);
+  DCHECK_GE(index, 0);
   auto search = slots_.find(index);
   if (search != slots_.end()) {
     return search->second;
@@ -2204,16 +2204,8 @@ class CodeData : public HeapObjectData {
 HEAP_BROKER_OBJECT_LIST(DEFINE_IS)
 #undef DEFINE_IS
 
-// TODO(solanes, v8:10866): Remove support for kNeverSerialized objects here
-// once broker()->is_concurrent_inlining() is removed.
-// AsFoo() methods for NeverSerialized objects should only be used with direct
-// heap access off.
 #define DEFINE_AS(Name, Kind)                                      \
   Name##Data* ObjectData::As##Name() {                             \
-    DCHECK_IMPLIES(Kind == RefSerializationKind::kNeverSerialized, \
-                   !broker()->is_concurrent_inlining());           \
-    DCHECK_IMPLIES(Kind == RefSerializationKind::kNeverSerialized, \
-                   kind_ == kSerializedHeapObject);                \
     CHECK(Is##Name());                                             \
     CHECK(kind_ == kSerializedHeapObject ||                        \
           kind_ == kBackgroundSerializedHeapObject);               \
@@ -2518,16 +2510,15 @@ ContextRef ContextRef::previous(size_t* depth,
 
 base::Optional<ObjectRef> ContextRef::get(int index,
                                           SerializationPolicy policy) const {
+  CHECK_LE(0, index);
   if (data_->should_access_heap()) {
-    Handle<Object> value(object()->get(index), broker()->isolate());
-    return ObjectRef(broker(), value);
+    if (index >= object()->length()) return {};
+    return TryMakeRef(broker(), object()->get(index));
   }
   ObjectData* optional_slot =
       data()->AsContext()->GetSlot(broker(), index, policy);
-  if (optional_slot != nullptr) {
-    return ObjectRef(broker(), optional_slot);
-  }
-  return base::nullopt;
+  if (optional_slot == nullptr) return {};
+  return ObjectRef(broker(), optional_slot);
 }
 
 SourceTextModuleRef ContextRef::GetModule(SerializationPolicy policy) const {

src/compiler/heap-refs.h

@@ -117,7 +117,7 @@ enum class RefSerializationKind {
   V(CallHandlerInfo, RefSerializationKind::kNeverSerialized)              \
   V(Cell, RefSerializationKind::kNeverSerialized)                         \
   V(Code, RefSerializationKind::kNeverSerialized)                         \
-  V(Context, RefSerializationKind::kSerialized)                           \
+  V(Context, RefSerializationKind::kNeverSerialized)                      \
   V(DescriptorArray, RefSerializationKind::kNeverSerialized)              \
   V(FeedbackCell, RefSerializationKind::kNeverSerialized)                 \
   V(FeedbackVector, RefSerializationKind::kNeverSerialized)               \
@@ -445,9 +445,9 @@ class ContextRef : public HeapObjectRef {
   Handle<Context> object() const;
 
   // {previous} decrements {depth} by 1 for each previous link successfully
-  // followed. If {depth} != 0 on function return, then it only got
-  // partway to the desired depth. If {serialize} is true, then
-  // {previous} will cache its findings.
+  // followed. If {depth} != 0 on function return, then it only got partway to
+  // the desired depth. If {serialize} is true, then {previous} will cache its
+  // findings (unless concurrent inlining is enabled).
   ContextRef previous(size_t* depth,
                       SerializationPolicy policy =
                           SerializationPolicy::kAssumeSerialized) const;
@@ -458,11 +458,6 @@ class ContextRef : public HeapObjectRef {
                      SerializationPolicy::kAssumeSerialized) const;
 
   SourceTextModuleRef GetModule(SerializationPolicy policy) const;
-
-  // We only serialize the ScopeInfo if certain Promise
-  // builtins are called.
-  void SerializeScopeInfo();
-  base::Optional<ScopeInfoRef> scope_info() const;
 };
 
 #define BROKER_COMPULSORY_NATIVE_CONTEXT_FIELDS(V) \

src/heap/factory.cc

@@ -1080,7 +1080,7 @@ Handle<NativeContext> Factory::NewNativeContext() {
   // The ExternalPointerTable is a C++ object.
   context.AllocateExternalPointerEntries(isolate());
   context.set_scope_info(*native_scope_info());
-  context.set_previous(Context::unchecked_cast(Smi::zero()));
+  context.set_previous(Context());
   context.set_extension(*undefined_value());
   context.set_errors_thrown(Smi::zero());
   context.set_math_random_index(Smi::zero());

src/init/bootstrapper.cc

@@ -1480,7 +1480,6 @@ static void InstallError(
 void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
                                Handle<JSFunction> empty_function) {
   // --- N a t i v e   C o n t e x t ---
-  native_context()->set_previous(Context());
   // Set extension and global object.
   native_context()->set_extension(*global_object);
   // Security setup: Set the security token of the native context to the global

src/objects/contexts-inl.h

@@ -61,10 +61,8 @@ V8_INLINE Object Context::get(int index) const {
 V8_INLINE Object Context::get(PtrComprCageBase cage_base, int index) const {
   return elements(cage_base, index, kRelaxedLoad);
 }
-V8_INLINE void Context::set(int index, Object value) {
-  set_elements(index, value, kRelaxedStore);
-}
 V8_INLINE void Context::set(int index, Object value, WriteBarrierMode mode) {
+  DCHECK_NE(index, PREVIOUS_INDEX);
   set_elements(index, value, kRelaxedStore, mode);
 }
 
@@ -99,7 +97,7 @@ Context Context::previous() {
   return Context::unchecked_cast(result);
 }
 void Context::set_previous(Context context, WriteBarrierMode mode) {
-  set(PREVIOUS_INDEX, context, mode);
+  set_elements(PREVIOUS_INDEX, context, kRelaxedStore, mode);
 }
 
 Object Context::next_context_link() { return get(Context::NEXT_CONTEXT_LINK); }

src/objects/contexts.h

@@ -447,9 +447,8 @@ class Context : public TorqueGeneratedContext<Context, HeapObject> {
   // Setter and getter for elements.
   V8_INLINE Object get(int index) const;
   V8_INLINE Object get(PtrComprCageBase cage_base, int index) const;
-  V8_INLINE void set(int index, Object value);
-  // Setter with explicit barrier mode.
-  V8_INLINE void set(int index, Object value, WriteBarrierMode mode);
+  V8_INLINE void set(int index, Object value,
+                     WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
   // Setter and getter with synchronization semantics.
   V8_INLINE Object synchronized_get(int index) const;
   V8_INLINE Object synchronized_get(PtrComprCageBase cage_base,
@@ -547,8 +546,6 @@ class Context : public TorqueGeneratedContext<Context, HeapObject> {
 
   inline Object unchecked_previous();
   inline Context previous();
-  inline void set_previous(Context context,
-                           WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
 
   inline Object next_context_link();
 
@@ -662,6 +659,10 @@ class Context : public TorqueGeneratedContext<Context, HeapObject> {
   static bool IsBootstrappingOrValidParentContext(Object object, Context kid);
 #endif
 
+  friend class Factory;
+  inline void set_previous(Context context,
+                           WriteBarrierMode mode = UPDATE_WRITE_BARRIER);
+
   TQ_OBJECT_CONSTRUCTORS(Context)
 };
 

src/objects/scope-info.h

@@ -224,7 +224,7 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
   template <typename IsolateT>
   static Handle<ScopeInfo> Create(IsolateT* isolate, Zone* zone, Scope* scope,
                                   MaybeHandle<ScopeInfo> outer_scope);
-  static Handle<ScopeInfo> CreateForWithScope(
+  V8_EXPORT_PRIVATE static Handle<ScopeInfo> CreateForWithScope(
       Isolate* isolate, MaybeHandle<ScopeInfo> outer_scope);
   V8_EXPORT_PRIVATE static Handle<ScopeInfo> CreateForEmptyFunction(
       Isolate* isolate);

test/cctest/compiler/test-js-context-specialization.cc

@@ -39,6 +39,7 @@ class ContextSpecializationTester : public HandleAndZoneScope {
               MaybeHandle<JSFunction>()) {}
 
   JSContextSpecialization* spec() { return &spec_; }
+  Isolate* isolate() { return main_isolate(); }
   Factory* factory() { return main_isolate()->factory(); }
   CommonOperatorBuilder* common() { return &common_; }
   JSOperatorBuilder* javascript() { return &javascript_; }
@@ -111,7 +112,16 @@ void ContextSpecializationTester::CheckContextInputAndDepthChanges(
   CHECK_EQ(new_access.immutable(), access.immutable());
 }
 
-static const int slot_index = Context::PREVIOUS_INDEX;
+namespace {
+Handle<Context> NewContextForTesting(Isolate* isolate,
+                                     Handle<Context> previous) {
+  Handle<ScopeInfo> scope_info = ScopeInfo::CreateForWithScope(isolate, {});
+  Handle<JSObject> extension = isolate->factory()->NewJSObjectWithNullProto();
+  return isolate->factory()->NewWithContext(previous, scope_info, extension);
+}
+}  // namespace
+
+static const int slot_index = 5;
 
 TEST(ReduceJSLoadContext0) {
   ContextSpecializationTester t(Nothing<OuterContext>());
@@ -121,12 +131,10 @@ TEST(ReduceJSLoadContext0) {
 
   // Make a context and initialize it a bit for this test.
   Handle<Context> native = t.factory()->NewNativeContext();
-  Handle<Context> subcontext1 = t.factory()->NewNativeContext();
-  Handle<Context> subcontext2 = t.factory()->NewNativeContext();
-  subcontext2->set_previous(*subcontext1);
-  subcontext1->set_previous(*native);
+  Handle<Context> subcontext1 = NewContextForTesting(t.isolate(), native);
+  Handle<Context> subcontext2 = NewContextForTesting(t.isolate(), subcontext1);
   Handle<Object> expected = t.factory()->InternalizeUtf8String("gboy!");
-  const int slot = Context::PREVIOUS_INDEX;
+  const int slot = 5;
   native->set(slot, *expected);
 
   Node* const_context = t.jsgraph()->Constant(ObjectRef(t.broker(), native));
@@ -271,8 +279,8 @@ TEST(ReduceJSLoadContext2) {
   Handle<HeapObject> slot_value1 = t.factory()->InternalizeUtf8String("1");
 
   Handle<Context> context_object0 = t.factory()->NewNativeContext();
-  Handle<Context> context_object1 = t.factory()->NewNativeContext();
-  context_object1->set_previous(*context_object0);
+  Handle<Context> context_object1 =
+      NewContextForTesting(t.isolate(), context_object0);
   context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
   context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
 
@@ -342,14 +350,15 @@ TEST(ReduceJSLoadContext3) {
   // expectations are the same as in ReduceJSLoadContext2.
 
   HandleAndZoneScope handle_zone_scope;
-  auto factory = handle_zone_scope.main_isolate()->factory();
+  auto isolate = handle_zone_scope.main_isolate();
+  auto factory = isolate->factory();
 
   Handle<HeapObject> slot_value0 = factory->InternalizeUtf8String("0");
   Handle<HeapObject> slot_value1 = factory->InternalizeUtf8String("1");
 
   Handle<Context> context_object0 = factory->NewNativeContext();
-  Handle<Context> context_object1 = factory->NewNativeContext();
-  context_object1->set_previous(*context_object0);
+  Handle<Context> context_object1 =
+      NewContextForTesting(isolate, context_object0);
   context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
   context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
 
@@ -427,12 +436,10 @@ TEST(ReduceJSStoreContext0) {
 
   // Make a context and initialize it a bit for this test.
   Handle<Context> native = t.factory()->NewNativeContext();
-  Handle<Context> subcontext1 = t.factory()->NewNativeContext();
-  Handle<Context> subcontext2 = t.factory()->NewNativeContext();
-  subcontext2->set_previous(*subcontext1);
-  subcontext1->set_previous(*native);
+  Handle<Context> subcontext1 = NewContextForTesting(t.isolate(), native);
+  Handle<Context> subcontext2 = NewContextForTesting(t.isolate(), subcontext1);
   Handle<Object> expected = t.factory()->InternalizeUtf8String("gboy!");
-  const int slot = Context::PREVIOUS_INDEX;
+  const int slot = 5;
   native->set(slot, *expected);
 
   Node* const_context = t.jsgraph()->Constant(ObjectRef(t.broker(), native));
@@ -542,8 +549,8 @@ TEST(ReduceJSStoreContext2) {
   Handle<HeapObject> slot_value1 = t.factory()->InternalizeUtf8String("1");
 
   Handle<Context> context_object0 = t.factory()->NewNativeContext();
-  Handle<Context> context_object1 = t.factory()->NewNativeContext();
-  context_object1->set_previous(*context_object0);
+  Handle<Context> context_object1 =
+      NewContextForTesting(t.isolate(), context_object0);
   context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
   context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
 
@@ -585,14 +592,15 @@ TEST(ReduceJSStoreContext2) {
 
 TEST(ReduceJSStoreContext3) {
   HandleAndZoneScope handle_zone_scope;
-  auto factory = handle_zone_scope.main_isolate()->factory();
+  auto isolate = handle_zone_scope.main_isolate();
+  auto factory = isolate->factory();
 
   Handle<HeapObject> slot_value0 = factory->InternalizeUtf8String("0");
   Handle<HeapObject> slot_value1 = factory->InternalizeUtf8String("1");
 
   Handle<Context> context_object0 = factory->NewNativeContext();
-  Handle<Context> context_object1 = factory->NewNativeContext();
-  context_object1->set_previous(*context_object0);
+  Handle<Context> context_object1 =
+      NewContextForTesting(isolate, context_object0);
   context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
   context_object1->set(Context::EXTENSION_INDEX, *slot_value1);