Commit 7506e063 authored by Thibaud Michaud's avatar Thibaud Michaud Committed by Commit Bot

[codegen] Skip invalid optimization in tail calls

Preparing for tail call is usually done by emitting the gap moves and
then moving the stack pointer to its new position. An optimization
consists in moving the stack pointer first and transforming some of the
moves into pushes. In the attached case it looks like this (arm):

138  add sp, sp, #40
13c  str r6, [sp, #-4]!
140  str r6, [sp, #-4]!
144  str r6, [sp, #-4]!
148  str r6, [sp, #-4]!
14c  str r6, [sp, #-4]!
...
160  vldr d1, [sp - 4*3]

The last line is a gap reload, but because the stack pointer was already
moved, the slot is now below the stack pointer. This is invalid and
triggers this DCHECK:

Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
Debug check failed: 0 <= offset (0 vs. -12).

A comment already explains that we skip the optimization if the gap
contains stack moves to prevent this, but the code only checks for
non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
"source.IsAnyStackSlot()":

108  vldr d1, [sp + 4*2]
...
118  str r0, [sp, #+36]
11c  str r0, [sp, #+32]
120  str r0, [sp, #+28]
124  str r0, [sp, #+24]
128  str r0, [sp, #+20]
...
134  add sp, sp, #20

R=jgruber@chromium.org

Bug: chromium:1137608
Change-Id: If2b85dde49bf31a6bd3f5e0255407f9390727f9d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474784Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70603}
parent 853c17a9
......@@ -615,8 +615,8 @@ void CodeGenerator::GetPushCompatibleMoves(Instruction* instr,
// then the full gap resolver must be used since optimization with
// pushes don't participate in the parallel move and might clobber
// values needed for the gap resolve.
if (source.IsStackSlot() && LocationOperand::cast(source).index() >=
first_push_compatible_index) {
if (source.IsAnyStackSlot() && LocationOperand::cast(source).index() >=
first_push_compatible_index) {
pushes->clear();
return;
}
......
// Copyright 2020 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.
//
// Flags: --no-liftoff --experimental-wasm-return-call --experimental-wasm-threads
load("test/mjsunit/wasm/wasm-module-builder.js");
(function Regress1137608() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
let sig0 = builder.addType(kSig_i_iii);
let sig1 = builder.addType(makeSig([kWasmF64, kWasmF64, kWasmI32,
kWasmI32, kWasmI32, kWasmF32, kWasmI32, kWasmF64, kWasmI32, kWasmF32,
kWasmI32, kWasmF32, kWasmI32, kWasmF64, kWasmI32], [kWasmI32]));
let main = builder.addFunction("main", sig0)
.addBody([
kExprI64Const, 0,
kExprF64UConvertI64,
kExprF64Const, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x00, 0x00,
kExprF64Const, 0x30, 0x30, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64Mul,
kExprI32Const, 0,
kExprF64Const, 0x30, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64StoreMem, 0x00, 0xb0, 0xe0, 0xc0, 0x81, 0x03,
kExprI32Const, 0,
kExprI32Const, 0,
kExprI32Const, 0,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprI32Const, 2,
kExprReturnCallIndirect, sig1, kTableZero]).exportFunc();
builder.addFunction("f", sig1).addBody([kExprI32Const, 0]);
builder.addTable(kWasmAnyFunc, 4, 4);
builder.addMemory(16, 32, false, true);
let module = new WebAssembly.Module(builder.toBuffer());
let instance = new WebAssembly.Instance(module);
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment