Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

box-tap/key_def.test.lua flaky fails #4252

Closed
Totktonada opened this issue May 27, 2019 · 6 comments
Closed

box-tap/key_def.test.lua flaky fails #4252

Totktonada opened this issue May 27, 2019 · 6 comments

Comments

@Totktonada
Copy link
Member

Totktonada commented May 27, 2019

builtin/fun.lua:958: attempt to call local 'gen_x' (a nil value)

https://travis-ci.org/tarantool/tarantool/jobs/535024408#L3750

I'm able to reproduce it with the following command:

TEST_RUN_TESTS="$(yes box-tap/key_def.test.lua | head -n 1000)" make test
@Totktonada Totktonada added qa Issues related to tests or testing subsystem flaky test lua labels May 27, 2019
@kyukhin kyukhin added this to the 1.10.4 milestone May 28, 2019
@Totktonada
Copy link
Member Author

It reproduces with good probability after the following patch:

diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
index 4b468a696..2c1e3afda 100755
--- a/test/box-tap/key_def.test.lua
+++ b/test/box-tap/key_def.test.lua
@@ -440,9 +440,13 @@ test:test('merge()', function(test)
     test:is_deeply(key_def_ab:extract_key(tuple_a):totable(), {1, 1, 22},
         'case 1: verify with :extract_key()')
 
-    local key_def_ba = key_def_b:merge(key_def_a)
-    local exp_parts = fun.iter(key_def_b:totable())
-        :chain(fun.iter(key_def_a:totable())):totable()
+    local key_def_ba
+    local exp_parts
+    for i = 1, 100000 do
+        key_def_ba = key_def_b:merge(key_def_a)
+        exp_parts = fun.iter(key_def_b:totable())
+            :chain(fun.iter(key_def_a:totable())):totable()
+    end
     test:is_deeply(key_def_ba:totable(), exp_parts,
         'case 2: verify with :totable()')
     test:is_deeply(key_def_ba:extract_key(tuple_a):totable(), {1, 22, 1},

But never reproduces when jit.off() is added somewhere at top of the file. So looks like a JIT problem.

@Totktonada
Copy link
Member Author

@sergos Agreed to look into this issue.

@printercu
Copy link

Here is reproducer:

local fun = require('fun')

local function concat(...)
    return fun.chain(...):totable()
end

local function x()
    for _ = 1, 10000 do
        concat({'a', 'b', 'c'}, {'q', 'w', 'e'})
    end
end

xpcall(function()
    for _ = 1, 100 do
        x()
    end
end, function(err)
    print(debug.traceback(tostring(err)))
    os.exit(1)
end)

Loops are not necessary. Sometimes it failed with single concat after function definition.

$ tarantool test.lua
builtin/fun.lua:958: attempt to call local 'gen_x' (a nil value)
stack traceback:
  test.lua:18: in function 'gen_x'
  builtin/fun.lua:958: in function 'gen_x'
  builtin/fun.lua:778: in function 'concat'
  test.lua:9: in function 'x'
  test.lua:15: in function <test.lua:13>
  [C]: in function 'xpcall'
  test.lua:13: in main chunk

Fails for me at
Tarantool 2.3.0-83-g165f8ee6f
Target: Darwin-x86_64-Debug

Tarantool 1.10.2-1-ge0017ad
Target: Darwin-x86_64-Release

@kyukhin kyukhin modified the milestones: 1.10.4, 1.10.5 Sep 26, 2019
@kyukhin kyukhin modified the milestones: 1.10.5, 1.10.6 Jan 13, 2020
@kyukhin kyukhin modified the milestones: 1.10.6, 1.10.7 Apr 20, 2020
@avtikhon
Copy link
Contributor

avtikhon commented Apr 22, 2020

The issue easy reproduced on dev1 host in debian-stretch image with
2.5.0-2-g098324556 version

with the following updated script from above:
a=0 ; while ../src/tarantool 4252.test.lua ; do a=$(($a+1)) ; echo -ne " $a"\\r ; done

4252.test:
NOTE: never fails when loops less than "8" below in the script

local fun = require('fun')

local function concat(...)
        return fun.chain(...):totable()
end

local function x()
        concat({'a', 'b', 'c'}, {'q', 'w', 'e'})
end

xpcall(function()
        for _ = 1, 8 do
                x()
        end
        end, function(err)
        print(debug.traceback(tostring(err)))
        os.exit(1)
end)

@kyukhin kyukhin added crash and removed qa Issues related to tests or testing subsystem labels Apr 23, 2020
@igormunkin igormunkin self-assigned this May 14, 2020
@igormunkin igormunkin removed the lua label May 22, 2020
igormunkin added a commit that referenced this issue Jun 19, 2020
JIT compiler can generate invalid trace for <totable> function breaking
its semantics (see LuaJIT/LuaJIT#584). Since interpreter works fine and
produces right results, disabling JIT for this function (and all its
subfunctions) stops execution failures.

Relates to LuaJIT/LuaJIT#584
Fixes #4252

Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Jun 23, 2020
JIT compiler can generate an invalid trace for <fun.chain> iterator
(i.e. chain_gen_r1) breaking its semantics (see LuaJIT/LuaJIT#584).
Since interpreter works fine and produces the right results, disabling
JIT for this function stops execution failures.

As a result box-tap/key_def.test.lua is removed from box-tap suite
fragile tests list.

Relates to LuaJIT/LuaJIT#584
Fixes #4252

Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Jun 23, 2020
JIT compiler can generate an invalid trace for <fun.chain> iterator
(i.e. chain_gen_r1) breaking its semantics (see LuaJIT/LuaJIT#584).
Since interpreter works fine and produces the right results, disabling
JIT for this function stops execution failures.

As a result box-tap/key_def.test.lua is removed from box-tap suite
fragile tests list.

Relates to LuaJIT/LuaJIT#584
Fixes #4252

Reviewed-by: Alexander V. Tikhonov <avtikhon@tarantool.org>
Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
kyukhin pushed a commit that referenced this issue Jun 26, 2020
JIT compiler can generate an invalid trace for <fun.chain> iterator
(i.e. chain_gen_r1) breaking its semantics (see LuaJIT/LuaJIT#584).
Since interpreter works fine and produces the right results, disabling
JIT for this function stops execution failures.

As a result box-tap/key_def.test.lua is removed from box-tap suite
fragile tests list.

Relates to LuaJIT/LuaJIT#584
Fixes #4252

Reviewed-by: Alexander V. Tikhonov <avtikhon@tarantool.org>
Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 5fa7ded)
kyukhin pushed a commit that referenced this issue Jun 26, 2020
JIT compiler can generate an invalid trace for <fun.chain> iterator
(i.e. chain_gen_r1) breaking its semantics (see LuaJIT/LuaJIT#584).
Since interpreter works fine and produces the right results, disabling
JIT for this function stops execution failures.

As a result box-tap/key_def.test.lua is removed from box-tap suite
fragile tests list.

Relates to LuaJIT/LuaJIT#584
Fixes #4252

Reviewed-by: Alexander V. Tikhonov <avtikhon@tarantool.org>
Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 5fa7ded)
@printercu
Copy link

Does it solve only flaky test? Do we need separate ticket for #4252 (comment) ?

igormunkin added a commit that referenced this issue Jul 24, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
igormunkin added a commit that referenced this issue Jul 24, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118

Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Jul 26, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin pushed a commit to tarantool/luajit that referenced this issue Aug 3, 2021
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Aug 3, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
igormunkin added a commit that referenced this issue Aug 3, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin pushed a commit to tarantool/luajit that referenced this issue Aug 4, 2021
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 3a2e484)
igormunkin pushed a commit to tarantool/luajit that referenced this issue Aug 4, 2021
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 3a2e484)
igormunkin pushed a commit to tarantool/luajit that referenced this issue Aug 4, 2021
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 3a2e484)
igormunkin pushed a commit to tarantool/luajit that referenced this issue Aug 4, 2021
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Aug 4, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 20c0a12)
igormunkin added a commit that referenced this issue Aug 4, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 20c0a12)
igormunkin added a commit that referenced this issue Aug 4, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 20c0a12)
igormunkin added a commit that referenced this issue Aug 4, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch adds changelog entry for commit
6a420c2 ('luajit: bump new version')
and commit 20c0a12 ('test: enable JIT
for Lua Fun chain iterator back').

Follows up #5118
Follows up #4252
Follows up #5049

Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 964aeb3)
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch adds changelog entry for commit
6a420c2 ('luajit: bump new version')
and commit 20c0a12 ('test: enable JIT
for Lua Fun chain iterator back').

Follows up #5118
Follows up #4252
Follows up #5049

Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch adds changelog entry for commit
6a420c2 ('luajit: bump new version')
and commit 20c0a12 ('test: enable JIT
for Lua Fun chain iterator back').

Follows up #5118
Follows up #4252
Follows up #5049

Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 964aeb3)
igormunkin added a commit that referenced this issue Aug 4, 2021
This patch adds changelog entry for commit
6a420c2 ('luajit: bump new version')
and commit 20c0a12 ('test: enable JIT
for Lua Fun chain iterator back').

Follows up #5118
Follows up #4252
Follows up #5049

Signed-off-by: Igor Munkin <imun@tarantool.org>
(cherry picked from commit 964aeb3)
yanshtunder pushed a commit that referenced this issue Oct 4, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
yanshtunder pushed a commit that referenced this issue Oct 4, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
yanshtunder pushed a commit that referenced this issue Oct 4, 2021
This patch adds changelog entry for commit
6a420c2 ('luajit: bump new version')
and commit 20c0a12 ('test: enable JIT
for Lua Fun chain iterator back').

Follows up #5118
Follows up #4252
Follows up #5049

Signed-off-by: Igor Munkin <imun@tarantool.org>
yanshtunder pushed a commit that referenced this issue Oct 4, 2021
* Detect inconsistent renames even in the presence of sunk values.

Closes #5118
Follows up #4252
yanshtunder pushed a commit that referenced this issue Oct 4, 2021
This patch reverts the temporary fix introduced in commit
5fa7ded ("test: disable JIT for Lua Fun
chain iterator") since the issues with invalid traces generation for
<fun.chain> iterator are resolved and JIT can be enabled back then.

Follows up #4252
Follows up #5118
Relates to #5049

Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
yanshtunder pushed a commit that referenced this issue Oct 4, 2021
This patch adds changelog entry for commit
6a420c2 ('luajit: bump new version')
and commit 20c0a12 ('test: enable JIT
for Lua Fun chain iterator back').

Follows up #5118
Follows up #4252
Follows up #5049

Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin pushed a commit to tarantool/luajit that referenced this issue Jun 16, 2022
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
igormunkin pushed a commit to tarantool/luajit that referenced this issue Jun 16, 2022
Reported by Igor Munkin.

(cherry picked from commit 33e3f4b)

Side exits with the same exitno use the same snapshot for restoring
guest stack values. This obliges all guards related to the particular
snapshot use the same RegSP mapping for the values to be restored at the
trace exit. RENAME emitted prior to the guard for the same snapshot
leads to the aforementioned invariant violation. The easy way to save
the snapshot consistency is spilling the renamed IR reference, that is
done in scope of <asm_snap_checkrename>.

However, the previous <asm_snap_checkrename> implementation considers
only the IR references explicitly mentioned in the snapshot. E.g. if
there is a sunk[1] object to be restored at the trace exit, and the
renamed reference is a *STORE to that object, the spill slot is not
allocated. As a result an invalid value is stored while unsinking that
object at all corresponding side exits prior to the emitted renaming.

To handle also those IR references implicitly used in the snapshot, all
non-constant and non-sunk references are added to the Bloom filter (it's
worth to mention that two hash functions are used to reduce collisions
for the cases when the number of IR references emitted between two
different snapshots exceeds the filter size). New <asm_snap_checkrename>
implementation tests whether the renamed IR reference is in the filter
and forces a spill slot for it as a result.

[1]: http://wiki.luajit.org/Allocation-Sinking-Optimization

Igor Munkin:
* added the description and the test for the problem

Resolves tarantool/tarantool#5118
Follows up tarantool/tarantool#4252

Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants