[Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again).

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit c64020f3c6d124503213147f2fb47c20335a395b)

This patch fixes JIT recording for the vararg calls, that was not fixed
in the previous patch. The `ctr` pointer to the child cdata value may
become invalid after the ctype state table reallocation in the
`crec_call_args()`.

This patch preserves `ctr->info` in the separate variable to avoid
dereferencing an invalid pointer.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11691
---
 src/lj_crecord.c                              | 11 +--
 ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++
 2 files changed, 88 insertions(+), 5 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua

diff --git a/src/lj_crecord.c b/src/lj_crecord.c
index 935106dc..b016eaec 100644
--- a/src/lj_crecord.c
+++ b/src/lj_crecord.c
@@ -1236,6 +1236,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
   if (ctype_isfunc(info)) {
     TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR);
     CType *ctr = ctype_rawchild(cts, ct);
+    CTInfo ctr_info = ctr->info;  /* crec_call_args may invalidate ctr. */
     IRType t = crec_ct2irt(cts, ctr);
     TRef tr;
     TValue tv;
@@ -1243,11 +1244,11 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
     tv.u64 = ((uintptr_t)cdata_getptr(cdataptr(cd), (LJ_64 && tp == IRT_P64) ? 8 : 4) >> 2) | U64x(800000000, 00000000);
     if (tvistrue(lj_tab_get(J->L, cts->miscmap, &tv)))
       lj_trace_err(J, LJ_TRERR_BLACKL);
-    if (ctype_isvoid(ctr->info)) {
+    if (ctype_isvoid(ctr_info)) {
       t = IRT_NIL;
       rd->nres = 0;
-    } else if (!(ctype_isnum(ctr->info) || ctype_isptr(ctr->info) ||
-		 ctype_isenum(ctr->info)) || t == IRT_CDATA) {
+    } else if (!(ctype_isnum(ctr_info) || ctype_isptr(ctr_info) ||
+		 ctype_isenum(ctr_info)) || t == IRT_CDATA) {
       lj_trace_err(J, LJ_TRERR_NYICALL);
     }
     if ((info & CTF_VARARG)
@@ -1258,7 +1259,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
       func = emitir(IRT(IR_CARG, IRT_NIL), func,
 		    lj_ir_kint(J, ctype_typeid(cts, ct)));
     tr = emitir(IRT(IR_CALLXS, t), crec_call_args(J, rd, cts, ct), func);
-    if (ctype_isbool(ctr->info)) {
+    if (ctype_isbool(ctr_info)) {
       if (frame_islua(J->L->base-1) && bc_b(frame_pc(J->L->base-1)[-1]) == 1) {
 	/* Don't check result if ignored. */
 	tr = TREF_NIL;
@@ -1274,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
 	tr = TREF_TRUE;
       }
     } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) ||
-	       t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) {
+	       t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr_info)) {
       TRef trid = lj_ir_kint(J, ctype_cid(info));
       tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr);
       if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J);
diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua
new file mode 100644
index 00000000..388bec6e
--- /dev/null
+++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua
@@ -0,0 +1,82 @@
+local tap = require('tap')
+local ffi = require('ffi')
+
+-- This test demonstrates LuaJIT's incorrect behaviour when the
+-- reallocation of `cts->tab` strikes during the recording of the
+-- setup arguments for the FFI call.
+-- Before the patch, the test failed only under ASAN.
+-- This file tests the case similar to the
+-- <lj-1360-dangling-ctype-ref-on-ccall.test.lua>, but for the
+-- compiler part only.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1360.
+local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({
+  ['Impossible to predict the value of cts->top'] = _TARANTOOL,
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- XXX: Declare the structure to increase `cts->top` up to 128
+-- slots. The setting up of function's arguments to the next call
+-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()`
+-- and `ccall_set_args()` will use a dangling reference.
+ffi.cdef[[
+  struct test {
+      int a;
+      int b;
+      int c;
+      int d;
+      int e;
+      int f;
+      int g;
+      int h;
+      int i;
+      int j;
+      int k;
+      int l;
+      int m;
+      int n;
+      int o;
+      int p;
+      int q;
+      int r;
+      int s;
+      int t;
+      int u;
+      int v;
+      int w;
+      int x;
+      int y;
+      int z;
+      int aa;
+      int ab;
+      int ac;
+      int ad;
+  };
+  // Use an existing function that actually takes no arguments.
+  // We can declare it however we want.
+  // Need a vararg function for this issue.
+  int getppid(...);
+]]
+
+local arg_call = ffi.new('struct test')
+
+-- Place `getppid` symbol in cache to permit recording.
+ffi.C.getppid(0)
+
+-- Assertions to check the `cts->top` value.
+assert(ffi.typeinfo(127), 'cts->top >= 127')
+assert(not ffi.typeinfo(128), 'cts->top < 128')
+
+jit.opt.start('hotloop=1')
+local counter = 0
+while counter < 4 do
+  -- Don't check the result, just check that there is no invalid
+  -- memory access.
+  ffi.C.getppid(arg_call)
+  counter = counter + 1
+end
+
+test:ok(true, 'no heap-use-after-free in C call')
+
+test:done(true)
-- 
2.50.1