[Tarantool-patches] [PATCH luajit 1/2] Fix register mask for stack check in head of side trace.
Maxim Kokryashkin
m.kokryashkin at tarantool.org
Fri Oct 13 08:48:30 MSK 2023
Hi, Sergey!
Thanks for the patch!
LGTM, except for a few nits below.
On Wed, Oct 11, 2023 at 06:04:09PM +0300, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Analyzed by Sergey Kaplun.
>
> (cherry-picked from commit b7a8c7c184257858699454408420dd5f0b6c8a75)
>
> Assume we have parent and child traces with the following IRs from the
> IR dump:
>
> Parent:
> | 0009 rax > tab TNEW 0 0
> | 0010 p32 FLOAD 0008 tab.node
> | 0011 p32 HREFK 0010 "Name" @1
> | 0012 {0008} tab HSTORE 0011 0009
> | .... SNAP 2 [ ---- 0001 0002 0008 ---- ]
> | 0013 {sink} tab TNEW 0 0
> | 0014 {0008} fal HSTORE 0011 false
> | .... SNAP 3 [ ---- 0001 0002 0008 ---- ]
>
> Child:
> | 0001 r15 tab SLOAD 1 PI
> | 0002 rbp tab SLOAD 2 PI
> | 0003 tab PVAL 9
>
> As we can see from the trace dump above, the `rax` register is missing
> in the `0003 PVAL` IR for the side trace -- so it is assumed to be
> available in the allow RegSet inside `asm_stack_check()` and its value
> is spoiled during this check, so if we are restoring from the 3rd
Typo: s/spoiled/spilled/
> snapshot by stack overflow -- we are in trouble.
>
> The moment when IR is spoiled is when we set a hint on the register
Typo: s/spoiled/spilled/
> inherited from the parent trace (see `asm_setup_regsp()` for details).
> The 0th register (`rax`) shapeshifts into `RID_NONE`. Hence, when
> collecting register dependencies from the parent trace, `0003 PVAL` is
> considered the IR with `RID_NONE`, i.e., without an assigned register.
> So, this register is considered free (picked as bottom from the free
> set) in the `asm_stack_check()` and is used for stack overflow check, so
> the table reference is gone.
>
> This patch introduces another register set for the context of the parent
> trace to use in the stack check. All registers used on the child trace
> are excluded from this set.
>
> The test case for this patch is omitted since it requires specific
> register allocation, which is hard to construct and not stable in any
> future patches.
>
> Part of tarantool/tarantool#9145
>
> Sergey Kaplun:
> * added the description for the problem
> ---
> src/lj_asm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/lj_asm.c b/src/lj_asm.c
> index 3a1909d5..ca06860a 100644
> --- a/src/lj_asm.c
> +++ b/src/lj_asm.c
> @@ -1859,6 +1859,7 @@ static void asm_head_side(ASMState *as)
> IRRef1 sloadins[RID_MAX];
> RegSet allow = RSET_ALL; /* Inverse of all coalesced registers. */
> RegSet live = RSET_EMPTY; /* Live parent registers. */
> + RegSet pallow = RSET_GPR; /* Registers needed by the parent stack check. */
> IRIns *irp = &as->parent->ir[REF_BASE]; /* Parent base. */
> int32_t spadj, spdelta;
> int pass2 = 0;
> @@ -1899,6 +1900,7 @@ static void asm_head_side(ASMState *as)
> sloadins[rs] = (IRRef1)i;
> rset_set(live, rs); /* Block live parent register. */
> }
> + if (!ra_hasspill(regsp_spill(rs))) rset_clear(pallow, regsp_reg(rs));
> }
>
> /* Calculate stack frame adjustment. */
> @@ -2015,7 +2017,7 @@ static void asm_head_side(ASMState *as)
> ExitNo exitno = as->J->exitno;
> #endif
> as->T->topslot = (uint8_t)as->topslot; /* Remember for child traces. */
> - asm_stack_check(as, as->topslot, irp, allow & RSET_GPR, exitno);
> + asm_stack_check(as, as->topslot, irp, pallow, exitno);
> }
> }
>
> --
> 2.42.0
>
More information about the Tarantool-patches
mailing list