Modifying luac to build and run as a ps-exe

[nicolasnoble]

Arguments as passed through pcsx-redux' debugging ability to hook into invalid memory reads, through the args.lua script, which needs to be loaded at the same time as running the luac.ps-exe binary.

What's left to do: building the binary, maybe through xmake, using the xmake-psx project, and some documentation on how to run the compiler through pcsx-redux.

[rixnobis]

The approach is right and the PCDRV plumbing is clean. Reading argv out of an unmapped-memory hook is a good trick. One blocker, then a list.

Blocker - stack buffer overflow in main().

const char * argv[64] = { 0 };
...
while (1) {
    int len = luaA_strlen(argsPtr);
    if (len == 0) break;
    argv[argc++] = argsPtr;      // no bound on argc
    argsPtr += len + 1;
}

65 strings in the buffer and this writes past argv. It is fully reachable: createArgsBuffer(...) takes varargs straight from the console, so createArgsBuffer(unpack(t)) with a 65-element table smashes the stack. Needs if (argc == 64) break; at minimum. A truncation the caller never hears about is its own bug, so say something.

The same loop has no bound on the scan. If args.lua is not loaded, nothing guarantees a NUL ever appears at 0x40000000 and it walks until it faults. A total-length cap costs one comparison.

args.lua reads one byte past the end.

if offset > ffi.sizeof(argsBuffer) then return 0xff end
return argsBuffer[offset]

Valid indices are 0 .. sizeof-1, so offset == sizeof passes the guard and indexes off the end. >=.

Also spaceNeeded = spaceNeeded + #a + 1 in the second loop is dead. The size is fixed and the buffer allocated by the time it runs.

psx-glue.s builds an argv that nothing reads. main is now int main() and takes its arguments from 0x40000000, so li $a0, 1 / la $a1, _mainargv and the whole _mainargv/_progname rodata block are unreachable. That is also why the two files disagree about the program name - "PSX.EXE" in the assembly against 'luac.ps-exe' in the Lua - and the assembly one can never win. Either delete it, or give main its (argc, argv) back and let the glue supply the fallback for the no-hook case.

Two standard-library names squatted on.

#define EOF -1
static int getc(int f) { ... }

getc is a reserved identifier, and in hosted C it is a macro. EOF is a standard macro too. This compiles because stdio.h is gone; it breaks the moment anything pulls one in. LUAC_EOF and luaA_getc, and parenthesise the -1.

writer() compares signed to unsigned. int r against size_t size: r != size promotes r, so a PCwrite return of -1 compares as a huge unsigned and happens to give the right answer. r < 0 || (size_t)r != size.

Dead branch. doargs no longer handles -, so a bare - now falls to usage(). That makes IS("-") in pmain unreachable.

The reformat. +446/-445, and git diff -w --ignore-blank-lines says +211/-209 - so about half of this is reindentation of a vendored upstream file. psxlua's value is being Lua 5.2.4 with a delta somebody can read; this doubles the delta for no behaviour. Split it, or drop it. I do not mind which, and I do mind that they are in one commit.

Happy to take the fixes myself if you would rather hand it over - you said there was building and documentation left anyway.

[rixnobis]

The approach is right and the PCDRV plumbing is clean. Reading argv out of an unmapped-memory hook is a good trick. One blocker, then a list.

Blocker - stack buffer overflow in main().

const char * argv[64] = { 0 };
...
while (1) {
    int len = luaA_strlen(argsPtr);
    if (len == 0) break;
    argv[argc++] = argsPtr;      // no bound on argc
    argsPtr += len + 1;
}

65 strings in the buffer and this writes past argv. It is fully reachable: createArgsBuffer(...) takes varargs straight from the console, so createArgsBuffer(unpack(t)) with a 65-element table smashes the stack. Needs if (argc == 64) break; at minimum. A truncation the caller never hears about is its own bug, so say something.

The same loop has no bound on the scan. If args.lua is not loaded, nothing guarantees a NUL ever appears at 0x40000000 and it walks until it faults. A total-length cap costs one comparison.

args.lua reads one byte past the end.

if offset > ffi.sizeof(argsBuffer) then return 0xff end
return argsBuffer[offset]

Valid indices are 0 .. sizeof-1, so offset == sizeof passes the guard and indexes off the end. >=.

Also spaceNeeded = spaceNeeded + #a + 1 in the second loop is dead. The size is fixed and the buffer allocated by the time it runs.

psx-glue.s builds an argv that nothing reads. main is now int main() and takes its arguments from 0x40000000, so li $a0, 1 / la $a1, _mainargv and the whole _mainargv/_progname rodata block are unreachable. That is also why the two files disagree about the program name - "PSX.EXE" in the assembly against 'luac.ps-exe' in the Lua - and the assembly one can never win. Either delete it, or give main its (argc, argv) back and let the glue supply the fallback for the no-hook case.

Two standard-library names squatted on.

#define EOF -1
static int getc(int f) { ... }

getc is a reserved identifier, and in hosted C it is a macro. EOF is a standard macro too. This compiles because stdio.h is gone; it breaks the moment anything pulls one in. LUAC_EOF and luaA_getc, and parenthesise the -1.

writer() compares signed to unsigned. int r against size_t size: r != size promotes r, so a PCwrite return of -1 compares as a huge unsigned and happens to give the right answer. r < 0 || (size_t)r != size.

Dead branch. doargs no longer handles -, so a bare - now falls to usage(). That makes IS("-") in pmain unreachable.

The reformat. +446/-445, and git diff -w --ignore-blank-lines says +211/-209 - so about half of this is reindentation of a vendored upstream file. psxlua's value is being Lua 5.2.4 with a delta somebody can read; this doubles the delta for no behaviour. Split it, or drop it. I do not mind which, and I do mind that they are in one commit.

Happy to take the fixes myself if you would rather hand it over - you said there was building and documentation left anyway.

Duplicate - I fired gh pr review twice. The identical review below it is the one to read.

[nicolasnoble]

Regarding the argv trick: not having the args.lua loaded would crash immediately anyway. The 0x40000000 address is otherwise unmapped, and would trigger a violation on the first read, which is the whole point of this Lua hook.

Feel free to do the changes yourself, yes :) Maintainer edits have been enabled.

[rixnobis]

The scan point was wrong. 0x40000000 faults on the first read without args.lua, so the loop can't run past the end. Dropped it.

Pushed the rest:

• argv is bounded by MAXARGS; overflowing it is fatal("too many arguments").
• args.lua: >= on the size check, and the dead spaceNeeded line is out.
• getc/EOF are now luaA_getc/LUAC_EOF, -1 parenthesised.
• writer() is r < 0 || (size_t)r != size. That was a live -Wsign-compare hit; the file is clean under -Wall -Wextra now and wasn't before.
• IS("-") is out of pmain.
• psx-glue.s loses _mainargv/_progname and jumps to main with a nop in the delay slot.

Compile-checked with mipsel-none-elf-gcc against nugget's headers; luac.c and psx-glue.s both build, no new warnings.

I took the reformat split too, which means I rewrote the branch - git reset --hard yours before you touch it again. Three commits: the clang-format pass by itself, your port on top, then my fixes. Your commit is still yours, the tip tree is byte-identical to what you pushed, and luac.c inside your commit goes from +446/-445 to +125/-118.

Build wiring and docs are still yours. make psx only builds the library; there's no luac.ps-exe target yet.

Addressed on the branch. What's left is the build wiring, which isn't a review item.

[nicolasnoble]

Hah, are you making me do the remaining items instead of doing them yourself? :)

[rixnobis]

Your nugget note arrived after I'd added it as a submodule. Backed that out - NUGGET is a variable, empty by default, so psxlua picks up no new dependency:

make -C src psx-luac NUGGET=/path/to/nugget

Empty, it fails at that line with the variable named. make psx is unchanged and needs no NUGGET.

src/psx-heap.c is what the PR was missing. psxlua leaves luaI_realloc/luaI_free to its embedder - psyqo-lua --defsyms them onto psyqo_realloc/psyqo_free - and luac is standalone, so it takes the kernel heap from __bss_end to __sp - 64K. Blocks carry an 8-byte size header, since syscall_userMalloc has no realloc and luaI_realloc isn't given the old size. memcpy/memset come from nugget's common/crt0.

Links clean: 318 defined symbols, zero undefined, entry 0x80010000, PS-X EXE header. I have not run it. The only pcsx-redux binary on this box wants libavformat.so.58 and the box has .57, so that is unverified. Say the word and I'll build the emulator to close it.

And one I should have caught in the first pass: make host32 is broken by this PR. luac.c includes common/hardware/pcsxhw.h unconditionally, so the host luac no longer compiles:

luac.c:11:10: fatal error: common/hardware/pcsxhw.h: No such file or directory

psxlua's bytecode is not stock 5.2.4 bytecode - LUA_NUMBER_LONG changes it - so psxlua's own host luac is the only thing that cross-compiles scripts for the target. The ps-exe is the demonstration; the host binary is the tool. I'm putting the PSX I/O behind LUA_TARGET_PSX so both build. Say if you'd sooner drop luac from host32.

[rixnobis]

Then I had it backwards. Taken.

luac is out of host32: ALL_T is liblua.a and lua, and make host32 builds green again. psx-luac is a fourth platform in the README with the reason attached - compiling on the target makes the number representation, the pointer size and the endianness the target's by construction. The old host32 entry promised a compiler that "should be able to generate bytecode that can be run on the Sony PlayStation 1"; I deleted that claim.

The README also covers NUGGET, why it is not vendored, and that luac.ps-exe takes argv from 0x40000000 and does its I/O over PCDRV, so src/args.lua is how you hand it a command line.

make clean now removes luac.o, psx-heap.o, psx-glue.o, the elf, the map and the .dep files. A stale .dep naming a renamed source cost me a build earlier.

host32, psx and psx-luac all build clean from scratch here. Still unrun, same libavformat reason.

One discrepancy worth your eye: the README lists luaI_sprintf as one of four required imports, and luac links with zero undefined symbols without anyone supplying it. Either LUA_NUMBER_LONG removed the only caller or --gc-sections did. Left alone; your call.