From 7b48dd55e1cf105b7293250543084806b4d274e2 Mon Sep 17 00:00:00 2001 From: Dan Elphick Date: Fri, 14 Jun 2019 11:33:42 +0100 Subject: [PATCH] [builtins] Make ContinueToBuiltinHelper skip off-heap builtin trampolines This changes Generate_ContinueToBuiltinHelper to generate code to load the builtin address directly from the builtins table rather than going via the executable code in the trampoline's code object. The set up for Generate_ContinueToBuiltinHelper is changed so that the builtin index is stored on the stack in place of the builtin Code object which is no longer needed. Bug: v8:9338 Change-Id: I83f66af99fb27f131fc39ff426fdca4b1d674b70 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1648155 Commit-Queue: Dan Elphick Reviewed-by: Sigurd Schneider Reviewed-by: Ross McIlroy Cr-Commit-Position: refs/heads/master@{#62170} --- src/builtins/arm/builtins-arm.cc | 9 ++++++--- src/builtins/arm64/builtins-arm64.cc | 12 +++++++----- src/builtins/ia32/builtins-ia32.cc | 10 +++++++++- src/builtins/x64/builtins-x64.cc | 10 +++++++++- src/codegen/arm/macro-assembler-arm.cc | 6 +++++- src/codegen/arm/macro-assembler-arm.h | 3 +++ src/codegen/arm64/macro-assembler-arm64.cc | 6 +++++- src/codegen/arm64/macro-assembler-arm64.h | 3 +++ src/codegen/ia32/macro-assembler-ia32.cc | 6 +++++- src/codegen/ia32/macro-assembler-ia32.h | 3 +++ src/codegen/x64/macro-assembler-x64.cc | 14 +++++++++----- src/codegen/x64/macro-assembler-x64.h | 1 + src/deoptimizer/deoptimizer.cc | 5 +++-- src/execution/frame-constants.h | 2 +- 14 files changed, 69 insertions(+), 21 deletions(-) diff --git a/src/builtins/arm/builtins-arm.cc b/src/builtins/arm/builtins-arm.cc index 8369da01c3..5321b72a12 100644 --- a/src/builtins/arm/builtins-arm.cc +++ b/src/builtins/arm/builtins-arm.cc @@ -1509,13 +1509,16 @@ void Generate_ContinueToBuiltinHelper(MacroAssembler* masm, __ ldr(fp, MemOperand( sp, BuiltinContinuationFrameConstants::kFixedFrameSizeFromFp)); + // Load builtin index (stored as a Smi) and use it to get the builtin start + // address from the builtins table. UseScratchRegisterScope temps(masm); - Register scratch = temps.Acquire(); - __ Pop(scratch); + Register builtin = temps.Acquire(); + __ Pop(builtin); __ add(sp, sp, Operand(BuiltinContinuationFrameConstants::kFixedFrameSizeFromFp)); __ Pop(lr); - __ add(pc, scratch, Operand(Code::kHeaderSize - kHeapObjectTag)); + __ LoadEntryFromBuiltinIndex(builtin); + __ bx(builtin); } } // namespace diff --git a/src/builtins/arm64/builtins-arm64.cc b/src/builtins/arm64/builtins-arm64.cc index dee1b6b2a4..fd55ba7879 100644 --- a/src/builtins/arm64/builtins-arm64.cc +++ b/src/builtins/arm64/builtins-arm64.cc @@ -1683,18 +1683,20 @@ void Generate_ContinueToBuiltinHelper(MacroAssembler* masm, if (java_script_builtin) __ SmiUntag(kJavaScriptCallArgCountRegister); - // Load builtin object. + // Load builtin index (stored as a Smi) and use it to get the builtin start + // address from the builtins table. UseScratchRegisterScope temps(masm); Register builtin = temps.AcquireX(); - __ Ldr(builtin, - MemOperand(fp, BuiltinContinuationFrameConstants::kBuiltinOffset)); + __ Ldr( + builtin, + MemOperand(fp, BuiltinContinuationFrameConstants::kBuiltinIndexOffset)); // Restore fp, lr. __ Mov(sp, fp); __ Pop(fp, lr); - // Call builtin. - __ JumpCodeObject(builtin); + __ LoadEntryFromBuiltinIndex(builtin); + __ Jump(builtin); } } // namespace diff --git a/src/builtins/ia32/builtins-ia32.cc b/src/builtins/ia32/builtins-ia32.cc index 25c8b5f676..36efec05be 100644 --- a/src/builtins/ia32/builtins-ia32.cc +++ b/src/builtins/ia32/builtins-ia32.cc @@ -1534,6 +1534,15 @@ void Generate_ContinueToBuiltinHelper(MacroAssembler* masm, BuiltinContinuationFrameConstants::kFixedFrameSize), eax); } + + // Replace the builtin index Smi on the stack with the start address of the + // builtin loaded from the builtins table. The ret below will return to this + // address. + int offset_to_builtin_index = allocatable_register_count * kSystemPointerSize; + __ mov(eax, Operand(esp, offset_to_builtin_index)); + __ LoadEntryFromBuiltinIndex(eax); + __ mov(Operand(esp, offset_to_builtin_index), eax); + for (int i = allocatable_register_count - 1; i >= 0; --i) { int code = config->GetAllocatableGeneralCode(i); __ pop(Register::from_code(code)); @@ -1549,7 +1558,6 @@ void Generate_ContinueToBuiltinHelper(MacroAssembler* masm, kSystemPointerSize; __ pop(Operand(esp, offsetToPC)); __ Drop(offsetToPC / kSystemPointerSize); - __ add(Operand(esp, 0), Immediate(Code::kHeaderSize - kHeapObjectTag)); __ ret(0); } } // namespace diff --git a/src/builtins/x64/builtins-x64.cc b/src/builtins/x64/builtins-x64.cc index 91656e1a17..08e21f723a 100644 --- a/src/builtins/x64/builtins-x64.cc +++ b/src/builtins/x64/builtins-x64.cc @@ -1562,7 +1562,15 @@ void Generate_ContinueToBuiltinHelper(MacroAssembler* masm, kSystemPointerSize; __ popq(Operand(rsp, offsetToPC)); __ Drop(offsetToPC / kSystemPointerSize); - __ addq(Operand(rsp, 0), Immediate(Code::kHeaderSize - kHeapObjectTag)); + + // Replace the builtin index Smi on the stack with the instruction start + // address of the builtin from the builtins table, and then Ret to this + // address + __ movq(kScratchRegister, Operand(rsp, 0)); + __ movq(kScratchRegister, + __ EntryFromBuiltinIndexAsOperand(kScratchRegister)); + __ movq(Operand(rsp, 0), kScratchRegister); + __ Ret(); } } // namespace diff --git a/src/codegen/arm/macro-assembler-arm.cc b/src/codegen/arm/macro-assembler-arm.cc index f6bd2ca649..e4934b7bec 100644 --- a/src/codegen/arm/macro-assembler-arm.cc +++ b/src/codegen/arm/macro-assembler-arm.cc @@ -303,7 +303,7 @@ void TurboAssembler::Call(Handle code, RelocInfo::Mode rmode, Call(code.address(), rmode, cond, mode); } -void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { +void TurboAssembler::LoadEntryFromBuiltinIndex(Register builtin_index) { STATIC_ASSERT(kSystemPointerSize == 4); STATIC_ASSERT(kSmiShiftSize == 0); STATIC_ASSERT(kSmiTagSize == 1); @@ -316,6 +316,10 @@ void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { add(builtin_index, builtin_index, Operand(IsolateData::builtin_entry_table_offset())); ldr(builtin_index, MemOperand(kRootRegister, builtin_index)); +} + +void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { + LoadEntryFromBuiltinIndex(builtin_index); Call(builtin_index); } diff --git a/src/codegen/arm/macro-assembler-arm.h b/src/codegen/arm/macro-assembler-arm.h index cd8ea3081b..e4ce734f52 100644 --- a/src/codegen/arm/macro-assembler-arm.h +++ b/src/codegen/arm/macro-assembler-arm.h @@ -300,6 +300,9 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase { bool check_constant_pool = true); void Call(Label* target); + // Load the builtin given by the Smi in |builtin_index| into the same + // register. + void LoadEntryFromBuiltinIndex(Register builtin_index); void CallBuiltinByIndex(Register builtin_index) override; void LoadCodeObjectEntry(Register destination, Register code_object) override; diff --git a/src/codegen/arm64/macro-assembler-arm64.cc b/src/codegen/arm64/macro-assembler-arm64.cc index 5c34973b33..b1d1a05794 100644 --- a/src/codegen/arm64/macro-assembler-arm64.cc +++ b/src/codegen/arm64/macro-assembler-arm64.cc @@ -1925,7 +1925,7 @@ void TurboAssembler::Call(ExternalReference target) { Call(temp); } -void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { +void TurboAssembler::LoadEntryFromBuiltinIndex(Register builtin_index) { STATIC_ASSERT(kSystemPointerSize == 8); STATIC_ASSERT(kSmiTagSize == 1); STATIC_ASSERT(kSmiTag == 0); @@ -1941,6 +1941,10 @@ void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { #endif Add(builtin_index, builtin_index, IsolateData::builtin_entry_table_offset()); Ldr(builtin_index, MemOperand(kRootRegister, builtin_index)); +} + +void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { + LoadEntryFromBuiltinIndex(builtin_index); Call(builtin_index); } diff --git a/src/codegen/arm64/macro-assembler-arm64.h b/src/codegen/arm64/macro-assembler-arm64.h index 1c02ca3231..a4c2f10af1 100644 --- a/src/codegen/arm64/macro-assembler-arm64.h +++ b/src/codegen/arm64/macro-assembler-arm64.h @@ -852,6 +852,9 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase { // Generate an indirect call (for when a direct call's range is not adequate). void IndirectCall(Address target, RelocInfo::Mode rmode); + // Load the builtin given by the Smi in |builtin_index| into the same + // register. + void LoadEntryFromBuiltinIndex(Register builtin_index); void CallBuiltinByIndex(Register builtin_index) override; void LoadCodeObjectEntry(Register destination, Register code_object) override; diff --git a/src/codegen/ia32/macro-assembler-ia32.cc b/src/codegen/ia32/macro-assembler-ia32.cc index 845aa4b226..f6f0153e54 100644 --- a/src/codegen/ia32/macro-assembler-ia32.cc +++ b/src/codegen/ia32/macro-assembler-ia32.cc @@ -1887,7 +1887,7 @@ void TurboAssembler::Call(Handle code_object, RelocInfo::Mode rmode) { call(code_object, rmode); } -void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { +void TurboAssembler::LoadEntryFromBuiltinIndex(Register builtin_index) { STATIC_ASSERT(kSystemPointerSize == 4); STATIC_ASSERT(kSmiShiftSize == 0); STATIC_ASSERT(kSmiTagSize == 1); @@ -1900,6 +1900,10 @@ void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { mov(builtin_index, Operand(kRootRegister, builtin_index, times_half_system_pointer_size, IsolateData::builtin_entry_table_offset())); +} + +void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { + LoadEntryFromBuiltinIndex(builtin_index); call(builtin_index); } diff --git a/src/codegen/ia32/macro-assembler-ia32.h b/src/codegen/ia32/macro-assembler-ia32.h index 1ed26cfe72..9b13e87447 100644 --- a/src/codegen/ia32/macro-assembler-ia32.h +++ b/src/codegen/ia32/macro-assembler-ia32.h @@ -87,6 +87,9 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase { void Call(Label* target) { call(target); } void Call(Handle code_object, RelocInfo::Mode rmode); + // Load the builtin given by the Smi in |builtin_index| into the same + // register. + void LoadEntryFromBuiltinIndex(Register builtin_index); void CallBuiltinByIndex(Register builtin_index) override; void LoadCodeObjectEntry(Register destination, Register code_object) override; diff --git a/src/codegen/x64/macro-assembler-x64.cc b/src/codegen/x64/macro-assembler-x64.cc index 42f2b0c123..bf11534862 100644 --- a/src/codegen/x64/macro-assembler-x64.cc +++ b/src/codegen/x64/macro-assembler-x64.cc @@ -1608,7 +1608,7 @@ void TurboAssembler::Call(Handle code_object, RelocInfo::Mode rmode) { call(code_object, rmode); } -void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { +Operand TurboAssembler::EntryFromBuiltinIndexAsOperand(Register builtin_index) { #if defined(V8_COMPRESS_POINTERS) || defined(V8_31BIT_SMIS_ON_64BIT_ARCH) STATIC_ASSERT(kSmiShiftSize == 0); STATIC_ASSERT(kSmiTagSize == 1); @@ -1617,8 +1617,8 @@ void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { // The builtin_index register contains the builtin index as a Smi. // Untagging is folded into the indexing operand below (we use times_4 instead // of times_8 since smis are already shifted by one). - Call(Operand(kRootRegister, builtin_index, times_4, - IsolateData::builtin_entry_table_offset())); + return Operand(kRootRegister, builtin_index, times_4, + IsolateData::builtin_entry_table_offset()); #else // defined(V8_COMPRESS_POINTERS) || defined(V8_31BIT_SMIS_ON_64BIT_ARCH) STATIC_ASSERT(kSmiShiftSize == 31); STATIC_ASSERT(kSmiTagSize == 1); @@ -1626,11 +1626,15 @@ void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { // The builtin_index register contains the builtin index as a Smi. SmiUntag(builtin_index, builtin_index); - Call(Operand(kRootRegister, builtin_index, times_8, - IsolateData::builtin_entry_table_offset())); + return Operand(kRootRegister, builtin_index, times_8, + IsolateData::builtin_entry_table_offset()); #endif // defined(V8_COMPRESS_POINTERS) || defined(V8_31BIT_SMIS_ON_64BIT_ARCH) } +void TurboAssembler::CallBuiltinByIndex(Register builtin_index) { + Call(EntryFromBuiltinIndexAsOperand(builtin_index)); +} + void TurboAssembler::LoadCodeObjectEntry(Register destination, Register code_object) { // Code objects are called differently depending on whether we are generating diff --git a/src/codegen/x64/macro-assembler-x64.h b/src/codegen/x64/macro-assembler-x64.h index ad82b6f840..f80dfb2672 100644 --- a/src/codegen/x64/macro-assembler-x64.h +++ b/src/codegen/x64/macro-assembler-x64.h @@ -344,6 +344,7 @@ class V8_EXPORT_PRIVATE TurboAssembler : public TurboAssemblerBase { void Call(ExternalReference ext); void Call(Label* target) { call(target); } + Operand EntryFromBuiltinIndexAsOperand(Register builtin_index); void CallBuiltinByIndex(Register builtin_index) override; void LoadCodeObjectEntry(Register destination, Register code_object) override; diff --git a/src/deoptimizer/deoptimizer.cc b/src/deoptimizer/deoptimizer.cc index 748d76daa2..cbb6ffe5b7 100644 --- a/src/deoptimizer/deoptimizer.cc +++ b/src/deoptimizer/deoptimizer.cc @@ -1424,7 +1424,7 @@ Builtins::Name Deoptimizer::TrampolineForBuiltinContinuation( // +-------------------------+ // | context |<- this non-standard context slot contains // +-------------------------+ the context, even for non-JS builtins. -// | builtin address | +// | builtin index | // +-------------------------+ // | builtin input GPR reg0 |<- populated from deopt FrameState using // +-------------------------+ the builtin's CallInterfaceDescriptor @@ -1649,7 +1649,8 @@ void Deoptimizer::DoComputeBuiltinContinuation( "builtin JavaScript context\n"); // The builtin to continue to. - frame_writer.PushRawObject(builtin, "builtin address\n"); + frame_writer.PushRawObject(Smi::FromInt(builtin.builtin_index()), + "builtin index\n"); for (int i = 0; i < allocatable_register_count; ++i) { int code = config->GetAllocatableGeneralCode(i); diff --git a/src/execution/frame-constants.h b/src/execution/frame-constants.h index 7ddee5689e..e2a17b4a85 100644 --- a/src/execution/frame-constants.h +++ b/src/execution/frame-constants.h @@ -271,7 +271,7 @@ class BuiltinContinuationFrameConstants : public TypedFrameConstants { TYPED_FRAME_PUSHED_VALUE_OFFSET(1); static constexpr int kBuiltinContextOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(2); - static constexpr int kBuiltinOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(3); + static constexpr int kBuiltinIndexOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(3); // The argument count is in the first allocatable register, stored below the // fixed part of the frame and therefore is not part of the fixed frame size.