* [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