From a6a2720ddc22f9f62f119325881d05722c4f392e Mon Sep 17 00:00:00 2001

From: Thibault Charbonnier <thibaultcha@me.com>

Date: Tue, 19 Mar 2019 13:52:51 -0700

Subject: [PATCH 1/3] bugfix: fixed a segfault when unsinking 64-bit pointers.

The unsinking code was not using the correct layout for GC64 IR

constants (value in adjacent slot) for this case.

This patch is a derivative of

https://github.com/raptorjit/raptorjit/pull/246 ported for LuaJIT

itself.

Fixed after an intense debugging session with @lukego.

Co-authored-by: Luke Gorrie <lukego@gmail.com>

---

 src/lj_ir.h   | 12 ++++++------

 src/lj_snap.c |  2 +-

 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/lj_ir.h b/src/lj_ir.h

index 8057a750..a46b561f 100644

--- a/src/lj_ir.h

+++ b/src/lj_ir.h

@@ -562,6 +562,11 @@ typedef union IRIns {

   TValue tv;		/* TValue constant (overlaps entire slot). */

 } IRIns;

+#define ir_isk64(ir) ((ir)->o == IR_KNUM || (ir)->o == IR_KINT64 || \

+                      (LJ_GC64 && \

+                       ((ir)->o == IR_KGC || \

+                       (ir)->o == IR_KPTR || (ir)->o == IR_KKPTR)))

+

 #define ir_kgc(ir)	check_exp((ir)->o == IR_KGC, gcref((ir)[LJ_GC64].gcr))

 #define ir_kstr(ir)	(gco2str(ir_kgc((ir))))

 #define ir_ktab(ir)	(gco2tab(ir_kgc((ir))))

@@ -569,12 +574,7 @@ typedef union IRIns {

 #define ir_kcdata(ir)	(gco2cd(ir_kgc((ir))))

 #define ir_knum(ir)	check_exp((ir)->o == IR_KNUM, &(ir)[1].tv)

 #define ir_kint64(ir)	check_exp((ir)->o == IR_KINT64, &(ir)[1].tv)

-#define ir_k64(ir) \

-  check_exp((ir)->o == IR_KNUM || (ir)->o == IR_KINT64 || \

-	    (LJ_GC64 && \

-	     ((ir)->o == IR_KGC || \

-	      (ir)->o == IR_KPTR || (ir)->o == IR_KKPTR)), \

-	    &(ir)[1].tv)

+#define ir_k64(ir)	check_exp(ir_isk64(ir), &(ir)[1].tv)

 #define ir_kptr(ir) \

   check_exp((ir)->o == IR_KPTR || (ir)->o == IR_KKPTR, \

     mref((ir)[LJ_GC64].ptr, void))

diff --git a/src/lj_snap.c b/src/lj_snap.c

index ceaf2ca5..75888d80 100644

--- a/src/lj_snap.c

+++ b/src/lj_snap.c

@@ -688,7 +688,7 @@ static void snap_restoredata(GCtrace *T, ExitState *ex,

   int32_t *src;

   uint64_t tmp;

   if (irref_isk(ref)) {

-    if (ir->o == IR_KNUM || ir->o == IR_KINT64) {

+    if (ir_isk64(ir)) {

       src = (int32_t *)&ir[1];

     } else if (sz == 8) {

       tmp = (uint64_t)(uint32_t)ir->i;

-- 

2.21.0

From f36cddf49b664d713bfa7c332673bdc66861d2ad Mon Sep 17 00:00:00 2001

From: Thibault Charbonnier <thibaultcha@me.com>

Date: Tue, 19 Mar 2019 13:49:18 -0700

Subject: [PATCH 2/3] tests: ffi: added a test case unsinking a 64-bit pointer

 from a constant.

This test case reproduces the issue observed at:

https://github.com/openresty/lua-resty-core/issues/232 and was

contributed by @lukego and myself.

Co-authored-by: Luke Gorrie <lukego@gmail.com>

---

 test/ffi/unsink_64_kptr.lua | 26 ++++++++++++++++++++++++++

 1 file changed, 26 insertions(+)

 create mode 100644 test/ffi/unsink_64_kptr.lua

diff --git a/test/ffi/unsink_64_kptr.lua b/test/ffi/unsink_64_kptr.lua

new file mode 100644

index 00000000..7fab0e89

--- /dev/null

+++ b/test/ffi/unsink_64_kptr.lua

@@ -0,0 +1,26 @@

+local ffi = require("ffi")

+

+local array = ffi.new("struct { int x; } [1]")

+

+-- This test forces the VM to unsink a pointer that was constructed

+-- from a constant. The IR will include a 'cnewi' instruction to

+-- allocate an FFI pointer object, the pointer value will be an IR

+-- constant, the allocation will be sunk, and the allocation will

+-- at some point be "unsunk" due to a reference in the snapshot for

+-- a taken exit.

+

+-- Note: JIT will recognize <array> as a "singleton" and allow its

+-- address to be inlined ("constified") instead of looking up the

+-- upvalue at runtime.

+

+local function fn(i)

+  local struct = array[0]   -- Load pointer that the JIT will constify.

+  if i == 1000 then end     -- Force trace exit when i==1000.

+  struct.x = 0              -- Ensure that 'struct' is live after exit.

+end

+

+-- Loop over the function to make it compile and take a trace exit

+-- during the final iteration.

+for i = 1, 1000 do

+  fn(i)

+end

-- 

2.21.0

From 7b2f874b8061f206b22c04aee336b15030213637 Mon Sep 17 00:00:00 2001

From: Siddhesh Poyarekar <siddhesh@sourceware.org>

Date: Tue, 14 May 2019 22:01:37 +0530

Subject: [PATCH 3/3] Make unsink_64_kptr usable in the testsuite

---

 test/lib/ffi/index                    | 1 +

 test/{ => lib}/ffi/unsink_64_kptr.lua | 6 ++++--

 2 files changed, 5 insertions(+), 2 deletions(-)

 rename test/{ => lib}/ffi/unsink_64_kptr.lua (93%)

diff --git a/test/lib/ffi/index b/test/lib/ffi/index

index 59e36dd8..7933c5a7 100644

--- a/test/lib/ffi/index

+++ b/test/lib/ffi/index

@@ -10,3 +10,4 @@ jit_struct.lua

 meta_tostring.lua

 redir.lua

 type_punning.lua

+unsink_64_kptr.lua

diff --git a/test/ffi/unsink_64_kptr.lua b/test/lib/ffi/unsink_64_kptr.lua

similarity index 93%

rename from test/ffi/unsink_64_kptr.lua

rename to test/lib/ffi/unsink_64_kptr.lua

index 7fab0e89..f285d9ff 100644

--- a/test/ffi/unsink_64_kptr.lua

+++ b/test/lib/ffi/unsink_64_kptr.lua

@@ -21,6 +21,8 @@ end

 -- Loop over the function to make it compile and take a trace exit

 -- during the final iteration.

-for i = 1, 1000 do

-  fn(i)

+do --- unsink 64-bit pointers

+  for i = 1, 1000 do

+    fn(i)

+  end

 end

-- 

2.21.0