[Tarantool-patches] [PATCH luajit 2/2] Add stack check to pcall/xpcall.
Sergey Bronnikov
estetus at gmail.com
Wed Aug 27 12:44:39 MSK 2025
Analyzed by Peter Cawley.
(cherry picked from commit a4c1640432a9d8a60624cdc8065b15078c228e36)
In the commit "LJ_FR2: Fix stack checks in vararg calls."
stack overflow in `pcall()`/`xpcall()` was fixed partially and
there are still cases where stack overflow happens, see comments
in the test. The patch add stack check to `pcall()` and `xpcall()`.
Sergey Bronnikov:
* added the description and the test for the problem
Part of tarantool/tarantool#11691
---
src/vm_arm.dasc | 7 +++++
src/vm_arm64.dasc | 8 +++++
src/vm_mips.dasc | 10 +++++-
src/vm_mips64.dasc | 12 +++++--
src/vm_ppc.dasc | 9 ++++++
src/vm_x64.dasc | 6 ++++
src/vm_x86.dasc | 6 ++++
...048-fix-stack-checks-vararg-calls.test.lua | 31 ++++++++++++++++++-
8 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
index 7095e660..efe9dcb2 100644
--- a/src/vm_arm.dasc
+++ b/src/vm_arm.dasc
@@ -1201,8 +1201,11 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc pcall
+ | ldr RB, L->maxstack
+ | add INS, BASE, NARGS8:RC
| ldrb RA, [DISPATCH, #DISPATCH_GL(hookmask)]
| cmp NARGS8:RC, #8
+ | cmphs RB, INS
| blo ->fff_fallback
| tst RA, #HOOK_ACTIVE // Remember active hook before pcall.
| mov RB, BASE
@@ -1213,7 +1216,11 @@ static void build_subroutines(BuildCtx *ctx)
| b ->vm_call_dispatch
|
|.ffunc_2 xpcall
+ | ldr RB, L->maxstack
+ | add INS, BASE, NARGS8:RC
| ldrb RA, [DISPATCH, #DISPATCH_GL(hookmask)]
+ | cmp RB, INS
+ | blo ->fff_fallback
| checkfunc CARG4, ->fff_fallback // Traceback must be a function.
| mov RB, BASE
| strd CARG12, [BASE, #8] // Swap function and traceback.
diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index cf8e575a..53ff7162 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
@@ -1166,6 +1166,10 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc pcall
+ | ldr TMP1, L->maxstack
+ | add TMP2, BASE, NARGS8:RC
+ | cmp TMP1, TMP2
+ | blo ->fff_fallback
| cmp NARGS8:RC, #8
| ldrb TMP0w, GL->hookmask
| blo ->fff_fallback
@@ -1185,6 +1189,10 @@ static void build_subroutines(BuildCtx *ctx)
| b ->vm_call_dispatch
|
|.ffunc xpcall
+ | ldr TMP1, L->maxstack
+ | add TMP2, BASE, NARGS8:RC
+ | cmp TMP1, TMP2
+ | blo ->fff_fallback
| ldp CARG1, CARG2, [BASE]
| ldrb TMP0w, GL->hookmask
| subs NARGS8:TMP1, NARGS8:RC, #16
diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc
index 32caabf7..69d09d52 100644
--- a/src/vm_mips.dasc
+++ b/src/vm_mips.dasc
@@ -1382,9 +1382,13 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc pcall
+ | lw TMP1, L->maxstack
+ | addu TMP2, BASE, NARGS8:RC
| lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH)
| beqz NARGS8:RC, ->fff_fallback
- | move TMP2, BASE
+ |. sltu AT, TMP1, TMP2
+ | bnez AT, ->fff_fallback
+ |. move TMP2, BASE
| addiu BASE, BASE, 8
| // Remember active hook before pcall.
| srl TMP3, TMP3, HOOK_ACTIVE_SHIFT
@@ -1394,8 +1398,12 @@ static void build_subroutines(BuildCtx *ctx)
|. addiu NARGS8:RC, NARGS8:RC, -8
|
|.ffunc xpcall
+ | lw TMP1, L->maxstack
+ | addu TMP2, BASE, NARGS8:RC
| sltiu AT, NARGS8:RC, 16
| lw CARG4, 8+HI(BASE)
+ | sltu TMP1, TMP1, TMP2
+ | or AT, AT, TMP1
| bnez AT, ->fff_fallback
|. lw CARG3, 8+LO(BASE)
| lw CARG1, LO(BASE)
diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc
index 7f49df5b..06b143a2 100644
--- a/src/vm_mips64.dasc
+++ b/src/vm_mips64.dasc
@@ -1418,8 +1418,12 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc pcall
+ | ld TMP1, L->maxstack
+ | daddu TMP2, BASE, NARGS8:RC
+ | sltu AT, TMP1, TMP2
+ | bnez AT, ->fff_fallback
+ |. lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH)
| daddiu NARGS8:RC, NARGS8:RC, -8
- | lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH)
| bltz NARGS8:RC, ->fff_fallback
|. move TMP2, BASE
| daddiu BASE, BASE, 16
@@ -1440,8 +1444,12 @@ static void build_subroutines(BuildCtx *ctx)
|. nop
|
|.ffunc xpcall
+ | ld TMP1, L->maxstack
+ | daddu TMP2, BASE, NARGS8:RC
+ | sltu AT, TMP1, TMP2
+ | bnez AT, ->fff_fallback
+ |. ld CARG1, 0(BASE)
| daddiu NARGS8:RC, NARGS8:RC, -16
- | ld CARG1, 0(BASE)
| ld CARG2, 8(BASE)
| bltz NARGS8:RC, ->fff_fallback
|. lbu TMP1, DISPATCH_GL(hookmask)(DISPATCH)
diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
index 980ad897..f2ea933b 100644
--- a/src/vm_ppc.dasc
+++ b/src/vm_ppc.dasc
@@ -1755,8 +1755,12 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc pcall
+ | lwz TMP1, L->maxstack
+ | add TMP2, BASE, NARGS8:RC
| cmplwi NARGS8:RC, 8
| lbz TMP3, DISPATCH_GL(hookmask)(DISPATCH)
+ | cmplw cr1, TMP1, TMP2
+ | cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt
| blt ->fff_fallback
| mr TMP2, BASE
| la BASE, 8(BASE)
@@ -1767,14 +1771,19 @@ static void build_subroutines(BuildCtx *ctx)
| b ->vm_call_dispatch
|
|.ffunc xpcall
+ | lwz TMP1, L->maxstack
+ | add TMP2, BASE, NARGS8:RC
| cmplwi NARGS8:RC, 16
| lwz CARG3, 8(BASE)
+ | cmplw cr1, TMP1, TMP2
|.if FPU
| lfd FARG2, 8(BASE)
+ | cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt
| lfd FARG1, 0(BASE)
|.else
| lwz CARG1, 0(BASE)
| lwz CARG2, 4(BASE)
+ | cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt
| lwz CARG4, 12(BASE)
|.endif
| blt ->fff_fallback
diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
index d5296759..141f5f82 100644
--- a/src/vm_x64.dasc
+++ b/src/vm_x64.dasc
@@ -1545,6 +1545,9 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc_1 pcall
+ | mov L:RB, SAVE_L
+ | lea RA, [BASE+NARGS:RD*8]
+ | cmp RA, L:RB->maxstack; ja ->fff_fallback
| lea RA, [BASE+16]
| sub NARGS:RDd, 1
| mov PCd, 16+FRAME_PCALL
@@ -1563,6 +1566,9 @@ static void build_subroutines(BuildCtx *ctx)
| jmp ->vm_call_dispatch
|
|.ffunc_2 xpcall
+ | mov L:RB, SAVE_L
+ | lea RA, [BASE+NARGS:RD*8]
+ | cmp RA, L:RB->maxstack; ja ->fff_fallback
| mov LFUNC:RA, [BASE+8]
| checktp_nc LFUNC:RA, LJ_TFUNC, ->fff_fallback
| mov LFUNC:RB, [BASE] // Swap function and traceback.
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index b043b830..1ba5abce 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -1914,6 +1914,9 @@ static void build_subroutines(BuildCtx *ctx)
|//-- Base library: catch errors ----------------------------------------
|
|.ffunc_1 pcall
+ | mov L:RB, SAVE_L
+ | lea RA, [BASE+NARGS:RD*8]
+ | cmp RA, L:RB->maxstack; ja ->fff_fallback
| lea RA, [BASE+8]
| sub NARGS:RD, 1
| mov PC, 8+FRAME_PCALL
@@ -1925,6 +1928,9 @@ static void build_subroutines(BuildCtx *ctx)
| jmp ->vm_call_dispatch
|
|.ffunc_2 xpcall
+ | mov L:RB, SAVE_L
+ | lea RA, [BASE+NARGS:RD*8]
+ | cmp RA, L:RB->maxstack; ja ->fff_fallback
| cmp dword [BASE+12], LJ_TFUNC; jne ->fff_fallback
| mov RB, [BASE+4] // Swap function and traceback.
| mov [BASE+12], RB
diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
index e300d5c1..367aecb6 100644
--- a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
+++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
@@ -7,7 +7,7 @@ local test = tap.test('lj-1048-fix-stack-checks-vararg-calls'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
})
-test:plan(2)
+test:plan(4)
-- The first testcase demonstrate a stack overflow in `pcall()`
-- by recursive calling `pcall()`. The functions are vararg
@@ -53,4 +53,33 @@ pcall(coroutine.wrap(looper_2), 0)
test:ok(true, 'no stack overflow with using metamethod')
+-- The third testcase demonstrate a stack overflow in
+-- `pcall()`/xpcall()` similar to the first testcase, but it is
+-- triggered using hand-crafted Lua chunk with a lot `pcall()`
+-- builtins.
+
+for i = 1, 100 do
+ local code = 'return pcall(' .. string.rep('pcall, ', i) .. 'pairs, {})'
+ local f = load(code)
+ coroutine.wrap(f)()
+end
+
+test:ok(true, 'no stack overflow with pcalls in load()')
+
+-- The fourth testcase demonstrate a stack overflow in
+-- `pcall()`/`xpcall()` similar to the first testcase, but it is
+-- triggered using `unpack()`.
+
+local t = {}
+local function f()
+ return pcall(unpack(t))
+end
+
+for i = 1, 100 do
+ t[i], t[i + 1], t[i + 2] = pcall, pairs, {}
+ coroutine.wrap(f)()
+end
+
+test:ok(true, 'no stack overflow with unpacked pcalls')
+
test:done(true)
--
2.43.0
More information about the Tarantool-patches
mailing list