From 169bdfe40814b648bd0e1fe758ad00e28008c89f Mon Sep 17 00:00:00 2001 From: Victor Gomes Date: Wed, 11 Jan 2023 15:07:45 +0100 Subject: [PATCH] [maglev] Fix CheckJSDataViewBounds clobbered argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the register allocator assigns kJavaScriptCallArgCountRegister to {object}, we were clobbering the object, before pushing it to the stack. Additionally, we use PushReverse instead of Push to indicate that kDataViewPrototypeGetByteLength has a JS call convention (arguments are reversed). This is a no-op for x64, but it guarantees the correct order of the padding in arm64. Fixed: chromium:1406456 Bug: v8:7700, v8:13645 Change-Id: Ia9126ff5315ab4ab08ae733f138a1e0cb2d021a2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4156053 Auto-Submit: Victor Gomes Reviewed-by: Marja Hölttä Commit-Queue: Victor Gomes Cr-Commit-Position: refs/heads/main@{#85227} --- src/maglev/arm64/maglev-ir-arm64.cc | 2 +- src/maglev/x64/maglev-ir-x64.cc | 2 +- test/mjsunit/maglev/regress-1406456.js | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 test/mjsunit/maglev/regress-1406456.js diff --git a/src/maglev/arm64/maglev-ir-arm64.cc b/src/maglev/arm64/maglev-ir-arm64.cc index 4a9b27cf74..6ac1d57dad 100644 --- a/src/maglev/arm64/maglev-ir-arm64.cc +++ b/src/maglev/arm64/maglev-ir-arm64.cc @@ -1438,9 +1438,9 @@ void CheckJSDataViewBounds::GenerateCode(MaglevAssembler* masm, // TODO(v8:7700): Inline DataViewPrototypeGetByteLength or create a // different builtin that does not re-check the DataView object. SaveRegisterStateForCall save_register_state(masm, snapshot); + __ PushReverse(object); __ Mov(kContextRegister, masm->native_context().object()); __ Mov(kJavaScriptCallArgCountRegister, 1); - __ Push(object); __ CallBuiltin(Builtin::kDataViewPrototypeGetByteLength); } __ SmiUntag(byte_length, kReturnRegister0); diff --git a/src/maglev/x64/maglev-ir-x64.cc b/src/maglev/x64/maglev-ir-x64.cc index 86023dd83f..1a37b546f2 100644 --- a/src/maglev/x64/maglev-ir-x64.cc +++ b/src/maglev/x64/maglev-ir-x64.cc @@ -536,9 +536,9 @@ void CheckJSDataViewBounds::GenerateCode(MaglevAssembler* masm, // TODO(v8:7700): Inline DataViewPrototypeGetByteLength or create a // different builtin that does not re-check the DataView object. SaveRegisterStateForCall save_register_state(masm, snapshot); + __ PushReverse(object); __ Move(kContextRegister, masm->native_context().object()); __ Move(kJavaScriptCallArgCountRegister, 1); - __ Push(object); __ CallBuiltin(Builtin::kDataViewPrototypeGetByteLength); } __ SmiUntag(byte_length, kReturnRegister0); diff --git a/test/mjsunit/maglev/regress-1406456.js b/test/mjsunit/maglev/regress-1406456.js new file mode 100644 index 0000000000..5c6edf3e71 --- /dev/null +++ b/test/mjsunit/maglev/regress-1406456.js @@ -0,0 +1,17 @@ +// Copyright 2022 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: --allow-natives-syntax --maglev --harmony-rab-gsab + +function foo() { + const buffer = new SharedArrayBuffer(1395, { + "maxByteLength": 2110270, + }); + const data = new DataView(buffer); + data.setInt16(); +} +%PrepareFunctionForOptimization(foo); +foo(); +%OptimizeMaglevOnNextCall(foo); +foo();