Bug in `amx->Callback`

[Y-Less]

Issue description:

https://github.com/pawn-lang/compiler/blob/0f2bd0b282e84e3d6af5f38581ff48738374b6ca/source/amx/amx.c#L499

Should be:

if (*(cell*)(code-sizeof(cell))!=OP_SYSREQ_PRI)

Currently it is checking that the parameter is not that op, instead of the op itself. There are two ways this manifests - 1) it breaks sysreq.pri because the following asserts fail when they shouldn't. 2) that will also fail a check if a native happens to have the same value as sysreq.pri. The second one isn't so bad, it just means that sysreq.c will never be replaced by sysreq.d.

[Y-Less]

Actually, that's not the problem - SYSREQ.pri has no parameter so the address is right, but something there is broken on Linux for SYSREQ.pri.

[Daniel-Cortez]

SYSREQ.pri has no parameter so the address is right

~sysreq.pri doesn't have any parameters, but CIP is also increased because of the opcode itself (see the _RCODE macro), so initially you were right, the address is not correct. If the code expects SYSREQ.c and SYSREQ.n to be at code-sizeof(cell)~ https://github.com/pawn-lang/compiler/blob/0f2bd0b282e84e3d6af5f38581ff48738374b6ca/source/amx/amx.c#L500 ~then the same should apply to SYSREQ.pri as well.~

[Y-Less]

I'm so good I'm right even when I think I'm not!

But more seriously, I think I just started overthinking it too much.

[Daniel-Cortez]

~Also,~

but something there is broken on Linux for SYSREQ.pri

~That "something" must be this:~ https://github.com/pawn-lang/compiler/blob/0f2bd0b282e84e3d6af5f38581ff48738374b6ca/source/amx/amx.c#L496-L498 ~This check doesn't take SYSREQ.pri in account at all, so, for example, for the following code~

#emit const.pri 17
#emit sysreq.pri
#emit stor.s.pri var

~the value of the cell right after sysreq.pri (stor.s.pri, which is 17) will be the same as the native ID, so sysreq.pri and stor.s.pri will get overwritten with sysreq.d and the native address.~

EDIT: Ah, nevermind, it was me who was on the wrong after all; the code pointer already has sizeof(cell) subtracted from amx->cip...

[Daniel-Cortez]

I think there might be another bug related to SYSREQ.pri though. https://github.com/pawn-lang/compiler/blob/0f2bd0b282e84e3d6af5f38581ff48738374b6ca/source/amx/amx.c#L494-L495 If the script is compiled with -O2 (and thus uses SYSREQ.n instead of SYSREQ.c), then one extra sizeof(cell) is subtracted from the code pointer, so if a native function is called via SYSREQ.pri, code points at the cell right before SYSREQ.pri, and if the value of that cell is the same as the native ID, then the two cells before SYSREQ.pri would be patched with SYSREQ.nd.

Example:

#pragma option -d0
#pragma option -O2 // Make the compiler use 'sysreq.n' instead of 'sysreq.c'

native heapspace();

SysReqBug()
{
    __emit push.c 0;

    // 'const.pri heapspace' would be overwritten with 'sysreq.nd <native address>'
    // so the next time this code is executed, 'sysreq.pri' would be treated
    // as the 2'nd argument of 'sysreq.nd' (122), thus causing stack corruption
    __emit const.pri heapspace;
    __emit sysreq.pri;

    __emit stack 4;
}

main()
{
    // Call a native function at least once, so the compiler
    // will set the 'AMX_FLAG_SYSREQN' flag in the *.amx file.
    heapspace();

    // Call 'SysReqBug()' for the first time, so the interpreter would overwrite
    // two instructions right before 'sysreq.pri'.
    SysReqBug();

    // Now that the code in 'SysReqBug()' is patched, calling this function again
    // would cause a runtime error.
    SysReqBug();
}

Running this code with the pawnruns utility from this repo makes make the program quit with the following message:

Run time error 7: "Stack underflow"

EDIT: This seems to occur only with the normal version of the interpreter core, not with the GCC/ICC-specific one. EDIT (2): Edited the example code, so this bug would cause a runtime error on both the GCC-specific version and the normal one.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity.