<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey</p>
<p>thanks for the fixes and answers!</p>
<p>See my answers below.<br>
</p>
<div class="moz-cite-prefix">On 13.06.2024 13:25, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<pre class="moz-quote-pre" wrap="">Hi, Sergey!
Thanks for the review!
Please consider my answers below.
On 06.06.24, Sergey Bronnikov wrote:
</pre>
</blockquote>
<snipped><br>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<pre class="moz-quote-pre" wrap="">
--- /dev/null
+++ b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
@@ -0,0 +1,46 @@
+local tap = require('tap')
+
+-- Test file to demonstrate unbalanced Lua stack after instruction
+-- recording due to throwing an error at recording of a stitched
+-- function.
+-- See also:<a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1166">https://github.com/LuaJIT/LuaJIT/issues/1166</a>.
+
+local test = tap.test('lj-1166-error-stitch-oom-snap-buff'):skipcond({
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
should a name in tap.test match to test file name?
now it is not.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
My mistake during copipasting. Fixed.
</pre>
</blockquote>
Thanks!<br>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ ['Test requires JIT enabled'] = not jit.status(),
+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
+
+test:plan(1)
+
+local mockalloc = require('mockalloc')
+
+local function create_chunk(n_slots)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I would add a comment like this:
--- a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
+++ b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
@@ -14,6 +14,18 @@ test:plan(1)
local mockalloc = require('mockalloc')
+-- Generate a Lua chunk like below:
+-- local s1
+-- local s2
+-- ...
+-- local sN
+-- for i = 1, 2 do
+-- s1 = i + 1
+-- s2 = i + 2
+-- ...
+-- sN = i + N
+-- math.modf(1)
+-- end
local function create_chunk(n_slots)
local chunk = ''
for i = 1, n_slots do
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Added, see the iterative patch below.</pre>
</blockquote>
<p>Thanks!</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ local chunk = ''
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+local mockalloc = require('mockalloc')
+
+local function create_chunk(n_conds)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">the same as above: please add a comment with an example of generated Lua
chunk
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Added, see the iterative patch below.
</pre>
</blockquote>
Thanks!<br>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<pre class="moz-quote-pre" wrap="">
diff --git a/test/tarantool-tests/lj-1166-error-stitch-table-bump.test.lua b/test/tarantool-tests/lj-1166-error-stitch-table-bump.test.lua
new file mode 100644
index 00000000..f2453bbe
--- /dev/null
+++ b/test/tarantool-tests/lj-1166-error-stitch-table-bump.test.lua
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
this test is not failed after reverting patch
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Do you build luajit with -DLUAJIT_ENABLE_TABLE_BUMP=ON?
<snipped>
</pre>
</blockquote>
<p>My bad. With enabled option tests failed:</p>
<p>The following tests FAILED:<br>
127 -
test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
(SEGFAULT)<br>
128 -
test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
(SEGFAULT)<br>
129 -
test/tarantool-tests/lj-1166-error-stitch-table-bump.test.lua
(SEGFAULT)<br>
Errors while running CTest<br>
</p>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">diff --git a/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c b/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c
new file mode 100644
index 00000000..d6d3492e
--- /dev/null
+++ b/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c
@@ -0,0 +1,51 @@
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+/* Function to be used instead of the default allocator. */
+static void *mock_allocf(void *ud, void *ptr, size_t osize, size_t nsize)
+{
+ assert(old_allocf != NULL);
+ /*
+ * Check the specific reallocation related to the IR
+ * buffer or the snapshot buffer.
+ */
+ if (osize * 2 == nsize)
+ return NULL;
+ return old_allocf(ud, ptr, osize, nsize);
+}
+
+static int mock(lua_State *L)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
It is actually not a test mock.
According to definition [1] test mock imitate a behavior of a real object.
Your memory allocator behaves as a real allocator, but in some cases it
will return
a NULL instead of memory address. What if we rename "mock" to "allocator
with fault injection"?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I renamed `mock_allocf` to `allocf_with_injection()` to avoid
confusing.
===================================================================
diff --git a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
index cf3ab0f5..eb229a5c 100644
--- a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
+++ b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
@@ -5,7 +5,7 @@ local tap = require('tap')
-- function.
-- See also: <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1166">https://github.com/LuaJIT/LuaJIT/issues/1166</a>.
-local test = tap.test('lj-1166-error-stitch-oom-snap-buff'):skipcond({
+local test = tap.test('lj-1166-error-stitch-oom-ir-buff'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
})
@@ -22,6 +22,16 @@ jit.flush()
test:plan(2)
+-- Generate the following Lua chunk:
+-- local s1
+-- ...
+-- local sN
+-- for i = 1, 2 do
+-- s1 = i + 1
+-- ...
+-- sN = i + N
+-- math.modf(1)
+-- end
local function create_chunk(n_slots)
local chunk = ''
for i = 1, n_slots do
diff --git a/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua b/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
index 8bbdd96b..8ae26386 100644
--- a/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
+++ b/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
@@ -20,6 +20,13 @@ jit.flush()
test:plan(2)
+-- Generate the following Lua chunk:
+-- for i = 1, 2 do
+-- if i < 1 then end
+-- ...
+-- if i < N then end
+-- math.modf(1)
+-- end
local function create_chunk(n_conds)
local chunk = ''
chunk = chunk .. 'for i = 1, 2 do\n'
diff --git a/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c b/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c
index d6d3492e..19d32f8b 100644
--- a/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c
+++ b/test/tarantool-tests/lj-1166-error-stitch/mockalloc.c
@@ -8,7 +8,8 @@ static lua_Alloc old_allocf = NULL;
static void *old_alloc_state = NULL;
/* Function to be used instead of the default allocator. */
-static void *mock_allocf(void *ud, void *ptr, size_t osize, size_t nsize)
+static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
+ size_t nsize)
{
assert(old_allocf != NULL);
/*
@@ -24,14 +25,14 @@ static int mock(lua_State *L)
{
assert(old_allocf == NULL);
old_allocf = lua_getallocf(L, &old_alloc_state);
- lua_setallocf(L, mock_allocf, old_alloc_state);
+ lua_setallocf(L, allocf_with_injection, old_alloc_state);
return 0;
}
static int unmock(lua_State *L)
{
assert(old_allocf != NULL);
- assert(old_allocf != mock_allocf);
+ assert(old_allocf != allocf_with_injection);
lua_setallocf(L, old_allocf, old_alloc_state);
old_allocf = NULL;
old_alloc_state = NULL;
===================================================================
</pre>
</blockquote>
<p>Filename and other variables still use prefix "mock":</p>
<p>[0] ~/sources/MRG/tarantool/third_party/luajit$ grep -R mock test<br>
test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua:local
mockalloc = require('mockalloc')<br>
test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua:mockalloc.mock()<br>
test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua:mockalloc.unmock()<br>
test/tarantool-tests/CMakeLists.txt:# Some tests use `LD_PRELOAD`
to mock system calls (like<br>
test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua:local
mockalloc = require('mockalloc')<br>
test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua:mockalloc.mock()<br>
test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua:mockalloc.unmock()<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c:static int
mock(lua_State *L)<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c:static int
unmock(lua_State *L)<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c:static const
struct luaL_Reg mockalloc[] = {<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c: {"mock",
mock},<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c: {"unmock",
unmock},<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c:LUA_API int
luaopen_mockalloc(lua_State *L)<br>
test/tarantool-tests/lj-1166-error-stitch/mockalloc.c:
luaL_register(L, "mockalloc", mockalloc);<br>
test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt:BuildTestCLib(mockalloc
mockalloc.c)</p>
<p>have you left it intentionally?<br>
</p>
<blockquote type="cite" cite="mid:ZmrI_4Ywx1wFNkpG@root">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
1. <a class="moz-txt-link-freetext" href="https://www.martinfowler.com/articles/mocksArentStubs.html">https://www.martinfowler.com/articles/mocksArentStubs.html</a>
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+{
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">diff --git a/test/tarantool-tests/lj-720-errors-before-stitch.test.lua b/test/tarantool-tests/lj-720-errors-before-stitch.test.lua
index d750b721..6e8f70c2 100644
--- a/test/tarantool-tests/lj-720-errors-before-stitch.test.lua
+++ b/test/tarantool-tests/lj-720-errors-before-stitch.test.lua
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>