From 59bb18867addf9f1d4b64eb6e1f0cf3996da07e0 Mon Sep 17 00:00:00 2001 From: Andreas Haas Date: Wed, 8 Feb 2017 10:58:44 +0100 Subject: [PATCH] [x64] Consider both operands when emitting the REX prefix for testb. The testb instruction requires the REX prefix when either of its operands uses a register with the high bit set. The existing code only considered the register operand. In the test case the REX prefix was not emitted because the testb instruction had the register operand RAX which does not have the high bit set. The REX prefix was necessary though because the memory operand used R8, which has the high bit set. R=bmeurer@chromium.org BUG=chromium:688876 Change-Id: Ib214bebbe75965664f2aea530e29afa95a54f44f Reviewed-on: https://chromium-review.googlesource.com/439145 Commit-Queue: Andreas Haas Reviewed-by: Benedikt Meurer Cr-Commit-Position: refs/heads/master@{#43030} --- src/x64/assembler-x64.cc | 7 +++- .../mjsunit/regress/wasm/regression-688876.js | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/mjsunit/regress/wasm/regression-688876.js diff --git a/src/x64/assembler-x64.cc b/src/x64/assembler-x64.cc index 5e06322e2e..e7a53f18f3 100644 --- a/src/x64/assembler-x64.cc +++ b/src/x64/assembler-x64.cc @@ -2173,7 +2173,12 @@ void Assembler::emit_test(const Operand& op, Register reg, int size) { bool byte_operand = size == sizeof(int8_t); if (byte_operand) { size = sizeof(int32_t); - if (!reg.is_byte_register()) emit_rex_32(reg, op); + if (!reg.is_byte_register()) { + // Register is not one of al, bl, cl, dl. Its encoding needs REX. + emit_rex_32(reg, op); + } else { + emit_optional_rex_32(reg, op); + } } else { emit_rex(reg, op, size); } diff --git a/test/mjsunit/regress/wasm/regression-688876.js b/test/mjsunit/regress/wasm/regression-688876.js new file mode 100644 index 0000000000..83bebbb802 --- /dev/null +++ b/test/mjsunit/regress/wasm/regression-688876.js @@ -0,0 +1,42 @@ +// Copyright 2017 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. + +load('test/mjsunit/wasm/wasm-constants.js'); +load('test/mjsunit/wasm/wasm-module-builder.js'); + +(function() { + var builder = new WasmModuleBuilder(); + builder.addMemory(16, 32, false); + builder.addFunction('test', kSig_i_i) + .addBodyWithEnd([ + kExprI32Const, 0x41, + kExprI32Const, 0x45, + kExprI32Const, 0x41, + kExprI32DivU, + kExprI32LoadMem8S, 0x00, 0x3a, + kExprI32Const, 0x75, + kExprI32Const, 0x75, + kExprI32Const, 0x6e, + kExprI32Eqz, + kExprI32LoadMem8S, 0x00, 0x3a, + kExprI32Add, + kExprI32DivU, + kExprI32LoadMem8S, 0x00, 0x74, + kExprI32And, + kExprI32Eqz, + kExprI32And, +kExprGrowMemory, 0x00, + kExprI32Const, 0x55, + kExprI32LoadMem8S, 0x00, 0x3a, + kExprI32LoadMem16U, 0x00, 0x71, + kExprI32Const, 0x00, + kExprI32RemU, + kExprI32And, +kExprI32Eqz, +kExprEnd, // @44 + ]) + .exportFunc(); + var module = builder.instantiate(); + assertThrows(() => {module.exports.test(1);}); +})();