Skip to content

PPC soft-float: fix ipairs() returning corrupted values and pairs() yielding a spurious nil - #270

Open
BKPepe wants to merge 2 commits into
openresty:v2.1-agentzhfrom
BKPepe:ppc-softfp-iter-fixes
Open

PPC soft-float: fix ipairs() returning corrupted values and pairs() yielding a spurious nil#270
BKPepe wants to merge 2 commits into
openresty:v2.1-agentzhfrom
BKPepe:ppc-softfp-iter-fixes

Conversation

@BKPepe

@BKPepe BKPepe commented Aug 15, 2026

Copy link
Copy Markdown

Two independent bugs in src/vm_ppc.dasc, both confined to the .if not FPU branch and both introduced by 2763a42 ("Patch for PPC64 support"). Upstream LuaJIT is not affected — on the same board, the distro's upstream LuaJIT returns the correct results.

Reproducers on Turris 1.x (e500v2, 32-bit big-endian, musl, soft-float):

local sum = 0
for _, v in ipairs({10, 20, 30}) do
    sum = sum + v
end
assert(sum == 60)
local expected = {7, 8, 9}
local i = 0
for k, v in pairs(expected) do
    i = i + 1
    assert(k == i and v == expected[i])
end
assert(i == 3)

Before the fixes, the ipairs() test yields -42 instead of 60 and the pairs() test sees an extra (nil, nil) iteration. With the fixes, both pass, with and without the JIT.

ipairs() also has a memory-safety angle: only the payload half of the TValue is wrong while the itype stays correct, so for GC-typed elements the type tag ends up in the GCref and tostring() dereferences a fabricated pointer.

Verified on hardware, and through the OpenWrt package build in openwrt/packages#30280, where the resulting powerpc_8548 package was installed on the device and behaves correctly.

BKPepe added 2 commits August 15, 2026 11:04
The soft-float branch of the ipairs_aux fast function loads the array
slot value from WORD_HI instead of WORD_LO. On big-endian targets,
WORD_HI contains the itype while the value word is in WORD_LO, so the
itype is loaded twice and the actual value is never loaded.

Every element therefore comes back carrying the itype in its payload.
Numbers surface as -14, the LJ_TNUMX tag read as an int32. For GC
types the payload is the GCref, so the tag becomes a fabricated
pointer that tostring() then dereferences.

The original soft-float code used a hardcoded 4(TMP1), which is
WORD_LO on big-endian. Commit 2763a42 ("Patch for PPC64 support")
rewrote it as WORD_HI. Upstream LuaJIT is unaffected.

Reproducer on Turris 1.x (e500v2, 32-bit big-endian, soft-float):

  $ luajit -e 'local s=0 for i,v in ipairs({10,20,30}) do s=s+v end print(s)'
  -42     -- expected 60

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
BC_ITERN checks the itype of the node value in RB to skip empty slots
in the hash part. Upstream loads RB unconditionally before the FPU
split; commit 2763a42 ("Patch for PPC64 support") moved that load
into the FPU branch only. On soft-float builds, RB still contains
RC*8 from the hash-part setup, so the nil check never succeeds and
iterating a table whose hash part is empty yields one extra
(nil, nil) pair.

Check CARG1 instead, which the soft-float branch already loads with
the itype. The FPU branch is left untouched. Upstream LuaJIT is
unaffected.

Reproducer on Turris 1.x (e500v2, 32-bit big-endian, soft-float):

  $ luajit -e 'for k,v in pairs({7,8,9}) do print(k,v) end'
  1    7
  2    8
  3    9
  nil  nil     -- spurious

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Copilot AI lite review requested due to automatic review settings August 15, 2026 09:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants