From bce0e5596844c005a46603c3364139cb008c3d39 Mon Sep 17 00:00:00 2001 From: Mike Pall Date: Sun, 12 Nov 2023 15:18:44 +0100 Subject: [PATCH] x86/x64: Add more red zone checks to assembler backend. Thanks to Peter Cawley. (cherry picked from commit d854d00ce94b274359e5181bed13e977420daf5c) Assembling some instructions (like `IR_CONV int.num`, for example) with many mcode to be emitted may overflow the `MCLIM_REDZONE` (64) at once due to the huge mcode emitting. For example `IR_CONV` in this test requires 66 bytes of the machine code: | cvttsd2si r15d, xmm5 | xorps xmm9, xmm9 | cvtsi2sd xmm9, r15d | ucomisd xmm5, xmm9 | jnz 0x11edb00e5 ->37 | jpe 0x11edb00e5 ->37 | mov [rsp+0x80], r15d | mov r15, [rsp+0xe8] | movsd xmm9, [rsp+0xe0] | movsd xmm5, [rsp+0xd8] The reproducer needs sufficient register pressure as to immediately spill the result of the instruction to the stack and then reload the three registers used by the instruction, and to have chosen enough registers with numbers >=8 (because shaving off a REX prefix [1] or two would get 66 back down to <= `MCLIM_REDZONE`), and to be using lots of spill slots (because memory offsets <= 0x7f are shorter to encode compared to those >= 0x80. So, each reload instruction consumes 9 bytes. This makes this reproducer unstable (regarding the register allocator changes). Thus, only original test case is added as a regression test. This patch adds the red zone overflow checks more often for the IRs with many instructions to be emitted. Sergey Kaplun: * added the description for the problem [1]: https://wiki.osdev.org/X86-64_Instruction_Encoding#REX_prefix Part of tarantool/tarantool#10709 --- src/lj_asm_x86.h | 8 +- .../lj-1116-redzones-checks.test.lua | 118 ++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/lj-1116-redzones-checks.test.lua diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h index a96bc2e7ac..4ae721a45d 100644 --- a/src/lj_asm_x86.h +++ b/src/lj_asm_x86.h @@ -794,6 +794,7 @@ static void asm_tointg(ASMState *as, IRIns *ir, Reg left) emit_rr(as, XO_UCOMISD, left, tmp); emit_rr(as, XO_CVTSI2SD, tmp, dest); emit_rr(as, XO_XORPS, tmp, tmp); /* Avoid partial register stall. */ + checkmclim(as); emit_rr(as, XO_CVTTSD2SI, dest, left); /* Can't fuse since left is needed twice. */ } @@ -836,6 +837,7 @@ static void asm_conv(ASMState *as, IRIns *ir) emit_rr(as, XO_SUBSD, dest, bias); /* Subtract 2^52+2^51 bias. */ emit_rr(as, XO_XORPS, dest, bias); /* Merge bias and integer. */ emit_rma(as, XO_MOVSD, bias, k); + checkmclim(as); emit_mrm(as, XO_MOVD, dest, asm_fuseload(as, lref, RSET_GPR)); return; } else { /* Integer to FP conversion. */ @@ -1151,6 +1153,7 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) asm_guardcc(as, CC_E); else emit_sjcc(as, CC_E, l_end); + checkmclim(as); if (irt_isnum(kt)) { if (isk) { /* Assumes -0.0 is already canonicalized to +0.0. */ @@ -1210,7 +1213,6 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) #endif } emit_sfixup(as, l_loop); - checkmclim(as); #if LJ_GC64 if (!isk && irt_isaddr(kt)) { emit_rr(as, XO_OR, tmp|REX_64, key); @@ -1242,6 +1244,7 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) emit_rr(as, XO_ARITH(XOg_SUB), dest, tmp); emit_shifti(as, XOg_ROL, tmp, HASH_ROT3); emit_rr(as, XO_ARITH(XOg_XOR), dest, tmp); + checkmclim(as); emit_shifti(as, XOg_ROL, dest, HASH_ROT2); emit_rr(as, XO_ARITH(XOg_SUB), tmp, dest); emit_shifti(as, XOg_ROL, dest, HASH_ROT1); @@ -1259,7 +1262,6 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) } else { emit_rr(as, XO_MOV, tmp, key); #if LJ_GC64 - checkmclim(as); emit_gri(as, XG_ARITHi(XOg_XOR), dest, irt_toitype(kt) << 15); if ((as->flags & JIT_F_BMI2)) { emit_i8(as, 32); @@ -1530,6 +1532,7 @@ static void asm_ahuvload(ASMState *as, IRIns *ir) if (irt_islightud(ir->t)) { Reg dest = asm_load_lightud64(as, ir, 1); if (ra_hasreg(dest)) { + checkmclim(as); asm_fuseahuref(as, ir->op1, RSET_GPR); emit_mrm(as, XO_MOV, dest|REX_64, RID_MRM); } @@ -1574,6 +1577,7 @@ static void asm_ahuvload(ASMState *as, IRIns *ir) if (LJ_64 && irt_type(ir->t) >= IRT_NUM) { lj_assertA(irt_isinteger(ir->t) || irt_isnum(ir->t), "bad load type %d", irt_type(ir->t)); + checkmclim(as); #if LJ_GC64 emit_u32(as, LJ_TISNUM << 15); #else diff --git a/test/tarantool-tests/lj-1116-redzones-checks.test.lua b/test/tarantool-tests/lj-1116-redzones-checks.test.lua new file mode 100644 index 0000000000..70062ec9a7 --- /dev/null +++ b/test/tarantool-tests/lj-1116-redzones-checks.test.lua @@ -0,0 +1,118 @@ +local tap = require('tap') +-- Test file to demonstrate mcode area overflow during recording a +-- trace with the high FPR pressure. +-- See also, https://github.com/LuaJIT/LuaJIT/issues/1116. +-- +-- XXX: Test fails only with GC64 enabled before the commit. +local test = tap.test('lj-1116-redzones-checks'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +jit.opt.start('hotloop=1') + +-- XXX: This test snippet was originally created by the fuzzer. +-- See https://oss-fuzz.com/testcase-detail/5622965122170880. +-- +-- Unfortunately, it's impossible to reduce the testcase further. +-- Before the patch, assembling some instructions (like `IR_CONV +-- int.num`, for example) with many mcode to be emitted may +-- overflow the `MCLIM_REDZONE` (64) at once due to the huge +-- mcode emitting. +-- For example `IR_CONV` in this test requires 66 bytes of the +-- machine code: +-- | cvttsd2si r15d, xmm5 +-- | xorps xmm9, xmm9 +-- | cvtsi2sd xmm9, r15d +-- | ucomisd xmm5, xmm9 +-- | jnz 0x11edb00e5 ->37 +-- | jpe 0x11edb00e5 ->37 +-- | mov [rsp+0x80], r15d +-- | mov r15, [rsp+0xe8] +-- | movsd xmm9, [rsp+0xe0] +-- | movsd xmm5, [rsp+0xd8] +-- +-- The reproducer needs sufficient register pressure as to +-- immediately spill the result of the instruction to the stack +-- and then reload the three registers used by the instruction, +-- and to have chosen enough registers with numbers >=8 (because +-- shaving off a REX prefix [1] or two would get 66 back down +-- to <= `MCLIM_REDZONE`), and to be using lots of spill slots +-- (because memory offsets <= 0x7f are shorter to encode compared +-- to those >= 0x80. So, each reload instruction consumes 9 bytes. +-- This makes this reproducer unstable (regarding the register +-- allocator changes). So, lets use this as a regression test. +-- +-- [1]: https://wiki.osdev.org/X86-64_Instruction_Encoding#REX_prefix + +_G.a = 0 +_G.b = 0 +_G.c = 0 +_G.d = 0 +_G.e = 0 +_G.f = 0 +_G.g = 0 +_G.h = 0 +-- Skip `i`. +_G.j = 0 +_G.k = 0 +_G.l = 0 +_G.m = 0 +_G.n = 0 +_G.o = 0 +_G.p = 0 +_G.q = 0 +_G.r = 0 +_G.s = 0 +_G.t = 0 +_G.u = 0 +_G.v = 0 +_G.w = 0 +_G.x = 0 +_G.y = 0 +_G.z = 0 + +-- XXX: Need here not 4, but 4.5 top border of the cycle to create +-- FPR pressure. +for i = 1, 4.5 do + _G.a = _G.a + 1 + _G.b = _G.b + 1 + _G.c = _G.c + 1 + _G.d = _G.d + 1 + for _ = i, 2 do + _G.e = _G.e + 1 + end + -- Here we emit `IR_CONV int.num`. This loop is inlined. + -- Assertion failed after emitting the variant part of the + -- big loop. + for _ = 2, i do + _G.f = _G.f + 1 + _G.g = _G.g + 1 + _G.h = _G.h + 1 + _G.j = _G.j + 1 + _G.k = _G.k + 1 + _G.l = _G.l + 1 + end + _G.m = _G.m + 1 + _G.n = _G.n + 1 + _G.o = _G.o + 1 + _G.p = _G.p + 1 + _G.q = _G.q + 1 + _G.r = _G.r + 1 + _G.s = _G.s + 1 + _G.t = _G.t + 1 + _G.u = _G.u + 1 + _G.v = _G.v + 1 + for _ = i, 2.1 do + _G.aa = _G.a + _G.w = _G.w + 1 + _G.x = _G.x + 1 + _G.y = _G.y + 1 + _G.z = _G.z + 1 + end +end + +test:ok(true, 'no mcode limit assertion failed during recording') + +test:done(true)