From 1ab3888fd2f39392ffad3b0e3095a0d9d334313f Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Tue, 18 Sep 2018 13:13:11 +0200 Subject: [PATCH] [ia32] Remove invalid indirect call/jump code Indirect calls need a scratch register to load the target address. On ia32 there's no easily-available scratch register. This removes invalid code and documents a potential solution. But ideally, this will remain unreachable since all inter-builtin calls will be pc-relative. Bug: v8:6666 Change-Id: I19e0ac699ee4757e3d5ec130b3e34a67cd1f851c Reviewed-on: https://chromium-review.googlesource.com/1230096 Reviewed-by: Sigurd Schneider Commit-Queue: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#55999} --- src/ia32/macro-assembler-ia32.cc | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index e3953b6c24..32bf8e9b04 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -1725,14 +1725,16 @@ void TurboAssembler::CallCFunction(Register function, int num_arguments) { void TurboAssembler::Call(Handle code_object, RelocInfo::Mode rmode) { if (FLAG_embedded_builtins) { - // TODO(jgruber): Figure out which register we can clobber here. // TODO(jgruber): Pc-relative builtin-to-builtin calls. - Register scratch = kOffHeapTrampolineRegister; if (root_array_available_ && options().isolate_independent_code) { - IndirectLoadConstant(scratch, code_object); - lea(scratch, FieldOperand(scratch, Code::kHeaderSize)); - call(scratch); - return; + // TODO(jgruber): There's no scratch register on ia32. Any call that + // requires loading a code object from the builtins constant table must: + // 1) spill two scratch registers, 2) load the target into scratch1, 3) + // store the target into a virtual register on the isolate using scratch2, + // 4) restore both scratch registers, and finally 5) call through the + // virtual register. All affected call sites should vanish once all + // builtins are embedded on ia32. + UNREACHABLE(); } else if (options().inline_offheap_trampolines) { int builtin_index = Builtins::kNoBuiltinId; if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index) && @@ -1753,14 +1755,16 @@ void TurboAssembler::Call(Handle code_object, RelocInfo::Mode rmode) { void TurboAssembler::Jump(Handle code_object, RelocInfo::Mode rmode) { if (FLAG_embedded_builtins) { - // TODO(jgruber): Figure out which register we can clobber here. // TODO(jgruber): Pc-relative builtin-to-builtin calls. - Register scratch = kOffHeapTrampolineRegister; if (root_array_available_ && options().isolate_independent_code) { - IndirectLoadConstant(scratch, code_object); - lea(scratch, FieldOperand(scratch, Code::kHeaderSize)); - jmp(scratch); - return; + // TODO(jgruber): There's no scratch register on ia32. Any call that + // requires loading a code object from the builtins constant table must: + // 1) spill two scratch registers, 2) load the target into scratch1, 3) + // store the target into a virtual register on the isolate using scratch2, + // 4) restore both scratch registers, and finally 5) call through the + // virtual register. All affected call sites should vanish once all + // builtins are embedded on ia32. + UNREACHABLE(); } else if (options().inline_offheap_trampolines) { int builtin_index = Builtins::kNoBuiltinId; if (isolate()->builtins()->IsBuiltinHandle(code_object, &builtin_index) &&