[Interpreter] Avoid accessing Isolate from during bytecode generation.

src/interpreter/bytecode-generator.cc

 // Copyright 2015 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.
 
 #include "src/interpreter/bytecode-generator.h"
 
 #include "src/ast/scopes.h"
 #include "src/code-stubs.h"
 #include "src/compiler.h"
 #include "src/interpreter/bytecode-flags.h"
@@ skipping 582 matching lines @@
   BytecodeLabels* else_labels_;
   TestFallthrough fallthrough_;
   bool result_consumed_by_test_;
 
   DISALLOW_COPY_AND_ASSIGN(TestResultScope);
 };
 
 // Used to build a list of global declaration initial value pairs.
 class BytecodeGenerator::GlobalDeclarationsBuilder final : public ZoneObject {
  public:
-  GlobalDeclarationsBuilder(Isolate* isolate, Zone* zone)
-      : isolate_(isolate),
-        declarations_(0, zone),
+  explicit GlobalDeclarationsBuilder(Zone* zone)
+      : declarations_(0, zone),
         constant_pool_entry_(0),
         has_constant_pool_entry_(false) {}
 
   void AddFunctionDeclaration(FeedbackVectorSlot slot, FunctionLiteral* func) {
     DCHECK(!slot.IsInvalid());
     declarations_.push_back(std::make_pair(slot, func));
   }
 
   void AddUndefinedDeclaration(FeedbackVectorSlot slot) {
     DCHECK(!slot.IsInvalid());
     declarations_.push_back(std::make_pair(slot, nullptr));
   }
 
   Handle<FixedArray> AllocateDeclarationPairs(CompilationInfo* info) {
     DCHECK(has_constant_pool_entry_);
     int array_index = 0;
-    Handle<FixedArray> pairs = isolate_->factory()->NewFixedArray(
+    Handle<FixedArray> pairs = info->isolate()->factory()->NewFixedArray(
         static_cast<int>(declarations_.size() * 2), TENURED);
     for (std::pair<FeedbackVectorSlot, FunctionLiteral*> declaration :
          declarations_) {
       FunctionLiteral* func = declaration.second;
       Handle<Object> initial_value;
       if (func == nullptr) {
-        initial_value = isolate_->factory()->undefined_value();
+        initial_value = info->isolate()->factory()->undefined_value();
       } else {
         initial_value =
             Compiler::GetSharedFunctionInfo(func, info->script(), info);
       }
 
       // Return a null handle if any initial values can't be created. Caller
       // will set stack overflow.
       if (initial_value.is_null()) return Handle<FixedArray>();
 
       pairs->set(array_index++, Smi::FromInt(declaration.first.ToInt()));
@@ skipping 10 matching lines @@
   void set_constant_pool_entry(size_t constant_pool_entry) {
     DCHECK(!empty());
     DCHECK(!has_constant_pool_entry_);
     constant_pool_entry_ = constant_pool_entry;
     has_constant_pool_entry_ = true;
   }
 
   bool empty() { return declarations_.empty(); }
 
  private:
-  Isolate* isolate_;
   ZoneVector<std::pair<FeedbackVectorSlot, FunctionLiteral*>> declarations_;
   size_t constant_pool_entry_;
   bool has_constant_pool_entry_;
 };
 
+class DisallowIsolateAccessScope {

[Michael Starzinger, 2016/08/16 09:10:22]

Just for my own clarification mostly: How much worse would it be to remove the
Isolate field from the BytecodeGenerator and explicitly pass it into
FinalizeBytecode instead. Would that allow us to get rid of this helper class
and the "isolate_access_allowed" flag? Feel free to disregard in case I am
missing something obvious.

[rmcilroy, 2016/08/16 11:24:54]

That works, done.

+ public:
+  explicit DisallowIsolateAccessScope(BytecodeGenerator* generator)
+      : generator_(generator), previous_(generator->isolate_access_allowed()) {
+    generator_->set_isolate_access_allowed(false);
+  }
+
+  ~DisallowIsolateAccessScope() {
+    generator_->set_isolate_access_allowed(previous_);
+  }
+
+ private:
+  BytecodeGenerator* generator_;
+  bool previous_;
+};
+
 BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
     : isolate_(info->isolate()),
       zone_(info->zone()),
       builder_(new (zone()) BytecodeArrayBuilder(
           info->isolate(), info->zone(), info->num_parameters_including_this(),
           info->scope()->MaxNestedContextChainLength(),
           info->scope()->num_stack_slots(), info->literal(),
           info->SourcePositionRecordingMode())),
       info_(info),
       scope_(info->scope()),
-      globals_builder_(new (zone()) GlobalDeclarationsBuilder(info->isolate(),
-                                                              info->zone())),
+      isolate_access_allowed_(true),
+      globals_builder_(new (zone()) GlobalDeclarationsBuilder(info->zone())),
       global_declarations_(0, info->zone()),
       function_literals_(0, info->zone()),
       native_function_literals_(0, info->zone()),
       execution_control_(nullptr),
       execution_context_(nullptr),
       execution_result_(nullptr),
       register_allocator_(nullptr),
       generator_resume_points_(info->literal()->yield_count(), info->zone()),
       generator_state_(),
-      loop_depth_(0) {
+      loop_depth_(0),
+      home_object_symbol_(info->isolate()->factory()->home_object_symbol()),
+      prototype_string_(info->isolate()->factory()->prototype_string()) {
   InitializeAstVisitor(isolate()->stack_guard()->real_climit());
 }
 
 Handle<BytecodeArray> BytecodeGenerator::MakeBytecode() {
   // Create an inner HandleScope to avoid unnecessarily canonicalizing handles
   // created as part of bytecode finalization.
   HandleScope scope(isolate());
 
   GenerateBytecode();
   FinalizeBytecode();
 
   if (HasStackOverflow()) return Handle<BytecodeArray>();
 
-  return scope.CloseAndEscape(builder()->ToBytecodeArray());
+  return scope.CloseAndEscape(builder()->ToBytecodeArray(isolate()));
 }
 
 void BytecodeGenerator::FinalizeBytecode() {
   // Build global declaration pair arrays.
   for (GlobalDeclarationsBuilder* globals_builder : global_declarations_) {
     Handle<FixedArray> declarations =
         globals_builder->AllocateDeclarationPairs(info());
     if (declarations.is_null()) return SetStackOverflow();
     builder()->InsertConstantPoolEntryAt(globals_builder->constant_pool_entry(),
                                          declarations);
@@ skipping 17 matching lines @@
                                                  expr->name());
     if (shared_info.is_null()) return SetStackOverflow();
     builder()->InsertConstantPoolEntryAt(literal.second, shared_info);
   }
 }
 
 void BytecodeGenerator::GenerateBytecode() {
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
   DisallowHandleDereference no_deref;
+  DisallowIsolateAccessScope no_isolate(this);
 
   // Initialize the incoming context.
   ContextScope incoming_context(this, scope(), false);
 
   // Initialize control scope.
   ControlScopeForTopLevel control(this);
 
   RegisterAllocationScope register_scope(this);
 
   if (IsResumableFunction(info()->literal()->kind())) {
@@ skipping 266 matching lines @@
   builder()
       ->LoadConstantPoolEntry(globals_builder()->constant_pool_entry())
       .StoreAccumulatorInRegister(pairs)
       .LoadLiteral(Smi::FromInt(encoded_flags))
       .StoreAccumulatorInRegister(flags)
       .MoveRegister(Register::function_closure(), function)
       .CallRuntime(Runtime::kDeclareGlobalsForInterpreter, pairs, 3);
 
   // Push and reset globals builder.
   global_declarations_.push_back(globals_builder());
-  globals_builder_ = new (zone()) GlobalDeclarationsBuilder(isolate(), zone());
+  globals_builder_ = new (zone()) GlobalDeclarationsBuilder(zone());
 }
 
 void BytecodeGenerator::VisitStatements(ZoneList<Statement*>* statements) {
   for (int i = 0; i < statements->length(); i++) {
     // Allocate an outer register allocations scope for the statement.
     RegisterAllocationScope allocation_scope(this);
     Statement* stmt = statements->at(i);
     Visit(stmt);
     if (stmt->IsJump()) break;
   }
@@ skipping 451 matching lines @@
   execution_result()->SetResultInAccumulator();
 }
 
 void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr) {
   VisitClassLiteralForRuntimeDefinition(expr);
 
   // Load the "prototype" from the constructor.
   register_allocator()->PrepareForConsecutiveAllocations(2);
   Register literal = register_allocator()->NextConsecutiveRegister();
   Register prototype = register_allocator()->NextConsecutiveRegister();
-  Handle<String> name = isolate()->factory()->prototype_string();
   FeedbackVectorSlot slot = expr->PrototypeSlot();
   builder()
       ->StoreAccumulatorInRegister(literal)
-      .LoadNamedProperty(literal, name, feedback_index(slot))
+      .LoadNamedProperty(literal, prototype_string(), feedback_index(slot))
       .StoreAccumulatorInRegister(prototype);
 
   VisitClassLiteralProperties(expr, literal, prototype);
   builder()->CallRuntime(Runtime::kToFastProperties, literal, 1);
   // Assign to class variable.
   if (expr->class_variable_proxy() != nullptr) {
     Variable* var = expr->class_variable_proxy()->var();
     FeedbackVectorSlot slot = expr->NeedsProxySlot()
                                   ? expr->ProxySlot()
                                   : FeedbackVectorSlot::Invalid();
@@ skipping 97 matching lines @@
         break;
       }
     }
   }
 }
 
 void BytecodeGenerator::VisitClassLiteralStaticPrototypeWithComputedName(
     Register key) {
   BytecodeLabel done;
   builder()
-      ->LoadLiteral(isolate()->factory()->prototype_string())
+      ->LoadLiteral(prototype_string())
       .CompareOperation(Token::Value::EQ_STRICT, key)
       .JumpIfFalse(&done)
       .CallRuntime(Runtime::kThrowStaticPrototypeError, Register(0), 0)
       .Bind(&done);
 }
 
 void BytecodeGenerator::VisitNativeFunctionLiteral(
     NativeFunctionLiteral* expr) {
   size_t entry = builder()->AllocateConstantPoolEntry();
   builder()->CreateClosure(entry, NOT_TENURED);
@@ skipping 1532 matching lines @@
 
   // Allocate a new local context.
   if (scope->is_script_scope()) {
     RegisterAllocationScope register_scope(this);
     Register closure = register_allocator()->NewRegister();
     Register scope_info = register_allocator()->NewRegister();
     DCHECK(Register::AreContiguous(closure, scope_info));
     builder()
         ->LoadAccumulatorWithRegister(Register::function_closure())
         .StoreAccumulatorInRegister(closure)
-        .LoadLiteral(scope->GetScopeInfo(isolate()))
+        .LoadLiteral(scope->scope_info())
         .StoreAccumulatorInRegister(scope_info)
         .CallRuntime(Runtime::kNewScriptContext, closure, 2);
   } else {
     int slot_count = scope->num_heap_slots() - Context::MIN_CONTEXT_SLOTS;
     builder()->CreateFunctionContext(slot_count);
   }
   execution_result()->SetResultInAccumulator();
 }
 
 void BytecodeGenerator::VisitBuildLocalActivationContext() {
@@ skipping 27 matching lines @@
 void BytecodeGenerator::VisitNewLocalBlockContext(Scope* scope) {
   AccumulatorResultScope accumulator_execution_result(this);
   DCHECK(scope->is_block_scope());
 
   // Allocate a new local block context.
   register_allocator()->PrepareForConsecutiveAllocations(2);
   Register scope_info = register_allocator()->NextConsecutiveRegister();
   Register closure = register_allocator()->NextConsecutiveRegister();
 
   builder()
-      ->LoadLiteral(scope->GetScopeInfo(isolate()))
+      ->LoadLiteral(scope->scope_info())
       .StoreAccumulatorInRegister(scope_info);
   VisitFunctionClosureForContext();
   builder()
       ->StoreAccumulatorInRegister(closure)
       .CallRuntime(Runtime::kPushBlockContext, scope_info, 2);
   execution_result()->SetResultInAccumulator();
 }
 
 void BytecodeGenerator::VisitNewLocalWithContext() {
   AccumulatorResultScope accumulator_execution_result(this);
@@ skipping 39 matching lines @@
     builder()->StoreAccumulatorInRegister(value_out);
     VisitSetHomeObject(value_out, home_object, property);
   }
 }
 
 void BytecodeGenerator::VisitSetHomeObject(Register value, Register home_object,
                                            ObjectLiteralProperty* property,
                                            int slot_number) {
   Expression* expr = property->value();
   if (FunctionLiteral::NeedsHomeObject(expr)) {
-    Handle<Name> name = isolate()->factory()->home_object_symbol();
     FeedbackVectorSlot slot = property->GetSlot(slot_number);
     builder()
         ->LoadAccumulatorWithRegister(home_object)
-        .StoreNamedProperty(value, name, feedback_index(slot), language_mode());
+        .StoreNamedProperty(value, home_object_symbol(), feedback_index(slot),
+                            language_mode());
   }
 }
 
 void BytecodeGenerator::VisitArgumentsObject(Variable* variable) {
   if (variable == nullptr) return;
 
   DCHECK(variable->IsContextSlot() || variable->IsStackAllocated());
 
   // Allocate and initialize a new arguments object and assign to the
   // {arguments} variable.
@@ skipping 136 matching lines @@
   return execution_context()->scope()->language_mode();
 }
 
 int BytecodeGenerator::feedback_index(FeedbackVectorSlot slot) const {
   return TypeFeedbackVector::GetIndex(slot);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8