From a6a2720ddc22f9f62f119325881d05722c4f392e Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier 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 --- 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 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 --- 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 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 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