Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, imun@tarantool.org,
	skaplun@tarantool.org
Subject: [Tarantool-patches] [PATCH luajit 1/7] core: save current frame top to lj_obj
Date: Wed,  8 Sep 2021 20:50:44 +0300	[thread overview]
Message-ID: <3b4c18b62a916634690d741535b7e1db61177777.1631122521.git.m.kokryashkin@tarantool.org> (raw)
In-Reply-To: <cover.1631122521.git.m.kokryashkin@tarantool.org>

From: Mikhail Shishatskiy <m.shishatskiy@tarantool.org>

Since commit 111d377d524e54e02187148a1832683291d620b2
the VM has LFUNC and FFUNC states. The upcoming sampling
profiler uses these vmstates to determine if the guest
stack is valid or not. There is an inconsistent behavior
of the VM when the Lua stack is not valid, but the state
is set to LFUNC. This patch is just a gross hack with which
the profiler works fine. The problem is to be investigated
more deeply :(
---
 src/lj_obj.h    | 12 ++++++++++++
 src/vm_x64.dasc | 52 +++++++++++++++++++++++++++++++++++++++----------
 src/vm_x86.dasc | 52 +++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 96 insertions(+), 20 deletions(-)

diff --git a/src/lj_obj.h b/src/lj_obj.h
index d26e60be..b76c3155 100644
--- a/src/lj_obj.h
+++ b/src/lj_obj.h
@@ -514,6 +514,17 @@ typedef struct GCtab {
 #define setfreetop(t, n, v)	(setmref((n)->freetop, (v)))
 #endif
 
+/* -- Misc objects -------------------------------------------------------- */
+
+struct lj_sysprof_topframe {
+  uint8_t ffid;          /* FFUNC: fast function id. */
+  union {
+    uint64_t raw;        /* Raw value for context save/restore. */
+    TValue *interp_base; /* LFUNC: Base of the executed coroutine. */
+    lua_CFunction cf;    /* CFUNC: Address of the C function. */
+  } guesttop;
+};
+
 /* -- State objects ------------------------------------------------------- */
 
 /* VM states. */
@@ -674,6 +685,7 @@ typedef struct global_State {
   MRef jit_base;	/* Current JIT code L->base or NULL. */
   MRef ctype_state;	/* Pointer to C type state. */
   GCRef gcroot[GCROOT_MAX];  /* GC roots. */
+  struct lj_sysprof_topframe top_frame;  /* Top frame for sysprof */
 } global_State;
 
 #define mainthread(g)	(&gcref(g->mainthref)->th)
diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
index 974047d3..c4beb5e7 100644
--- a/src/vm_x64.dasc
+++ b/src/vm_x64.dasc
@@ -345,6 +345,35 @@
 |  mov dword [DISPATCH+DISPATCH_GL(vmstate)], ~LJ_VMST_..st
 |.endmacro
 |
+|// Stash interpreter's internal base and enter LFUNC VM state.
+|// PROFILER: Each time profiler sees LFUNC state, it will inspect [BASE-1]
+|// expecting to see a valid framelink there. So enter this state only when
+|// BASE is stable and slots are not moved on the stack.
+|.macro set_vmstate_lfunc
+|  set_vmstate INTERP // Guard for non-atomic VM context restoration
+|  mov dword [DISPATCH+DISPATCH_GL(top_frame.guesttop)], BASE
+|  set_vmstate LFUNC
+|.endmacro
+|
+|// Stash ID of the fast function about to be executed and enter FFUNC VM state.
+|// PROFILER: Each time profiler sees FFUNC state, it will write ffid
+|// to the profile stream.
+|.macro set_vmstate_ffunc
+|  set_vmstate INTERP // Guard for non-atomic VM context restoration
+|  mov XCHGd, dword [BASE-8]
+|  mov dword [DISPATCH+DISPATCH_GL(top_frame.ffid)], XCHGd
+|  set_vmstate FFUNC
+|.endmacro
+|
+|// Stash address of the C function about to be executed and enter CFUNC VM state.
+|// PROFILER: Each time profiler sees CFUNC state, it will write this address
+|// to the profile stream.
+|.macro set_vmstate_cfunc
+|  set_vmstate INTERP // Guard for non-atomic VM context restoration
+|  mov dword [DISPATCH+DISPATCH_GL(top_frame.guesttop)], BASE
+|  set_vmstate CFUNC
+|.endmacro
+|
 |// Uses TMPRd (r10d).
 |.macro save_vmstate
 |.if not WIN
@@ -435,7 +464,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  jnz ->vm_returnp
   |
   |  // Return to C.
-  |  set_vmstate CFUNC
+  |  set_vmstate_cfunc
   |  and PC, -8
   |  sub PC, BASE
   |  neg PC				// Previous base = BASE - delta.
@@ -467,6 +496,9 @@ static void build_subroutines(BuildCtx *ctx)
   |  xor eax, eax			// Ok return status for vm_pcall.
   |
   |->vm_leave_unw:
+  |  set_vmstate INTERP // Guard for non-atomic VM context restoration
+  |  mov XCHGd, L:RBa->base
+  |  mov dword [DISPATCH+DISPATCH_GL(top_frame.guesttop)], XCHGd
   |  // DISPATCH required to set properly.
   |  restore_vmstate			// Caveat: uses TMPRd (r10d).
   |  restoreregs
@@ -725,7 +757,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  cleartp LFUNC:KBASE
   |  mov KBASE, LFUNC:KBASE->pc
   |  mov KBASE, [KBASE+PC2PROTO(k)]
-  |  set_vmstate LFUNC			// LFUNC after KBASE restoration.
+  |  set_vmstate_lfunc			// LFUNC after KBASE restoration.
   |  // BASE = base, RC = result, RB = meta base
   |  jmp RA				// Jump to continuation.
   |
@@ -1166,7 +1198,7 @@ static void build_subroutines(BuildCtx *ctx)
   |
   |.macro .ffunc, name
   |->ff_ .. name:
-  |  set_vmstate FFUNC
+  |  set_vmstate_ffunc
   |.endmacro
   |
   |.macro .ffunc_1, name
@@ -1748,7 +1780,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  movzx RAd, PC_RA
   |  neg RA
   |  lea BASE, [BASE+RA*8-16]		// base = base - (RA+2)*8
-  |  set_vmstate LFUNC			// LFUNC state after BASE restoration.
+  |  set_vmstate_lfunc			// LFUNC state after BASE restoration.
   |  ins_next
   |
   |6:  // Fill up results with nil.
@@ -2513,7 +2545,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  mov KBASE, [KBASE+PC2PROTO(k)]
   |  mov L:RB->base, BASE
   |  mov qword [DISPATCH+DISPATCH_GL(jit_base)], 0
-  |  set_vmstate LFUNC			// LFUNC after BASE & KBASE restoration.
+  |  set_vmstate_lfunc			// LFUNC after BASE & KBASE restoration.
   |  // Modified copy of ins_next which handles function header dispatch, too.
   |  mov RCd, [PC]
   |  movzx RAd, RCH
@@ -2730,7 +2762,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  call extern lj_ccallback_enter	// (CTState *cts, void *cf)
   |  // lua_State * returned in eax (RD).
   |  mov BASE, L:RD->base
-  |  set_vmstate LFUNC			// LFUNC after BASE restoration.
+  |  set_vmstate_lfunc			// LFUNC after BASE restoration.
   |  mov RD, L:RD->top
   |  sub RD, BASE
   |  mov LFUNC:RB, [BASE-16]
@@ -4299,7 +4331,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |  mov KBASE, LFUNC:KBASE->pc
     |  mov KBASE, [KBASE+PC2PROTO(k)]
     |  // LFUNC after the old BASE & KBASE is restored.
-    |  set_vmstate LFUNC
+    |  set_vmstate_lfunc
     |  ins_next
     |
     |6:  // Fill up results with nil.
@@ -4591,7 +4623,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |  ins_AD  // BASE = new base, RA = framesize, RD = nargs+1
     |  mov KBASE, [PC-4+PC2PROTO(k)]
     |  mov L:RB, SAVE_L
-    |  set_vmstate LFUNC		// LFUNC after KBASE restoration.
+    |  set_vmstate_lfunc		// LFUNC after KBASE restoration.
     |  lea RA, [BASE+RA*8]		// Top of frame.
     |  cmp RA, L:RB->maxstack
     |  ja ->vm_growstack_f
@@ -4629,7 +4661,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |  mov [RD-8], RB			// Store delta + FRAME_VARG.
     |  mov [RD-16], LFUNC:KBASE		// Store copy of LFUNC.
     |  mov L:RB, SAVE_L
-    |  set_vmstate LFUNC		// LFUNC after KBASE restoration.
+    |  set_vmstate_lfunc		// LFUNC after KBASE restoration.
     |  lea RA, [RD+RA*8]
     |  cmp RA, L:RB->maxstack
     |  ja ->vm_growstack_v		// Need to grow stack.
@@ -4685,7 +4717,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
       |  mov CARG1, L:RB		// Caveat: CARG1 may be RA.
     }
     |  ja ->vm_growstack_c		// Need to grow stack.
-    |  set_vmstate CFUNC		// CFUNC before entering C function.
+    |  set_vmstate_cfunc		// CFUNC before entering C function.
     if (op == BC_FUNCC) {
       |  call KBASE			// (lua_State *L)
     } else {
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index ab8e6f27..222754fe 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -443,6 +443,35 @@
 |  mov dword [DISPATCH+DISPATCH_GL(vmstate)], ~LJ_VMST_..st
 |.endmacro
 |
+|// Stash interpreter's internal base and enter LFUNC VM state.
+|// PROFILER: Each time profiler sees LFUNC state, it will inspect [BASE-1]
+|// expecting to see a valid framelink there. So enter this state only when
+|// BASE is stable and slots are not moved on the stack.
+|.macro set_vmstate_lfunc
+|  set_vmstate INTERP // Guard for non-atomic VM context restoration
+|  mov dword [DISPATCH+DISPATCH_GL(top_frame.guesttop)], BASE
+|  set_vmstate LFUNC
+|.endmacro
+|
+|// Stash ID of the fast function about to be executed and enter FFUNC VM state.
+|// PROFILER: Each time profiler sees FFUNC state, it will write ffid
+|// to the profile stream.
+|.macro set_vmstate_ffunc
+|  set_vmstate INTERP // Guard for non-atomic VM context restoration
+|  mov XCHGd, dword [BASE-8]
+|  mov dword [DISPATCH+DISPATCH_GL(top_frame.ffid)], XCHGd
+|  set_vmstate FFUNC
+|.endmacro
+|
+|// Stash address of the C function about to be executed and enter CFUNC VM state.
+|// PROFILER: Each time profiler sees CFUNC state, it will write this address
+|// to the profile stream.
+|.macro set_vmstate_cfunc
+|  set_vmstate INTERP // Guard for non-atomic VM context restoration
+|  mov dword [DISPATCH+DISPATCH_GL(top_frame.guesttop)], BASE
+|  set_vmstate CFUNC
+|.endmacro
+|
 |// Uses spilled ecx on x86 or XCHGd (r11d) on x64.
 |.macro save_vmstate
 |.if not WIN
@@ -560,7 +589,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  jnz ->vm_returnp
   |
   |  // Return to C.
-  |  set_vmstate CFUNC
+  |  set_vmstate_cfunc
   |  and PC, -8
   |  sub PC, BASE
   |  neg PC				// Previous base = BASE - delta.
@@ -599,6 +628,9 @@ static void build_subroutines(BuildCtx *ctx)
   |  xor eax, eax			// Ok return status for vm_pcall.
   |
   |->vm_leave_unw:
+  |  set_vmstate INTERP // Guard for non-atomic VM context restoration
+  |  mov XCHGd, L:RBa->base
+  |  mov dword [DISPATCH+DISPATCH_GL(top_frame.guesttop)], XCHGd
   |  // DISPATCH required to set properly.
   |  restore_vmstate			// Caveat: on x64 uses XCHGd (r11d).
   |  restoreregs
@@ -934,7 +966,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  mov KBASE, LFUNC:KBASE->pc
   |  mov KBASE, [KBASE+PC2PROTO(k)]
   |  // BASE = base, RC = result, RB = meta base
-  |  set_vmstate LFUNC			// LFUNC after KBASE restoration.
+  |  set_vmstate_lfunc			// LFUNC after KBASE restoration.
   |  jmp RAa				// Jump to continuation.
   |
   |.if FFI
@@ -1459,7 +1491,7 @@ static void build_subroutines(BuildCtx *ctx)
   |
   |.macro .ffunc, name
   |->ff_ .. name:
-  |  set_vmstate FFUNC
+  |  set_vmstate_ffunc
   |.endmacro
   |
   |.macro .ffunc_1, name
@@ -2141,7 +2173,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  movzx RA, PC_RA
   |  not RAa				// Note: ~RA = -(RA+1)
   |  lea BASE, [BASE+RA*8]		// base = base - (RA+1)*8
-  |  set_vmstate LFUNC			// LFUNC state after BASE restoration.
+  |  set_vmstate_lfunc			// LFUNC state after BASE restoration.
   |  ins_next
   |
   |6:  // Fill up results with nil.
@@ -2986,7 +3018,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  mov KBASE, [KBASE+PC2PROTO(k)]
   |  mov L:RB->base, BASE
   |  mov dword [DISPATCH+DISPATCH_GL(jit_base)], 0
-  |  set_vmstate LFUNC			// LFUNC after BASE & KBASE restoration.
+  |  set_vmstate_lfunc			// LFUNC after BASE & KBASE restoration.
   |  // Modified copy of ins_next which handles function header dispatch, too.
   |  mov RC, [PC]
   |  movzx RA, RCH
@@ -3257,7 +3289,7 @@ static void build_subroutines(BuildCtx *ctx)
   |  call extern lj_ccallback_enter@8	// (CTState *cts, void *cf)
   |  // lua_State * returned in eax (RD).
   |  mov BASE, L:RD->base
-  |  set_vmstate LFUNC			// LFUNC after BASE restoration.
+  |  set_vmstate_lfunc			// LFUNC after BASE restoration.
   |  mov RD, L:RD->top
   |  sub RD, BASE
   |  mov LFUNC:RB, [BASE-8]
@@ -5103,7 +5135,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |  mov KBASE, LFUNC:KBASE->pc
     |  mov KBASE, [KBASE+PC2PROTO(k)]
     |  // LFUNC after the old BASE & KBASE is restored.
-    |  set_vmstate LFUNC
+    |  set_vmstate_lfunc
     |  ins_next
     |
     |6:  // Fill up results with nil.
@@ -5391,7 +5423,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |  ins_AD  // BASE = new base, RA = framesize, RD = nargs+1
     |  mov KBASE, [PC-4+PC2PROTO(k)]
     |  mov L:RB, SAVE_L
-    |  set_vmstate LFUNC		// LFUNC after KBASE restoration.
+    |  set_vmstate_lfunc		// LFUNC after KBASE restoration.
     |  lea RA, [BASE+RA*8]		// Top of frame.
     |  cmp RA, L:RB->maxstack
     |  ja ->vm_growstack_f
@@ -5429,7 +5461,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
     |  mov [RD-4], RB			// Store delta + FRAME_VARG.
     |  mov [RD-8], LFUNC:KBASE		// Store copy of LFUNC.
     |  mov L:RB, SAVE_L
-    |  set_vmstate LFUNC		// LFUNC after KBASE restoration.
+    |  set_vmstate_lfunc		// LFUNC after KBASE restoration.
     |  lea RA, [RD+RA*8]
     |  cmp RA, L:RB->maxstack
     |  ja ->vm_growstack_v		// Need to grow stack.
@@ -5494,7 +5526,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop)
       |.endif
     }
     |  ja ->vm_growstack_c		// Need to grow stack.
-    |  set_vmstate CFUNC		// CFUNC before entering C function.
+    |  set_vmstate_cfunc		// CFUNC before entering C function.
     if (op == BC_FUNCC) {
       |  call KBASEa			// (lua_State *L)
     } else {
-- 
2.33.0


  reply	other threads:[~2021-09-08 17:51 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 17:50 [Tarantool-patches] [PATCH luajit 0/7] misc: introduce sysprof Maxim Kokryashkin via Tarantool-patches
2021-09-08 17:50 ` Maxim Kokryashkin via Tarantool-patches [this message]
2021-09-20 17:21   ` [Tarantool-patches] [PATCH luajit 1/7] core: save current frame top to lj_obj Sergey Kaplun via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 2/7] core: separate the profiling timer from lj_profile Maxim Kokryashkin via Tarantool-patches
2021-09-21 11:13   ` Sergey Kaplun via Tarantool-patches
2021-09-23 11:37     ` Mikhail Shishatskiy via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 3/7] memprof: move symtab to a separate module Maxim Kokryashkin via Tarantool-patches
2021-09-22  7:51   ` Sergey Kaplun via Tarantool-patches
2021-09-22  8:14     ` Sergey Kaplun via Tarantool-patches
2021-09-23 14:51   ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 4/7] core: introduce lua and platform profiler Maxim Kokryashkin via Tarantool-patches
2021-09-29  6:53   ` Sergey Kaplun via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 5/7] memprof: add profile common section Maxim Kokryashkin via Tarantool-patches
2021-10-05 10:48   ` Sergey Kaplun via Tarantool-patches
2021-10-06 19:15     ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 6/7] sysprof: introduce Lua API Maxim Kokryashkin via Tarantool-patches
2021-10-05 15:36   ` Sergey Kaplun via Tarantool-patches
2021-09-08 17:50 ` [Tarantool-patches] [PATCH luajit 7/7] tools: introduce parsers for sysprof Maxim Kokryashkin via Tarantool-patches
2021-10-07 11:28   ` Sergey Kaplun via Tarantool-patches
2022-04-07 12:15 ` [Tarantool-patches] [PATCH luajit 0/7] misc: introduce sysprof Sergey Kaplun 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=3b4c18b62a916634690d741535b7e1db61177777.1631122521.git.m.kokryashkin@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 1/7] core: save current frame top to lj_obj' \
    /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