Persona 5 [NPJB00769]

Interesting find. I wonder what the CPU disassembly around this instruction looks like; might explain the check failing.

I don't know enough to understand why that check is there, there is probably some kind of assumption but it doesn't seem right. The check is run right after creating a new thread, and look how many times it "fails", and yet the game runs... Maybe it is some kind of weird edge case or something, I might have a look later.

E {PPU[0x70000000] Thread (main_thread) [0x012a3704]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x20021da8, entry=0x12b939c, arg=0x20021080, prio=1000, stacksize=0x8000, flags=0x1, threadname=“gfdSpursSpursHdlr1”)
E {PPU[0x70000000] Thread (main_thread) [0x012a3704]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x20021da0, entry=0x12b92d4, arg=0x20021080, prio=1000, stacksize=0x4000, flags=0x1, threadname=“gfdSpursSpursHdlr0”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x20021060, entry=0xcf5ac8, arg=0x20021060, prio=1001, stacksize=0x19000, flags=0x0, threadname=“gfdJobQueueFiberWorker”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x200379d0, entry=0xcf5ac8, arg=0x200379d0, prio=1001, stacksize=0x19000, flags=0x0, threadname=“gfdJobQueueFiberWorker”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x200209d0, entry=0xcf5ac8, arg=0x200209d0, prio=1001, stacksize=0x19000, flags=0x0, threadname=“gfdJobQueueFiberWorker”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x1146c68, entry=0xcf8e40, arg=0x1146c58, prio=800, stacksize=0x2000, flags=0x1, threadname=“CRI FS File Access 0”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034b890 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x11632c0, entry=0xcf8e40, arg=0x11632b0, prio=800, stacksize=0x2000, flags=0x1, threadname=“CRI FS Memory File System”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034b890 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x114ad50, entry=0xcf8e40, arg=0x114ad40, prio=800, stacksize=0x2000, flags=0x1, threadname=“CRI FS File Access 1”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034b890 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x114ee38, entry=0xcf8e40, arg=0x114ee28, prio=800, stacksize=0x2000, flags=0x1, threadname=“CRI FS File Access 2”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034b890 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x1152f20, entry=0xcf8e40, arg=0x1152f10, prio=800, stacksize=0x2000, flags=0x1, threadname=“CRI FS File Access 3”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034b890 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x1142a28, entry=0xcf8e40, arg=0x1142a18, prio=800, stacksize=0x2000, flags=0x1, threadname=“CRI FS Data Decompression 0”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034b9c0 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x113e7a0, entry=0xcf8e40, arg=0x113e790, prio=800, stacksize=0x4000, flags=0x1, threadname=“CRI Server Manager”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034bac0 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x203f3010, entry=0xcf5ac8, arg=0x203f3010, prio=1002, stacksize=0xa000, flags=0x0, threadname=“FileSeqThread”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c490 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x203f3030, entry=0xcf5ac8, arg=0x203f3030, prio=1002, stacksize=0xa000, flags=0x0, threadname=“pakDevFileSyncThread”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c490 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x2034c600, entry=0xce4048, arg=0x0, prio=1003, stacksize=0x4000, flags=0x0, threadname=“game contents hdd install”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c580 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x200209b0, entry=0xcf5ac8, arg=0x200209b0, prio=1002, stacksize=0xa000, flags=0x0, threadname=“threadSeqSndAdx2”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c350 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x2034ccb0, entry=0xcf5ac8, arg=0x2034ccb0, prio=999, stacksize=0xa000, flags=0x0, threadname=“threadExecuteSndAdx2”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c350 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x20c7c320, entry=0xcf8e40, arg=0x20c7c310, prio=800, stacksize=0x2800, flags=0x1, threadname=“CriManaDecodeThread”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c110 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x234ff190, entry=0xcf5ac8, arg=0x234ff190, prio=1002, stacksize=0xa000, flags=0x1, threadname=“UNLOCK Thread”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c3c0 stack_addr=0xd0000000
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} sysPrxForUser: sys_ppu_thread_create(thread_id=*0x234ff190, entry=0xcf5ac8, arg=0x234ff190, prio=1002, stacksize=0xa000, flags=0x0, threadname=“addContSyncThread”)
E {PPU[0x70000000] Thread (main_thread) [0x00b49400]} PPU: Fail, context.gpr[1] < context.stack_addr, gpr[1] = 0x2034c340 stack_addr=0xd0000000

Looking at ppu_thread, it seems that check should be removed, or at most just throw a pessimistic warning. I dont think the PPU has a requirement that r1 cannot be set to another target location by the calling thread before performing a push. The faulting addresses also seem to indicate that subsequent threads are all writing to some sequential memory location using a push and that does not seem random to me. Our implementation assumes that the stack frame shall not be changed by the application, but clearly this still happens and I'm guessing the real hardware doesn't care as long as we dont fault on access. This check ought to be moved to the page fault handler IMO and a guard page inserted to properly detect stack overflow, but CPU guys are better suited for this kind of task than I am. Unfortunately I have too much on my plate at the moment, otherwise this could've been a fun challenge.

---

By the way, an easier way to check for stack overflow without complicating design would be to check if r1 straddles the stack boundary instead of merely doing a check like this. Simplified:

if (old_r1 >= stack_addr && new_r1 < stack_addr) then except;
else
do the push;

This IMO is alot easier to implement than the full implementation idea I mentioned above.