Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>,
	Sergey Bronnikov <sergeyb@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH luajit 1/2] Fix register mask for stack check in head of side trace.
Date: Wed, 11 Oct 2023 18:04:09 +0300	[thread overview]
Message-ID: <9b2713c1405073d42c14f3179ff5a8fb338a37dd.1697034851.git.skaplun@tarantool.org> (raw)
In-Reply-To: <cover.1697034851.git.skaplun@tarantool.org>

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
snapshot by stack overflow -- we are in trouble.

The moment when IR is spoiled is when we set a hint on the register
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


  reply	other threads:[~2023-10-11 15:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-11 15:04 [Tarantool-patches] [PATCH luajit 0/2] Fix assembling of the head of side traces Sergey Kaplun via Tarantool-patches
2023-10-11 15:04 ` Sergey Kaplun via Tarantool-patches [this message]
2023-10-13  5:48   ` [Tarantool-patches] [PATCH luajit 1/2] Fix register mask for stack check in head of side trace Maxim Kokryashkin via Tarantool-patches
2023-10-23  9:27     ` Sergey Kaplun via Tarantool-patches
2023-10-24 13:51       ` Maxim Kokryashkin via Tarantool-patches
2023-11-07 18:42   ` Igor Munkin via Tarantool-patches
2023-10-11 15:04 ` [Tarantool-patches] [PATCH luajit 2/2] Fix base register coalescing in " Sergey Kaplun via Tarantool-patches
2023-10-13  5:59   ` Maxim Kokryashkin via Tarantool-patches
2023-10-23  9:50     ` Sergey Kaplun via Tarantool-patches
2023-10-24 13:50       ` Maxim Kokryashkin via Tarantool-patches
2023-11-07 18:42   ` Igor Munkin via Tarantool-patches
2023-11-23  6:30 ` [Tarantool-patches] [PATCH luajit 0/2] Fix assembling of the head of side traces Igor Munkin 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=9b2713c1405073d42c14f3179ff5a8fb338a37dd.1697034851.git.skaplun@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 1/2] Fix register mask for stack check in head of side trace.' \
    /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