Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH 0/2] luajit: Backports fixes from vanilla repo
@ 2019-05-08 12:10 Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH 1/2] Fix overflow of snapshot map offset Cyrill Gorcunov
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Cyrill Gorcunov @ 2019-05-08 12:10 UTC (permalink / raw)
  To: tml; +Cc: Cyrill Gorcunov

This series includes two backports 380e4409a70725df85034f02c968b6ebd7a5e513
and 046129dbdda5261c1b17469a2895a113d14c070a. Both are important enough,
still we need an intensive testing for it.

The series is done in sake of

https://github.com/tarantool/tarantool/issues/4171

and in top of our tarantool-1.7 branch

 | commit 3de8f282ab6c50270e6fd0db2c9f7fc9f6277f9b (origin/tarantool-1.7, origin/HEAD, tarantool-1.7)
 | Merge: b8587fa b92abb5
 | Author: Konstantin Osipov <kostja.osipov@gmail.com>
 | Date:   Mon Feb 26 15:22:54 2018 +0300
 |
 |     Merge pull request #4 from igormunkin/tarantool-1.7
 |
 |     Fixed empty string creation in lj_str_new

Please review carefully.

Mike Pall (2):
  Fix overflow of snapshot map offset.
  Fix rechaining of pseudo-resurrected string keys.

 src/lj_jit.h      | 10 +++++-----
 src/lj_opt_loop.c |  8 ++++----
 src/lj_snap.c     |  6 +++---
 src/lj_tab.c      | 23 +++++++++++++++++++++++
 4 files changed, 35 insertions(+), 12 deletions(-)

-- 
2.20.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] [PATCH 1/2] Fix overflow of snapshot map offset.
  2019-05-08 12:10 [tarantool-patches] [PATCH 0/2] luajit: Backports fixes from vanilla repo Cyrill Gorcunov
@ 2019-05-08 12:10 ` Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH] Fix rechaining of pseudo-resurrected string keys Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH 2/2] " Cyrill Gorcunov
  2 siblings, 0 replies; 5+ messages in thread
From: Cyrill Gorcunov @ 2019-05-08 12:10 UTC (permalink / raw)
  To: tml

From: Mike Pall <mike>

Thanks to Yichun Zhang.

backport 380e4409a70725df85034f02c968b6ebd7a5e513
---
 src/lj_jit.h      | 10 +++++-----
 src/lj_opt_loop.c |  8 ++++----
 src/lj_snap.c     |  6 +++---
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/lj_jit.h b/src/lj_jit.h
index 92054e3..7eb3d2a 100644
--- a/src/lj_jit.h
+++ b/src/lj_jit.h
@@ -160,7 +160,7 @@ typedef uint32_t MCode;
 
 /* Stack snapshot header. */
 typedef struct SnapShot {
-  uint16_t mapofs;	/* Offset into snapshot map. */
+  uint32_t mapofs;	/* Offset into snapshot map. */
   IRRef1 ref;		/* First IR ref for this snapshot. */
   uint8_t nslots;	/* Number of valid slots. */
   uint8_t topslot;	/* Maximum frame extent. */
@@ -227,8 +227,7 @@ typedef enum {
 /* Trace object. */
 typedef struct GCtrace {
   GCHeader;
-  uint8_t topslot;	/* Top stack slot already checked to be allocated. */
-  uint8_t linktype;	/* Type of link. */
+  uint16_t nsnap;	/* Number of snapshots. */
   IRRef nins;		/* Next IR instruction. Biased with REF_BIAS. */
 #if LJ_GC64
   uint32_t unused_gc64;
@@ -236,8 +235,7 @@ typedef struct GCtrace {
   GCRef gclist;
   IRIns *ir;		/* IR instructions/constants. Biased with REF_BIAS. */
   IRRef nk;		/* Lowest IR constant. Biased with REF_BIAS. */
-  uint16_t nsnap;	/* Number of snapshots. */
-  uint16_t nsnapmap;	/* Number of snapshot map elements. */
+  uint32_t nsnapmap;	/* Number of snapshot map elements. */
   SnapShot *snap;	/* Snapshot array. */
   SnapEntry *snapmap;	/* Snapshot map. */
   GCRef startpt;	/* Starting prototype. */
@@ -254,6 +252,8 @@ typedef struct GCtrace {
   TraceNo1 nextroot;	/* Next root trace for same prototype. */
   TraceNo1 nextside;	/* Next side trace of same root trace. */
   uint8_t sinktags;	/* Trace has SINK tags. */
+  uint8_t topslot;	/* Top stack slot already checked to be allocated. */
+  uint8_t linktype;	/* Type of link. */
   uint8_t unused1;
 #ifdef LUAJIT_USE_GDBJIT
   void *gdbjit_entry;	/* GDB JIT entry. */
diff --git a/src/lj_opt_loop.c b/src/lj_opt_loop.c
index 04c6d06..441b8ad 100644
--- a/src/lj_opt_loop.c
+++ b/src/lj_opt_loop.c
@@ -223,7 +223,7 @@ static void loop_subst_snap(jit_State *J, SnapShot *osnap,
   }
   J->guardemit.irt = 0;
   /* Setup new snapshot. */
-  snap->mapofs = (uint16_t)nmapofs;
+  snap->mapofs = (uint32_t)nmapofs;
   snap->ref = (IRRef1)J->cur.nins;
   snap->nslots = nslots;
   snap->topslot = osnap->topslot;
@@ -251,7 +251,7 @@ static void loop_subst_snap(jit_State *J, SnapShot *osnap,
   nmap += nn;
   while (omap < nextmap)  /* Copy PC + frame links. */
     *nmap++ = *omap++;
-  J->cur.nsnapmap = (uint16_t)(nmap - J->cur.snapmap);
+  J->cur.nsnapmap = (uint32_t)(nmap - J->cur.snapmap);
 }
 
 typedef struct LoopState {
@@ -369,7 +369,7 @@ static void loop_unroll(LoopState *lps)
     }
   }
   if (!irt_isguard(J->guardemit))  /* Drop redundant snapshot. */
-    J->cur.nsnapmap = (uint16_t)J->cur.snap[--J->cur.nsnap].mapofs;
+    J->cur.nsnapmap = (uint32_t)J->cur.snap[--J->cur.nsnap].mapofs;
   lua_assert(J->cur.nsnapmap <= J->sizesnapmap);
   *psentinel = J->cur.snapmap[J->cur.snap[0].nent];  /* Restore PC. */
 
@@ -383,7 +383,7 @@ static void loop_undo(jit_State *J, IRRef ins, SnapNo nsnap, MSize nsnapmap)
   SnapShot *snap = &J->cur.snap[nsnap-1];
   SnapEntry *map = J->cur.snapmap;
   map[snap->mapofs + snap->nent] = map[J->cur.snap[0].nent];  /* Restore PC. */
-  J->cur.nsnapmap = (uint16_t)nsnapmap;
+  J->cur.nsnapmap = (uint32_t)nsnapmap;
   J->cur.nsnap = nsnap;
   J->guardemit.irt = 0;
   lj_ir_rollback(J, ins);
diff --git a/src/lj_snap.c b/src/lj_snap.c
index bb063c2..3ff8dd4 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -161,11 +161,11 @@ static void snapshot_stack(jit_State *J, SnapShot *snap, MSize nsnapmap)
   nent = snapshot_slots(J, p, nslots);
   snap->nent = (uint8_t)nent;
   nent += snapshot_framelinks(J, p + nent, &snap->topslot);
-  snap->mapofs = (uint16_t)nsnapmap;
+  snap->mapofs = (uint32_t)nsnapmap;
   snap->ref = (IRRef1)J->cur.nins;
   snap->nslots = (uint8_t)nslots;
   snap->count = 0;
-  J->cur.nsnapmap = (uint16_t)(nsnapmap + nent);
+  J->cur.nsnapmap = (uint32_t)(nsnapmap + nent + 1 + J->framedepth);
 }
 
 /* Add or merge a snapshot. */
@@ -326,7 +326,7 @@ void lj_snap_shrink(jit_State *J)
   snap->nent = (uint8_t)m;
   nlim = J->cur.nsnapmap - snap->mapofs - 1;
   while (n <= nlim) map[m++] = map[n++];  /* Move PC + frame links down. */
-  J->cur.nsnapmap = (uint16_t)(snap->mapofs + m);  /* Free up space in map. */
+  J->cur.nsnapmap = (uint32_t)(snap->mapofs + m);  /* Free up space in map. */
 }
 
 /* -- Snapshot access ----------------------------------------------------- */
-- 
2.20.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] [PATCH] Fix rechaining of pseudo-resurrected string keys.
  2019-05-08 12:10 [tarantool-patches] [PATCH 0/2] luajit: Backports fixes from vanilla repo Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH 1/2] Fix overflow of snapshot map offset Cyrill Gorcunov
@ 2019-05-08 12:10 ` Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH 2/2] " Cyrill Gorcunov
  2 siblings, 0 replies; 5+ messages in thread
From: Cyrill Gorcunov @ 2019-05-08 12:10 UTC (permalink / raw)
  To: tml

From: Mike Pall <mike>

This is a serious bug. But extremely hard to reproduce, so it went
undetected for 8 years. One needs two resurrections with different
main nodes, which are both in a hash chain which gets relinked on
key insertion where the colliding node is in a non-main position. Phew.

Thanks to lbeiming.
---
 src/lj_tab.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/lj_tab.c b/src/lj_tab.c
index 50f447e..f2f3c0b 100644
--- a/src/lj_tab.c
+++ b/src/lj_tab.c
@@ -457,6 +457,29 @@ TValue *lj_tab_newkey(lua_State *L, GCtab *t, cTValue *key)
 	  freenode->next = nn->next;
 	  nn->next = n->next;
 	  setmref(n->next, nn);
+	  /*
+	  ** Rechaining a resurrected string key creates a new dilemma:
+	  ** Another string key may have originally been resurrected via
+	  ** _any_ of the previous nodes as a chain anchor. Including
+	  ** a node that had to be moved, which makes them unreachable.
+	  ** It's not feasible to check for all previous nodes, so rechain
+	  ** any string key that's currently in a non-main positions.
+	  */
+	  while ((nn = nextnode(freenode))) {
+	    if (tvisstr(&nn->key) && !tvisnil(&nn->val)) {
+	      Node *mn = hashstr(t, strV(&nn->key));
+	      if (mn != freenode) {
+		freenode->next = nn->next;
+		nn->next = mn->next;
+		setmref(mn->next, nn);
+	      } else {
+		freenode = nn;
+	      }
+	    } else {
+	      freenode = nn;
+	    }
+	  }
+	  break;
 	} else {
 	  freenode = nn;
 	}
-- 
2.20.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] [PATCH 2/2] Fix rechaining of pseudo-resurrected string keys.
  2019-05-08 12:10 [tarantool-patches] [PATCH 0/2] luajit: Backports fixes from vanilla repo Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH 1/2] Fix overflow of snapshot map offset Cyrill Gorcunov
  2019-05-08 12:10 ` [tarantool-patches] [PATCH] Fix rechaining of pseudo-resurrected string keys Cyrill Gorcunov
@ 2019-05-08 12:10 ` Cyrill Gorcunov
  2019-05-08 12:43   ` [tarantool-patches] " Cyrill Gorcunov
  2 siblings, 1 reply; 5+ messages in thread
From: Cyrill Gorcunov @ 2019-05-08 12:10 UTC (permalink / raw)
  To: tml

From: Mike Pall <mike>

This is a serious bug. But extremely hard to reproduce, so it went
undetected for 8 years. One needs two resurrections with different
main nodes, which are both in a hash chain which gets relinked on
key insertion where the colliding node is in a non-main position. Phew.

Thanks to lbeiming.

backport 046129dbdda5261c1b17469a2895a113d14c070a
---
 src/lj_tab.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/lj_tab.c b/src/lj_tab.c
index 47c0cfd..c51666d 100644
--- a/src/lj_tab.c
+++ b/src/lj_tab.c
@@ -491,6 +491,29 @@ TValue *lj_tab_newkey(lua_State *L, GCtab *t, cTValue *key)
 	  freenode->next = nn->next;
 	  nn->next = n->next;
 	  setmref(n->next, nn);
+	  /*
+	  ** Rechaining a resurrected string key creates a new dilemma:
+	  ** Another string key may have originally been resurrected via
+	  ** _any_ of the previous nodes as a chain anchor. Including
+	  ** a node that had to be moved, which makes them unreachable.
+	  ** It's not feasible to check for all previous nodes, so rechain
+	  ** any string key that's currently in a non-main positions.
+	  */
+	  while ((nn = nextnode(freenode))) {
+	    if (tvisstr(&nn->key) && !tvisnil(&nn->val)) {
+	      Node *mn = hashstr(t, strV(&nn->key));
+	      if (mn != freenode) {
+		freenode->next = nn->next;
+		nn->next = mn->next;
+		setmref(mn->next, nn);
+	      } else {
+		freenode = nn;
+	      }
+	    } else {
+	      freenode = nn;
+	    }
+	  }
+	  break;
 	} else {
 	  freenode = nn;
 	}
-- 
2.20.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH 2/2] Fix rechaining of pseudo-resurrected string keys.
  2019-05-08 12:10 ` [tarantool-patches] [PATCH 2/2] " Cyrill Gorcunov
@ 2019-05-08 12:43   ` Cyrill Gorcunov
  0 siblings, 0 replies; 5+ messages in thread
From: Cyrill Gorcunov @ 2019-05-08 12:43 UTC (permalink / raw)
  To: tml

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

On Wed, May 8, 2019 at 3:11 PM Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> From: Mike Pall <mike>
>
> This is a serious bug. But extremely hard to reproduce, so it went
> undetected for 8 years. One needs two resurrections with different
> main nodes, which are both in a hash chain which gets relinked on
> key insertion where the colliding node is in a non-main position. Phew.
>
> Thanks to lbeiming.
>
> backport 046129dbdda5261c1b17469a2895a113d14c070a
>

Please ignore this particular patch, it causes
https://github.com/LuaJIT/LuaJIT/issues/494

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-05-08 12:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08 12:10 [tarantool-patches] [PATCH 0/2] luajit: Backports fixes from vanilla repo Cyrill Gorcunov
2019-05-08 12:10 ` [tarantool-patches] [PATCH 1/2] Fix overflow of snapshot map offset Cyrill Gorcunov
2019-05-08 12:10 ` [tarantool-patches] [PATCH] Fix rechaining of pseudo-resurrected string keys Cyrill Gorcunov
2019-05-08 12:10 ` [tarantool-patches] [PATCH 2/2] " Cyrill Gorcunov
2019-05-08 12:43   ` [tarantool-patches] " Cyrill Gorcunov

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