Fix moving dead slots in FFI finalizers table

Buristan · igormunkin · igormunkin · commit 30e7c977fd7b · 2025-04-11T23:24:08.000+03:00
LuaVela has the internal weak table to keep the mapping for GCcdata objects to their finalizers, which are set via <ffi.gc>. When GCcdata object becomes dead, there is still the key in this weak table until it's finalized by the Lua garbage collector (more precisely, until the next GCfinalize phase). When the table reallocation occurs, these keys are copied intact to the new hash part. LuaVela uses <copyTV> for such move, but there is just a plain pointer assignment in LuaJIT. As a result, the assertion within <tvchecklive> (to be more precisely within <isdead> check) hits. This patch replaces <copyTV> with the simple pointer move. Relates to tarantool/tarantool#5101 Co-authored-by: Igor Munkin <imun@cpan.org> Signed-off-by: Igor Munkin <imun@cpan.org>
diff --git a/src/lj_tab.c b/src/lj_tab.c
@@ -594,7 +594,8 @@ TValue* lj_tab_newkey(lua_State *L, GCtab *t, const TValue *key) {
       n = freenode;
     }
   }
-  copyTV(L, &n->key, key);
+  /* XXX: Do not use copyTV here: key may be dead (e.g. while rehashing). */
+  n->key = *key;
   if (LJ_UNLIKELY(tvismzero(&n->key))) { setrawV(&n->key, 0); }
   lj_gc_anybarriert(L, t);
   lua_assert(tvisnil(&n->val));
diff --git a/tests/impl/uJIT-tests-Lua/suite/chunks/ffi/gcfin_table_reallocation.lua b/tests/impl/uJIT-tests-Lua/suite/chunks/ffi/gcfin_table_reallocation.lua
@@ -0,0 +1,50 @@
+-- This is a part of uJIT's testing suite.
+-- Copyright (C) 2020-2025 LuaVela Authors. See Copyright Notice in COPYRIGHT
+-- Copyright (C) 2015-2020 IPONWEB Ltd. See Copyright Notice in COPYRIGHT
+
+local ffi = require('ffi')
+pcall(jit.off)
+
+ffi.cdef([[
+	struct old {int a;};
+	struct new {int a;};
+]])
+
+local anchor = 0
+local function create_fin(i)
+	return function()
+		anchor = anchor + i
+	end
+end
+
+-- Start GC and collect the existing garbage.
+collectgarbage('collect')
+-- Make GC collect as aggressive as possible.
+collectgarbage('setstepmul', 100)
+
+-- Create and mark GCcdata object as dead.
+-- As a result, ctype_state->finalizer->hmask is 1 (i.e. 2 fields):
+-- * __mode = "k";
+-- * finalizer for old GCcdata.
+local old_type = ffi.metatype('struct old', { __gc = create_fin(1) })
+local _ = old_type(1)
+_ = nil
+
+-- Reset the GC related counters.
+ujit.getmetrics()
+-- Finish all GCSpropagate steps and GCSatomic step.
+while ujit.getmetrics().gc_steps_sweepstring == 0 do
+	collectgarbage('step')
+end
+
+-- Freeze GC at GCSsweepstring phase.
+collectgarbage('stop')
+-- Create another GCcdata object.
+-- As a result, ctype_state->finalizer->hmask is 3 (i.e. 4 fields):
+-- * __mode = "k";
+-- * finalizer for the old (and dead) GCcdata;
+-- * finalizer for the new GCcdata.
+-- Hence, reallocation is required
+local new_type = ffi.metatype('struct new', { __gc = create_fin(1) })
+-- Reallocation occurs and the dead GCcdata is copied to the new hpart.
+_ = new_type(1)
diff --git a/tests/impl/uJIT-tests-Lua/suite/ffi.t b/tests/impl/uJIT-tests-Lua/suite/ffi.t
@@ -0,0 +1,51 @@
+#!/usr/bin/perl
+#
+# Tests for FFI machinery and related patches.
+# Copyright (C) 2020-2025 LuaVela Authors. See Copyright Notice in COPYRIGHT
+# Copyright (C) 2015-2020 IPONWEB Ltd. See Copyright Notice in COPYRIGHT
+
+use 5.010;
+use warnings;
+use strict;
+use lib './lib';
+
+use UJit::Test;
+
+sub _run_tests_group {
+    my ($extlib_func_name, $tester, @tests) = @_;
+
+    foreach my $test (@tests) {
+        my %asserts = %{ $test->{asserts} };
+
+        my $run_result = $tester->run($test->{file},
+                                      lua_args => $extlib_func_name);
+
+        while (my($check, $expr_list) = each %asserts) {
+            if (scalar @{ $expr_list}) {
+                $run_result->$check($_) for (@{ $expr_list });
+            } else {
+                $run_result->$check();
+            }
+        }
+    }
+}
+
+sub run_tests {
+    my ($tester, $tests) = @_;
+
+    while (my ($extlib_func_name, $tests_group) = each %{ $tests }) {
+        _run_tests_group($extlib_func_name, $tester, @{ $tests_group });
+    }
+}
+
+my $tester = UJit::Test->new(
+    chunks_dir => './chunks/ffi',
+);
+
+my @tarray = (
+    'gcfin_table_reallocation.lua',
+);
+
+$tester->run($_)->exit_ok() for (@tarray);
+
+exit;

Original file line number	Diff line number	Diff line change
`@@ -594,7 +594,8 @@ TValue* lj_tab_newkey(lua_State L, GCtab t, const TValue *key) {`
`594`	`594`	`n = freenode;`
`595`	`595`	`}`
`596`	`596`	`}`
`597`		`- copyTV(L, &n->key, key);`
	`597`	`+ /* XXX: Do not use copyTV here: key may be dead (e.g. while rehashing). */`
	`598`	`+ n->key = *key;`
`598`	`599`	`if (LJ_UNLIKELY(tvismzero(&n->key))) { setrawV(&n->key, 0); }`
`599`	`600`	`lj_gc_anybarriert(L, t);`
`600`	`601`	`lua_assert(tvisnil(&n->val));`