[maglev] Force (U)Int32 values to always be zero extended

Ensure that (U)Int32 values are always zero extended (in particular,
after Float64 truncation and constant materialisation), and add debug
code which asserts that (U)Int32 register inputs to nodes are zero
extended at input read time.

Bug: v8:7700
Change-Id: Idbebabdd48bc7a6d2d73f1dfce7da629b5814ca5
Fixed: chromium:1404066
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4147621
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85169}
This commit is contained in:
Leszek Swirski 2023-01-09 17:44:28 +01:00 committed by V8 LUCI CQ
parent 5c9560658b
commit fe54336953
9 changed files with 46 additions and 4 deletions

View File

@ -1589,6 +1589,13 @@ void MacroAssembler::AssertNotSmi(Register object, AbortReason reason) {
Check(ne, reason);
}
void TurboAssembler::AssertZeroExtended(Register int32_register) {
if (!v8_flags.debug_code) return;
ASM_CODE_COMMENT(this);
Tst(int32_register.X(), kMaxUInt32);
Check(ls, AbortReason::k32BitValueInRegisterIsNotZeroExtended);
}
void MacroAssembler::AssertCodeT(Register object) {
if (!v8_flags.debug_code) return;
ASM_CODE_COMMENT(this);

View File

@ -1910,6 +1910,10 @@ class V8_EXPORT_PRIVATE MacroAssembler : public TurboAssembler {
AbortReason reason = AbortReason::kOperandIsASmi)
NOOP_UNLESS_DEBUG_CODE
// Abort execution if a 64 bit register containing a 32 bit payload does
// not have zeros in the top 32 bits, enabled via --debug-code.
void AssertZeroExtended(Register int32_register) NOOP_UNLESS_DEBUG_CODE
// Abort execution if argument is not a CodeT, enabled via --debug-code.
void AssertCodeT(Register object) NOOP_UNLESS_DEBUG_CODE

View File

@ -439,7 +439,7 @@ inline void MaglevAssembler::Move(Register dst, TaggedIndex i) {
Mov(dst, i.ptr());
}
inline void MaglevAssembler::Move(Register dst, int32_t i) {
Mov(dst, Immediate(i));
Mov(dst.W(), Immediate(i));
}
inline void MaglevAssembler::Move(DoubleRegister dst, double n) {
Fmov(dst, n);

View File

@ -371,6 +371,7 @@ void MaglevAssembler::StringFromCharCode(RegisterSnapshot register_snapshot,
Label* char_code_fits_one_byte,
Register result, Register char_code,
Register scratch) {
AssertZeroExtended(char_code);
DCHECK_NE(char_code, scratch);
ZoneLabelRef done(this);
Cmp(char_code, Immediate(String::kMaxOneByteCharCode));
@ -578,6 +579,8 @@ void MaglevAssembler::TruncateDoubleToInt32(Register dst, DoubleRegister src) {
src, dst, done);
Bind(*done);
// Zero extend the converted value to complete the truncation.
Mov(dst, Operand(dst.W(), UXTW));
}
} // namespace maglev

View File

@ -7,7 +7,9 @@
#include "src/codegen/machine-type.h"
#include "src/codegen/macro-assembler.h"
#include "src/flags/flags.h"
#include "src/maglev/maglev-code-gen-state.h"
#include "src/maglev/maglev-ir.h"
namespace v8 {
namespace internal {

View File

@ -638,6 +638,22 @@ class MaglevCodeGeneratingNodeProcessor {
state);
}
if (v8_flags.debug_code) {
// Check that all int32/uint32 inputs are zero extended
for (Input& input : *node) {
ValueRepresentation rep =
input.node()->properties().value_representation();
if (rep == ValueRepresentation::kInt32 ||
rep == ValueRepresentation::kUint32) {
// TODO(leszeks): Ideally we'd check non-register inputs too, but
// AssertZeroExtended needs the scratch register, so we'd have to do
// some manual push/pop here to free up another register.
if (input.IsGeneralRegister()) {
__ AssertZeroExtended(ToRegister(input));
}
}
}
}
node->GenerateCode(masm(), state);
if (std::is_base_of<ValueNode, NodeT>::value) {

View File

@ -325,7 +325,8 @@ inline void MaglevAssembler::Move(Register dst, Register src) {
}
inline void MaglevAssembler::Move(Register dst, int32_t i) {
MacroAssembler::Move(dst, Immediate(i));
// Move as a uint32 to avoid sign extension.
MacroAssembler::Move(dst, static_cast<uint32_t>(i));
}
inline void MaglevAssembler::Move(DoubleRegister dst, double n) {

View File

@ -100,8 +100,7 @@ void MaglevAssembler::AllocateTwoByteString(RegisterSnapshot register_snapshot,
void MaglevAssembler::LoadSingleCharacterString(Register result,
Register char_code,
Register scratch) {
// Make sure char_code is zero extended.
movl(char_code, char_code);
AssertZeroExtended(char_code);
if (v8_flags.debug_code) {
cmpq(char_code, Immediate(String::kMaxOneByteCharCode));
Assert(below_equal, AbortReason::kUnexpectedValue);
@ -365,6 +364,8 @@ void MaglevAssembler::TruncateDoubleToInt32(Register dst, DoubleRegister src) {
},
src, dst, done);
bind(*done);
// Zero extend the converted value to complete the truncation.
movl(dst, dst);
}
void MaglevAssembler::Prologue(Graph* graph) {

View File

@ -792,6 +792,10 @@ void LoadTaggedElement::GenerateCode(MaglevAssembler* masm,
__ DecompressAnyTagged(kScratchRegister,
FieldOperand(object, JSObject::kElementsOffset));
}
if (v8_flags.debug_code) {
__ cmpq(index, Immediate(0));
__ Assert(above_equal, AbortReason::kUnexpectedNegativeValue);
}
__ DecompressAnyTagged(
result_reg, FieldOperand(kScratchRegister, index, times_tagged_size,
FixedArray::kHeaderSize));
@ -822,6 +826,10 @@ void LoadDoubleElement::GenerateCode(MaglevAssembler* masm,
__ DecompressAnyTagged(kScratchRegister,
FieldOperand(object, JSObject::kElementsOffset));
}
if (v8_flags.debug_code) {
__ cmpq(index, Immediate(0));
__ Assert(above_equal, AbortReason::kUnexpectedNegativeValue);
}
__ Movsd(result_reg, FieldOperand(kScratchRegister, index, times_8,
FixedDoubleArray::kHeaderSize));
}