srxfixup: detect and fix function symbols at .text offset 0#816
srxfixup: detect and fix function symbols at .text offset 0#816fjtrujy merged 2 commits intops2dev:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens IRX generation by detecting a dangerous layout case where function symbols can land at .text offset 0 (making zeroed export entries effectively jal 0x0), and updates module build/link ordering so export tables occupy .text+0 instead of executable code.
Changes:
- Add
--allow-zero-textand a.text+0function-symbol detection pass tosrxfixup. - Reorder many IOP module
IOP_OBJSlists soexports.ois linked first; move_retonlydefinitions after export table declarations in severalexports.tabfiles. - Update
iop/Rules.maketo automatically add--allow-zero-textfor modules that do not linkexports.o.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/srxfixup/src/srxfixup.c | Adds --allow-zero-text flag and a symbol-table scan to detect functions at .text offset 0. |
| tools/srxfixup/README.md | Documents the new zero-.text symbol check and the bypass flag. |
| iop/tcpip/tcpip/src/exports.tab | Moves _retonly definition after export table to avoid code at .text+0. |
| iop/tcpip/tcpip/Makefile | Links exports.o first to place export table at .text+0. |
| iop/tcpip/tcpip-netman/src/exports.tab | Moves _retonly definition after export table to avoid code at .text+0. |
| iop/system/timrman/Makefile | Links exports.o first to place export table at .text+0. |
| iop/system/stdio/Makefile | Links exports.o first to place export table at .text+0. |
| iop/system/siftoo/Makefile | Links exports.o first to place export table at .text+0. |
| iop/system/mtapman/Makefile | Links exports.o first to place export table at .text+0. |
| iop/system/intrman/Makefile | Links exports.o first to place export table at .text+0. |
| iop/system/dmacman/Makefile | Links exports.o first to place export table at .text+0. |
| iop/sound/libsd/Makefile | Links exports.o first to place export table at .text+0. |
| iop/network/udptty/src/exports.tab | Moves _retonly definition after export table to avoid code at .text+0. |
| iop/network/smap/src/exports.tab | Moves _retonly definition after export tables to avoid code at .text+0. |
| iop/network/smap/Makefile | Ensures exports.o is first (and makes IOP_OBJS override-friendly for variants). |
| iop/network/smap-ps2ip/Makefile | Defines an IOP_OBJS list that omits exports.o (no export table variant). |
| iop/network/smap-none/Makefile | Defines an IOP_OBJS list that omits exports.o (no export table variant). |
| iop/network/netman/src/exports.tab | Moves _retonly definition after export table to avoid code at .text+0. |
| iop/memorycard/mcserv/Makefile | Links exports.o first to place export table at .text+0. |
| iop/memorycard/mcman/Makefile | Links exports.o first to place export table at .text+0. |
| iop/fs/romdrv/src/exports.tab | Moves _retonly definition after export tables to avoid code at .text+0. |
| iop/fs/devfs/Makefile | Links exports.o first to place export table at .text+0. |
| iop/dev9/pvrdrv/Makefile | Links exports.o first to place export table at .text+0. |
| iop/debug/sior/Makefile | Links exports.o first to place export table at .text+0. |
| iop/debug/ioptrap/Makefile | Links exports.o first to place export table at .text+0. |
| iop/cdvd/cdvdstm/Makefile | Links exports.o first to place export table at .text+0. |
| iop/cdvd/cdvdman/Makefile | Links exports.o first to place export table at .text+0. |
| iop/cdvd/cdvdfsv/Makefile | Links exports.o first to place export table at .text+0. |
| iop/arcade/acdev9/Makefile | Links exports.o first to place export table at .text+0. |
| iop/arcade/acdev/Makefile | Links exports.o first to place export table at .text+0. |
| iop/arcade/accdvd/Makefile | Links exports.o first to place export table at .text+0. |
| iop/arcade/acatad/Makefile | Links exports.o first to place export table at .text+0. |
| iop/Rules.make | Automatically adds --allow-zero-text when exports.o is not part of IOP_OBJS. |
| common/sbus/Makefile | Links exports.o first to place export table at .text+0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
uyjulian
left a comment
There was a problem hiding this comment.
The srxfixup changes and the export table reorder LGTM
Add a check for STT_FUNC/STT_NOTYPE symbols at .text value 0, which can become unintended targets of jal 0x0 when called through an export table. New --allow-zero-text flag to bypass the check for modules that have no export table (where jal 0x0 is not a hazard). _start and _ftext are excluded (entry point / linker label, never dispatched via export table). Document the check and flag in README.md.
For modules with export tables, reorder IOP_OBJS so exports.o is linked first and move _retonly definitions after DECLARE_EXPORT_TABLE in exports.tab, ensuring the export struct (STT_OBJECT) occupies .text offset 0 instead of function code. For modules without export tables (smap-none, smap-ps2ip, and others), automatically pass --allow-zero-text to srxfixup via Rules.make when no exports.o is present in IOP_OBJS.
cdf0d5a to
1527c2b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem
When an IOP module's export table dispatches a function call, it uses
jal <address>. If a function symbol ends up at.textoffset 0, any zeroed or uninitialized export entry becomes an accidentaljal 0x0— silently calling real code instead of faulting. This is a latent bug that's hard to diagnose at runtime.Solution
1. Detection in
srxfixupAdded
check_zero_text_symbols()that scans theELFsymbol table forSTT_FUNCorSTT_NOTYPEsymbols at.textvalue 0. If found, srxfixup prints a descriptive error and exits with failure._startand_ftextare excluded since they are entry points / linker labels that are never dispatched via export tables.A new
--allow-zero-textflag bypasses the check for modules where it's safe (see below).2. Fixes for modules with export tables (Makefiles +
exports.tab)Reordered
IOP_OBJSsoexports.ois linked first. This places theirx_export_tablestruct (STT_OBJECT) at.textoffset 0 instead of function code.Moved
_retonlydefinitions afterDECLARE_EXPORT_TABLEinexports.tabfiles so the function body doesn't land before the table struct withinexports.o.Affected modules: sbus, acatad, accdvd, acdev, acdev9, cdvdfsv, cdvdman, cdvdstm, ioptrap, sior, pvrdrv, devfs, mcman, mcserv, smap, libsd, dmacman, intrman, mtapman, siftoo, stdio, timrman, tcpip, netman, udptty, romdrv, tcpip-netman.
3. Automatic
--allow-zero-textfor modules without export tablesRules.makeModules that have no
exports.oinIOP_OBJSdon't have export tables, sojal 0x0is not a hazard.Rules.makenow detects this with $(filter) and automatically passes--allow-zero-texttosrxfixup.This avoids the need to create fake/dummy export tables for modules like smap-none and smap-ps2ip that intentionally have no exports.