[Tarantool-patches] [PATCH luajit] Optionally return PC position in jit.util.tracesnap().
Sergey Bronnikov
sergeyb at tarantool.org
Fri Oct 16 17:25:29 MSK 2026
Hi, Sergey,
thanks for review! See my comments below.
Sergey
On 10/13/26 16:41, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, after fixing my comments below.
>
> On 08.10.26, Sergey Bronnikov wrote:
>> From: Mike Pall <mike>
>>
>> Suggested by Sergey Bronnikov.
>>
>> (cherry picked from commit 8e6520a7aecd0517e792b359afbbfd7274791f5f)
>>
>> The optional `getpos` argument is needed for ljopt [1], a tool that
>> verifies the correctness of LuaJIT optimizations by translating the
>> recorded IR into SMT-LIB and checking the formulas with an SMT solver.
>> Traces often lack sufficient information for accurate modeling, so
>> ljopt extracts the address of the bytecode pointed to by the snapshot
>> directly from LuaJIT during execution.
>>
>> The LuaJIT internal API includes the function `jit.util.tracesnap(tr,
>> sn)`. In principle, this function could be extended to return the
>> program counter (pc). This patch does exactly that: an optional third
>> argument is added, and when it is truthy the second value returned by
>> `jit.util.tracesnap()` is the PC position of the snapshot in the
>> bytecode of the function where the trace starts.
>>
>> Also, the patch adds a test that records a root loop trace and
>> checks that the returned position is the `FORL` jump target (the
>> loop entry), that no position is returned without the optional
>> argument or with a false value, and that every snapshot of the
>> trace reports a valid bytecode offset.
> I don't understand this paragraph. Usually we don't describe added tests
> for the features (especially in the way "what was done" instead of "why
> it was done").
removed third paragraph and rephrased the second:
The LuaJIT internal API includes the function
`jit.util.tracesnap(tr, sn)`. The patch adds an optional third
argument, and when it is truthy the second value returned by
`jit.util.tracesnap()` is the PC position of the snapshot in the
bytecode of the function where the trace starts.
>> 1. https://github.com/ligurio/ljopt
>>
> Please, mention the backporting issues as well.
Added
>
>> Sergey Bronnikov:
>> * added the description and the test for the feature
>> ---
>>
>> Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-tracesnap-getpos
>>
>> src/lib_jit.c | 9 +-
>> .../lj-1472-tracesnap-getpos.test.lua | 89 +++++++++++++++++++
>> 2 files changed, 97 insertions(+), 1 deletion(-)
>> create mode 100644 test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua
>>
>> diff --git a/src/lib_jit.c b/src/lib_jit.c
>> index 9f870f68f..5a0d38611 100644
>> --- a/src/lib_jit.c
>> +++ b/src/lib_jit.c
> <snipped>
>
>> diff --git a/test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua b/test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua
>> new file mode 100644
>> index 000000000..a56a7a679
>> --- /dev/null
>> +++ b/test/tarantool-tests/lj-1472-tracesnap-getpos.test.lua
>> @@ -0,0 +1,89 @@
>> +local tap = require('tap')
>> +local bit = require('bit')
>> +local jutil = require('jit.util')
>> +local vmdef = require('jit.vmdef')
>> +
>> +-- The test checks the optional `getpos` argument of
>> +-- `jit.util.tracesnap()` that makes the function return the PC
>> +-- position of the snapshot in the function bytecode.
>> +-- See also https://github.com/LuaJIT/LuaJIT/issues/1472.
>> +
>> +local test = tap.test('lj-1472-tracesnap-getpos'):skipcond({
>> + ['Test requires JIT enabled'] = not jit.status(),
>> +})
>> +
>> +test:plan(8)
>> +
>> +-- Entries in `vmdef.bcnames` are 6 characters long.
>> +local BC_NAME_LENGTH = 6
>> +
>> +-- Return the name of the bytecode operation for the given
>> +-- instruction, that is encoded in the low byte.
>> +local function opname(ins)
>> + local oidx = BC_NAME_LENGTH * bit.band(ins, 0xff)
>> + local name = vmdef.bcnames:sub(oidx + 1, oidx + BC_NAME_LENGTH)
>> + return (name:gsub('%s+$', ''))
> Minor: I suggest the more readable:
> | return name:match('%a+')
> Feel free to ignore.
this will break testcase:
tarantool> a = "FORL "
---
...
tarantool> a:gsub('%a+', '')
---
- ' '
- 1
...
tarantool> a:gsub('%s+', '')
---
- FORL
- 1
...
>
>> +end
>> +
>> +local function payload()
>> + local sum = 0
>> + for i = 1, 100 do
> Minor: Typo: s/100/4/
Fixed:
end
local function payload()
local sum = 0
- for i = 1, 100 do
+ for i = 1, 4 do
sum = sum + i
end
return sum
>
>> + sum = sum + i
>> + end
>> + return sum
>> +end
>> +
>> +-- Find the loop back-edge (FORL) in the function bytecode and
>> +-- compute its jump target, that is the first instruction of the
> Typo: s/that/which/
Fixed:
-- Find the loop back-edge (FORL) in the function bytecode and
--- compute its jump target, that is the first instruction of the
+-- compute its jump target, which is the first instruction of the
-- loop body. `pc == 0` is the JCproto header, so real
-- instructions start from `pc == 1`. JIT compilation is disabled
-- while scanning, so this loop is not recorded as a trace.
>> +-- loop body. `pc == 0` is the JCproto header, so real
>> +-- instructions start from `pc == 1`. JIT compilation is disabled
>> +-- while scanning, so this loop is not recorded as a trace.
>> +jit.off()
>> +local nbc = jutil.funcinfo(payload).bytecodes
>> +local loop_entry
>> +for pc = 1, nbc - 1 do
>> + local ins = jutil.funcbc(payload, pc)
>> + if opname(ins) == 'FORL' then
>> + -- The jump offset is relative to the next instruction and is
>> + -- biased by `BCBIAS_J`.
>> + local jmp = bit.rshift(ins, 16) - 0x8000
> Let's name the magic number:
> | local BCBIAS_J = 0x8000
Fixed:
jit.off()
local nbc = jutil.funcinfo(payload).bytecodes
local loop_entry
+local BCBIAS_J = 0x8000
for pc = 1, nbc - 1 do
local ins = jutil.funcbc(payload, pc)
if opname(ins) == 'FORL' then
-- The jump offset is relative to the next instruction and is
-- biased by `BCBIAS_J`.
- local jmp = bit.rshift(ins, 16) - 0x8000
+ local jmp = bit.rshift(ins, 16) - BCBIAS_J
loop_entry = pc + 1 + jmp
break
end
>
>> + loop_entry = pc + 1 + jmp
>> + break
>> + end
>> +end
>> +test:isnt(loop_entry, nil, 'FORL instruction is found')
> It should be an assert instead, since this is the test precondition.
>
>> +
>> +-- Record a root loop trace in `payload`. Snapshot #0 points to
>> +-- the loop entry (the FORL jump target).
>> +jit.on()
>> +jit.flush()
>> +jit.opt.start('hotloop=1')
>> +payload()
>> +
>> +local info = jutil.traceinfo(1)
>> +test:ok(info, 'the root loop trace is recorded')
> It should be an assert instead, since this is the test precondition.
Updated:
loop_entry = pc + 1 + jmp
break
end
end
-test:isnt(loop_entry, nil, 'FORL instruction is found')
+assert(loop_entry, 'FORL instruction is found')
-- Record a root loop trace in `payload`. Snapshot #0 points to
-- the loop entry (the FORL jump target).
>
>> +local snap, pos = jutil.tracesnap(1, 0, true)
>> +test:istable(snap, 'the snapshot table is returned')
>> +test:is(pos, loop_entry, 'the PC position points to the loop entry')
>> +
>> +local _, pos_without = jutil.tracesnap(1, 0)
>> +test:is(pos_without, nil, 'no PC position without getpos')
>> +
>> +local _, pos_false = jutil.tracesnap(1, 0, false)
>> +test:is(pos_false, nil, 'no PC position with getpos=false')
>> +
>> +local all_valid = true
>> +for sn = 0, info.nexit - 1 do
>> + local _, p = jutil.tracesnap(1, sn, true)
>> + if type(p) ~= 'number' or p < 0 or p >= nbc or
>> + jutil.funcbc(payload, p) == nil then
>> + all_valid = false
>> + end
>> +end
>> +test:ok(all_valid, 'positions of all snapshots are valid offsets')
>> +
>> +test:is(select('#', jutil.tracesnap(1, info.nexit, true)), 0,
>> + 'out-of-range snapshot returns nothing')
>> +
> It would be nice to add any correctness test.
> The most trivial in mind is the following:
>
> | src/luajit -Ohotloop=1 -e '
> | local jutil = require"jit.util"
> | local function trivial()
> | local a = 0
> | return a
> | end
> | trivial()
> | trivial()
> |
> | local _, b = jutil.tracesnap(1, 0, true)
> | print(b)
> | local _, b = jutil.tracesnap(1, 1, true)
> | print(b)
> | '
> | 1
> | 2
>
> The snapshot is emitted at the function entry and before returning the
> result which makes it trivial to detect:
>
> | ---- TRACE 1 start (command line):3
> | 0001 KSHORT 0 0
> | 0002 RET1 0 2
> | ---- TRACE 1 IR
> | .... SNAP #0 [ ---- ]
> | .... SNAP #1 [ ---- +0 ]
>
> Don't forget to reset the hotcounters during the recording of this
> example.
ok, thanks, added:
})
-test:plan(8)
+test:plan(10)
-- Entries in `vmdef.bcnames` are 6 characters long.
local BC_NAME_LENGTH = 6
@@ -87,4 +87,21 @@ test:ok(all_valid, 'positions of all snapshots are
valid offsets')
test:is(select('#', jutil.tracesnap(1, info.nexit, true)), 0,
'out-of-range snapshot returns nothing')
+-- The same correctness check for a trace without a loop: the
+-- first and the last snapshots of the trivial function point to
+-- its first and second bytecode instructions, respectively.
+jit.flush()
+local function trivial()
+ local a = 0
+ return a
+end
+trivial()
+trivial()
+
+local _, pos_first = jutil.tracesnap(1, 0, true)
+test:is(pos_first, 1, 'the first snapshot points to the first instruction')
+
+local _, pos_last = jutil.tracesnap(1, 1, true)
+test:is(pos_last, 2, 'the second snapshot points to the second
instruction')
+
test:done(true)
>
>> +test:done(true)
>> --
>> 2.51.0
>>
More information about the Tarantool-patches
mailing list