Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] x86/x64: Add more red zone checks to assembler backend.
Date: Thu, 16 Jan 2025 17:55:21 +0300	[thread overview]
Message-ID: <e177a0ed-27b0-46b1-bbe3-bbcec1d28f8c@tarantool.org> (raw)
In-Reply-To: <20250116133559.2686-1-skaplun@tarantool.org>

[-- Attachment #1: Type: text/plain, Size: 2620 bytes --]

Hi, Sergey,

thanks for the patch! See comments below.


On 16.01.2025 16:35, Sergey Kaplun wrote:


<snipped>

> 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 00000000..70062ec9
> --- /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.
I would rephrase: XXX: Test fails with reverted fix and enabled GC64.
> +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.
> +-- Seehttps://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`.

I didn't get it.


<snipped>

[-- Attachment #2: Type: text/html, Size: 3855 bytes --]

  reply	other threads:[~2025-01-16 14:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-16 13:35 Sergey Kaplun via Tarantool-patches
2025-01-16 14:55 ` Sergey Bronnikov via Tarantool-patches [this message]
2025-01-17  8:31   ` Sergey Kaplun via Tarantool-patches
2025-01-17 10:07     ` Sergey Bronnikov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e177a0ed-27b0-46b1-bbe3-bbcec1d28f8c@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] x86/x64: Add more red zone checks to assembler backend.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox