[builtins] Set {builtin_index} during code allocation.

This ensures that the {Code::builtin_index} field is only set during
allocation of new {Code} objects, making this field truly immutable.

R=jgruber@chromium.org
BUG=v8:6792

Change-Id: Ic793346976183149e2d077e92cb9da3c925ea865
Reviewed-on: https://chromium-review.googlesource.com/774439Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49414}

src/builtins/setup-builtins-internal.cc

@@ -42,7 +42,7 @@ void PostBuildProfileAndTracing(Isolate* isolate, Code* code,
 typedef void (*MacroAssemblerGenerator)(MacroAssembler*);
 typedef void (*CodeAssemblerGenerator)(compiler::CodeAssemblerState*);
 
-Handle<Code> BuildPlaceholder(Isolate* isolate) {
+Handle<Code> BuildPlaceholder(Isolate* isolate, int32_t builtin_index) {
   HandleScope scope(isolate);
   const size_t buffer_size = 1 * KB;
   byte buffer[buffer_size];  // NOLINT(runtime/arrays)
@@ -54,12 +54,12 @@ Handle<Code> BuildPlaceholder(Isolate* isolate) {
   }
   CodeDesc desc;
   masm.GetCode(isolate, &desc);
-  Handle<Code> code =
-      isolate->factory()->NewCode(desc, Code::BUILTIN, masm.CodeObject());
+  Handle<Code> code = isolate->factory()->NewCode(
+      desc, Code::BUILTIN, masm.CodeObject(), builtin_index);
   return scope.CloseAndEscape(code);
 }
 
-Code* BuildWithMacroAssembler(Isolate* isolate,
+Code* BuildWithMacroAssembler(Isolate* isolate, int32_t builtin_index,
                               MacroAssemblerGenerator generator,
                               const char* s_name) {
   HandleScope scope(isolate);
@@ -73,13 +73,14 @@ Code* BuildWithMacroAssembler(Isolate* isolate,
   generator(&masm);
   CodeDesc desc;
   masm.GetCode(isolate, &desc);
-  Handle<Code> code =
-      isolate->factory()->NewCode(desc, Code::BUILTIN, masm.CodeObject());
+  Handle<Code> code = isolate->factory()->NewCode(
+      desc, Code::BUILTIN, masm.CodeObject(), builtin_index);
   PostBuildProfileAndTracing(isolate, *code, s_name);
   return *code;
 }
 
-Code* BuildAdaptor(Isolate* isolate, Address builtin_address,
+Code* BuildAdaptor(Isolate* isolate, int32_t builtin_index,
+                   Address builtin_address,
                    Builtins::ExitFrameType exit_frame_type, const char* name) {
   HandleScope scope(isolate);
   // Canonicalize handles, so that we can share constant pool entries pointing
@@ -92,14 +93,14 @@ Code* BuildAdaptor(Isolate* isolate, Address builtin_address,
   Builtins::Generate_Adaptor(&masm, builtin_address, exit_frame_type);
   CodeDesc desc;
   masm.GetCode(isolate, &desc);
-  Handle<Code> code =
-      isolate->factory()->NewCode(desc, Code::BUILTIN, masm.CodeObject());
+  Handle<Code> code = isolate->factory()->NewCode(
+      desc, Code::BUILTIN, masm.CodeObject(), builtin_index);
   PostBuildProfileAndTracing(isolate, *code, name);
   return *code;
 }
 
 // Builder for builtins implemented in TurboFan with JS linkage.
-Code* BuildWithCodeStubAssemblerJS(Isolate* isolate,
+Code* BuildWithCodeStubAssemblerJS(Isolate* isolate, int32_t builtin_index,
                                    CodeAssemblerGenerator generator, int argc,
                                    const char* name) {
   HandleScope scope(isolate);
@@ -110,7 +111,7 @@ Code* BuildWithCodeStubAssemblerJS(Isolate* isolate,
   const int argc_with_recv =
       (argc == SharedFunctionInfo::kDontAdaptArgumentsSentinel) ? 0 : argc + 1;
   compiler::CodeAssemblerState state(isolate, &zone, argc_with_recv,
-                                     Code::BUILTIN, name);
+                                     Code::BUILTIN, name, builtin_index);
   generator(&state);
   Handle<Code> code = compiler::CodeAssembler::GenerateCode(&state);
   PostBuildProfileAndTracing(isolate, *code, name);
@@ -118,7 +119,7 @@ Code* BuildWithCodeStubAssemblerJS(Isolate* isolate,
 }
 
 // Builder for builtins implemented in TurboFan with CallStub linkage.
-Code* BuildWithCodeStubAssemblerCS(Isolate* isolate,
+Code* BuildWithCodeStubAssemblerCS(Isolate* isolate, int32_t builtin_index,
                                    CodeAssemblerGenerator generator,
                                    CallDescriptors::Key interface_descriptor,
                                    const char* name, int result_size) {
@@ -133,7 +134,7 @@ Code* BuildWithCodeStubAssemblerCS(Isolate* isolate,
   // Ensure descriptor is already initialized.
   DCHECK_LE(0, descriptor.GetRegisterParameterCount());
   compiler::CodeAssemblerState state(isolate, &zone, descriptor, Code::BUILTIN,
-                                     name, result_size);
+                                     name, result_size, 0, builtin_index);
   generator(&state);
   Handle<Code> code = compiler::CodeAssembler::GenerateCode(&state);
   PostBuildProfileAndTracing(isolate, *code, name);
@@ -143,11 +144,8 @@ Code* BuildWithCodeStubAssemblerCS(Isolate* isolate,
 
 void SetupIsolateDelegate::AddBuiltin(Builtins* builtins, int index,
                                       Code* code) {
-  // TODO(mstarzinger,6792): This code-space modification section should be
-  // moved into {Heap} eventually and a safe wrapper be provided.
-  CodeSpaceMemoryModificationScope modification_scope(code->GetHeap());
+  DCHECK_EQ(index, code->builtin_index());
   builtins->builtins_[index] = code;
-  code->set_builtin_index(index);
 }
 
 void SetupIsolateDelegate::PopulateWithPlaceholders(Isolate* isolate) {
@@ -156,20 +154,17 @@ void SetupIsolateDelegate::PopulateWithPlaceholders(Isolate* isolate) {
   // support circular references between builtins.
   Builtins* builtins = isolate->builtins();
   HandleScope scope(isolate);
-  Handle<Code> placeholder = BuildPlaceholder(isolate);
-  AddBuiltin(builtins, 0, *placeholder);
-  for (int i = 1; i < Builtins::builtin_count; i++) {
-    AddBuiltin(builtins, i, *isolate->factory()->CopyCode(placeholder));
+  for (int i = 0; i < Builtins::builtin_count; i++) {
+    Handle<Code> placeholder = BuildPlaceholder(isolate, i);
+    AddBuiltin(builtins, i, *placeholder);
   }
 }
 
 void SetupIsolateDelegate::ReplacePlaceholders(Isolate* isolate) {
-  // TODO(mstarzinger,6792): This code-space modification section should be
-  // moved into {Heap} eventually and a safe wrapper be provided.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   // Replace references from all code objects to placeholders.
   Builtins* builtins = isolate->builtins();
   DisallowHeapAllocation no_gc;
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   static const int kRelocMask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET) |
                                 RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT);
   HeapIterator iterator(isolate->heap());
@@ -217,38 +212,40 @@ void SetupIsolateDelegate::SetupBuiltinsInternal(Isolate* isolate) {
 
   int index = 0;
   Code* code;
-#define BUILD_CPP(Name)                                       \
-  code = BuildAdaptor(isolate, FUNCTION_ADDR(Builtin_##Name), \
-                      Builtins::BUILTIN_EXIT, #Name);         \
+#define BUILD_CPP(Name)                                              \
+  code = BuildAdaptor(isolate, index, FUNCTION_ADDR(Builtin_##Name), \
+                      Builtins::BUILTIN_EXIT, #Name);                \
   AddBuiltin(builtins, index++, code);
-#define BUILD_API(Name)                                                       \
-  code = BuildAdaptor(isolate, FUNCTION_ADDR(Builtin_##Name), Builtins::EXIT, \
-                      #Name);                                                 \
+#define BUILD_API(Name)                                              \
+  code = BuildAdaptor(isolate, index, FUNCTION_ADDR(Builtin_##Name), \
+                      Builtins::EXIT, #Name);                        \
   AddBuiltin(builtins, index++, code);
-#define BUILD_TFJ(Name, Argc, ...)                                         \
-  code = BuildWithCodeStubAssemblerJS(isolate, &Builtins::Generate_##Name, \
-                                      Argc, #Name);                        \
+#define BUILD_TFJ(Name, Argc, ...)                              \
+  code = BuildWithCodeStubAssemblerJS(                          \
+      isolate, index, &Builtins::Generate_##Name, Argc, #Name); \
   AddBuiltin(builtins, index++, code);
-#define BUILD_TFC(Name, InterfaceDescriptor, result_size)                   \
-  { InterfaceDescriptor##Descriptor descriptor(isolate); }                  \
-  code = BuildWithCodeStubAssemblerCS(isolate, &Builtins::Generate_##Name,  \
-                                      CallDescriptors::InterfaceDescriptor, \
-                                      #Name, result_size);                  \
+#define BUILD_TFC(Name, InterfaceDescriptor, result_size)        \
+  { InterfaceDescriptor##Descriptor descriptor(isolate); }       \
+  code = BuildWithCodeStubAssemblerCS(                           \
+      isolate, index, &Builtins::Generate_##Name,                \
+      CallDescriptors::InterfaceDescriptor, #Name, result_size); \
   AddBuiltin(builtins, index++, code);
-#define BUILD_TFS(Name, ...)                                               \
-  /* Return size for generic TF builtins (stub linkage) is always 1. */    \
-  code = BuildWithCodeStubAssemblerCS(isolate, &Builtins::Generate_##Name, \
-                                      CallDescriptors::Name, #Name, 1);    \
+#define BUILD_TFS(Name, ...)                                                   \
+  /* Return size for generic TF builtins (stub linkage) is always 1. */        \
+  code =                                                                       \
+      BuildWithCodeStubAssemblerCS(isolate, index, &Builtins::Generate_##Name, \
+                                   CallDescriptors::Name, #Name, 1);           \
   AddBuiltin(builtins, index++, code);
-#define BUILD_TFH(Name, InterfaceDescriptor)                                \
-  { InterfaceDescriptor##Descriptor descriptor(isolate); }                  \
-  /* Return size for IC builtins/handlers is always 1. */                   \
-  code = BuildWithCodeStubAssemblerCS(isolate, &Builtins::Generate_##Name,  \
-                                      CallDescriptors::InterfaceDescriptor, \
-                                      #Name, 1);                            \
+#define BUILD_TFH(Name, InterfaceDescriptor)               \
+  { InterfaceDescriptor##Descriptor descriptor(isolate); } \
+  /* Return size for IC builtins/handlers is always 1. */  \
+  code = BuildWithCodeStubAssemblerCS(                     \
+      isolate, index, &Builtins::Generate_##Name,          \
+      CallDescriptors::InterfaceDescriptor, #Name, 1);     \
   AddBuiltin(builtins, index++, code);
-#define BUILD_ASM(Name)                                                      \
-  code = BuildWithMacroAssembler(isolate, Builtins::Generate_##Name, #Name); \
+#define BUILD_ASM(Name)                                                     \
+  code = BuildWithMacroAssembler(isolate, index, Builtins::Generate_##Name, \
+                                 #Name);                                    \
   AddBuiltin(builtins, index++, code);
 
   BUILTIN_LIST(BUILD_CPP, BUILD_API, BUILD_TFJ, BUILD_TFC, BUILD_TFS, BUILD_TFH,

src/code-stubs.cc

@@ -129,8 +129,9 @@ Handle<Code> PlatformCodeStub::GenerateCode() {
   masm.GetCode(isolate(), &desc);
   // Copy the generated code into a heap object.
   Handle<Code> new_object = factory->NewCode(
-      desc, Code::STUB, masm.CodeObject(), table, MaybeHandle<ByteArray>(),
-      DeoptimizationData::Empty(isolate()), NeedsImmovableCode(), GetKey());
+      desc, Code::STUB, masm.CodeObject(), Builtins::kNoBuiltinId, table,
+      MaybeHandle<ByteArray>(), DeoptimizationData::Empty(isolate()),
+      NeedsImmovableCode(), GetKey());
   return new_object;
 }
 

src/compilation-info.cc

@@ -67,6 +67,7 @@ CompilationInfo::CompilationInfo(Vector<const char> debug_name,
       flags_(0),
       code_kind_(code_kind),
       stub_key_(0),
+      builtin_index_(Builtins::kNoBuiltinId),
       mode_(mode),
       osr_offset_(BailoutId::None()),
       feedback_vector_spec_(zone),

src/compilation-info.h

@@ -89,6 +89,8 @@ class V8_EXPORT_PRIVATE CompilationInfo final {
   Code::Kind code_kind() const { return code_kind_; }
   uint32_t stub_key() const { return stub_key_; }
   void set_stub_key(uint32_t stub_key) { stub_key_ = stub_key; }
+  int32_t builtin_index() const { return builtin_index_; }
+  void set_builtin_index(int32_t index) { builtin_index_ = index; }
   BailoutId osr_offset() const { return osr_offset_; }
   JavaScriptFrame* osr_frame() const { return osr_frame_; }
   int num_parameters() const;
@@ -279,6 +281,7 @@ class V8_EXPORT_PRIVATE CompilationInfo final {
 
   Code::Kind code_kind_;
   uint32_t stub_key_;
+  int32_t builtin_index_;
 
   Handle<SharedFunctionInfo> shared_info_;
 

src/compiler/code-assembler.cc

@@ -56,30 +56,31 @@ static_assert(
 
 CodeAssemblerState::CodeAssemblerState(
     Isolate* isolate, Zone* zone, const CallInterfaceDescriptor& descriptor,
-    Code::Kind kind, const char* name, size_t result_size, uint32_t stub_key)
+    Code::Kind kind, const char* name, size_t result_size, uint32_t stub_key,
+    int32_t builtin_index)
     : CodeAssemblerState(
           isolate, zone,
           Linkage::GetStubCallDescriptor(
               isolate, zone, descriptor, descriptor.GetStackParameterCount(),
               CallDescriptor::kNoFlags, Operator::kNoProperties,
               MachineType::AnyTagged(), result_size),
-          kind, name, stub_key) {}
+          kind, name, stub_key, builtin_index) {}
 
 CodeAssemblerState::CodeAssemblerState(Isolate* isolate, Zone* zone,
                                        int parameter_count, Code::Kind kind,
-                                       const char* name)
+                                       const char* name, int32_t builtin_index)
     : CodeAssemblerState(
           isolate, zone,
           Linkage::GetJSCallDescriptor(zone, false, parameter_count,
                                        kind == Code::BUILTIN
                                            ? CallDescriptor::kPushArgumentCount
                                            : CallDescriptor::kNoFlags),
-          kind, name, 0) {}
+          kind, name, 0, builtin_index) {}
 
 CodeAssemblerState::CodeAssemblerState(Isolate* isolate, Zone* zone,
                                        CallDescriptor* call_descriptor,
                                        Code::Kind kind, const char* name,
-                                       uint32_t stub_key)
+                                       uint32_t stub_key, int32_t builtin_index)
     : raw_assembler_(new RawMachineAssembler(
           isolate, new (zone) Graph(zone), call_descriptor,
           MachineType::PointerRepresentation(),
@@ -88,6 +89,7 @@ CodeAssemblerState::CodeAssemblerState(Isolate* isolate, Zone* zone,
       kind_(kind),
       name_(name),
       stub_key_(stub_key),
+      builtin_index_(builtin_index),
       code_generated_(false),
       variables_(zone) {}
 
@@ -180,7 +182,7 @@ Handle<Code> CodeAssembler::GenerateCode(CodeAssemblerState* state) {
 
   Handle<Code> code = Pipeline::GenerateCodeForCodeStub(
       rasm->isolate(), rasm->call_descriptor(), rasm->graph(), schedule,
-      state->kind_, state->name_, state->stub_key_,
+      state->kind_, state->name_, state->stub_key_, state->builtin_index_,
       should_optimize_jumps ? &jump_opt : nullptr);
 
   if (jump_opt.is_optimizable()) {
@@ -189,7 +191,8 @@ Handle<Code> CodeAssembler::GenerateCode(CodeAssemblerState* state) {
     // Regenerate machine code
     code = Pipeline::GenerateCodeForCodeStub(
         rasm->isolate(), rasm->call_descriptor(), rasm->graph(), schedule,
-        state->kind_, state->name_, state->stub_key_, &jump_opt);
+        state->kind_, state->name_, state->stub_key_, state->builtin_index_,
+        &jump_opt);
   }
 
   state->code_generated_ = true;

src/compiler/code-assembler.h

@@ -1192,11 +1192,13 @@ class V8_EXPORT_PRIVATE CodeAssemblerState {
   CodeAssemblerState(Isolate* isolate, Zone* zone,
                      const CallInterfaceDescriptor& descriptor, Code::Kind kind,
                      const char* name, size_t result_size = 1,
-                     uint32_t stub_key = 0);
+                     uint32_t stub_key = 0,
+                     int32_t builtin_index = Builtins::kNoBuiltinId);
 
   // Create with JSCall linkage.
   CodeAssemblerState(Isolate* isolate, Zone* zone, int parameter_count,
-                     Code::Kind kind, const char* name);
+                     Code::Kind kind, const char* name,
+                     int32_t builtin_index = Builtins::kNoBuiltinId);
 
   ~CodeAssemblerState();
 
@@ -1216,12 +1218,14 @@ class V8_EXPORT_PRIVATE CodeAssemblerState {
 
   CodeAssemblerState(Isolate* isolate, Zone* zone,
                      CallDescriptor* call_descriptor, Code::Kind kind,
-                     const char* name, uint32_t stub_key);
+                     const char* name, uint32_t stub_key,
+                     int32_t builtin_index);
 
   std::unique_ptr<RawMachineAssembler> raw_assembler_;
   Code::Kind kind_;
   const char* name_;
   uint32_t stub_key_;
+  int32_t builtin_index_;
   bool code_generated_;
   ZoneSet<CodeAssemblerVariable::Impl*> variables_;
   CodeAssemblerCallback call_prologue_;

src/compiler/code-generator.cc

@@ -315,8 +315,8 @@ Handle<Code> CodeGenerator::FinalizeCode() {
   }
 
   Handle<Code> result = isolate()->factory()->NewCode(
-      desc, info()->code_kind(), Handle<Object>(), table, source_positions,
-      deopt_data, kMovable, info()->stub_key(), true,
+      desc, info()->code_kind(), Handle<Object>(), info()->builtin_index(),
+      table, source_positions, deopt_data, kMovable, info()->stub_key(), true,
       frame()->GetTotalFrameSlotCount(), safepoints()->GetCodeOffset());
   isolate()->counters()->total_compiled_code_size()->Increment(
       result->instruction_size());

src/compiler/pipeline.cc

@@ -1927,8 +1927,9 @@ bool PipelineImpl::OptimizeGraph(Linkage* linkage) {
 Handle<Code> Pipeline::GenerateCodeForCodeStub(
     Isolate* isolate, CallDescriptor* call_descriptor, Graph* graph,
     Schedule* schedule, Code::Kind kind, const char* debug_name,
-    uint32_t stub_key, JumpOptimizationInfo* jump_opt) {
+    uint32_t stub_key, int32_t builtin_index, JumpOptimizationInfo* jump_opt) {
   CompilationInfo info(CStrVector(debug_name), graph->zone(), kind);
+  info.set_builtin_index(builtin_index);
   info.set_stub_key(stub_key);
 
   // Construct a pipeline for scheduling and code generation.

src/compiler/pipeline.h

@@ -56,7 +56,7 @@ class Pipeline : public AllStatic {
   static Handle<Code> GenerateCodeForCodeStub(
       Isolate* isolate, CallDescriptor* call_descriptor, Graph* graph,
       Schedule* schedule, Code::Kind kind, const char* debug_name,
-      uint32_t stub_key, JumpOptimizationInfo* jump_opt);
+      uint32_t stub_key, int32_t builtin_index, JumpOptimizationInfo* jump_opt);
 
   // Run the entire pipeline and generate a handle to a code object suitable for
   // testing.

src/deoptimizer.cc

@@ -1786,8 +1786,9 @@ void Deoptimizer::EnsureCodeForDeoptimizationEntry(Isolate* isolate,
   // Allocate the code as immovable since the entry addresses will be used
   // directly and there is no support for relocating them.
   Handle<Code> code = isolate->factory()->NewCode(
-      desc, Code::STUB, Handle<Object>(), MaybeHandle<HandlerTable>(),
-      MaybeHandle<ByteArray>(), MaybeHandle<DeoptimizationData>(), kImmovable);
+      desc, Code::STUB, Handle<Object>(), Builtins::kNoBuiltinId,
+      MaybeHandle<HandlerTable>(), MaybeHandle<ByteArray>(),
+      MaybeHandle<DeoptimizationData>(), kImmovable);
   CHECK(Heap::IsImmovable(*code));
 
   CHECK_NULL(data->deopt_entry_code_[type]);

src/factory.cc

@@ -1800,7 +1800,7 @@ Handle<Code> Factory::NewCodeRaw(int object_size, Movability movability) {
 
 Handle<Code> Factory::NewCode(
     const CodeDesc& desc, Code::Kind kind, Handle<Object> self_ref,
-    MaybeHandle<HandlerTable> maybe_handler_table,
+    int32_t builtin_index, MaybeHandle<HandlerTable> maybe_handler_table,
     MaybeHandle<ByteArray> maybe_source_position_table,
     MaybeHandle<DeoptimizationData> maybe_deopt_data, Movability movability,
     uint32_t stub_key, bool is_turbofanned, int stack_slots,
@@ -1857,7 +1857,7 @@ Handle<Code> Factory::NewCode(
   code->set_source_position_table(*source_position_table);
   code->set_protected_instructions(*empty_fixed_array(), SKIP_WRITE_BARRIER);
   code->set_constant_pool_offset(desc.instr_size - desc.constant_pool_size);
-  code->set_builtin_index(-1);
+  code->set_builtin_index(builtin_index);
   code->set_trap_handler_index(Smi::FromInt(-1));
 
   switch (code->kind()) {

src/factory.h

@@ -672,6 +672,7 @@ class V8_EXPORT_PRIVATE Factory final {
   // by containing this handle.
   Handle<Code> NewCode(const CodeDesc& desc, Code::Kind kind,
                        Handle<Object> self_reference,
+                       int32_t builtin_index = Builtins::kNoBuiltinId,
                        MaybeHandle<HandlerTable> maybe_handler_table =
                            MaybeHandle<HandlerTable>(),
                        MaybeHandle<ByteArray> maybe_source_position_table =

test/cctest/compiler/code-assembler-tester.h

@@ -39,7 +39,7 @@ class CodeAssemblerTester {
   CodeAssemblerTester(Isolate* isolate, CallDescriptor* call_descriptor)
       : zone_(isolate->allocator(), ZONE_NAME),
         scope_(isolate),
-        state_(isolate, &zone_, call_descriptor, Code::STUB, "test", 0) {}
+        state_(isolate, &zone_, call_descriptor, Code::STUB, "test", 0, -1) {}
 
   CodeAssemblerState* state() { return &state_; }
 

test/cctest/heap/test-heap.cc

@@ -5802,8 +5802,9 @@ Handle<Code> GenerateDummyImmovableCode(Isolate* isolate) {
   CodeDesc desc;
   assm.GetCode(isolate, &desc);
   Handle<Code> code = isolate->factory()->NewCode(
-      desc, Code::STUB, Handle<Code>(), HandlerTable::Empty(isolate),
-      MaybeHandle<ByteArray>(), DeoptimizationData::Empty(isolate), kImmovable);
+      desc, Code::STUB, Handle<Code>(), Builtins::kNoBuiltinId,
+      HandlerTable::Empty(isolate), MaybeHandle<ByteArray>(),
+      DeoptimizationData::Empty(isolate), kImmovable);
   CHECK(code->IsCode());
 
   return code;