bebbo / libnix

libnix (v4): a C link library for AmigaOS/m68k
15 stars 9 forks source link

__stkswap doesn't update tc_SPReg #48

Closed BSzili closed 2 years ago

BSzili commented 2 years ago

I missed one issue with the __stkswap function, which doesn't seem to update the tc_SPReg field of the task. This leads to some interesting problems where the task seems to use the old stack, crashing, etc. I created a test program to reproduce the crash, it can be built with m68k-amigaos-gcc -noixemul -o stacktest stacktest.c

#include <proto/exec.h>
#include <stdio.h>

extern void __stkinit(void);
void * __x = __stkinit;
unsigned long __stack = (32*1024);

int main(void)
{
    struct Task *task = FindTask(NULL);

    if ((ULONG)task->tc_SPUpper >= (ULONG)task->tc_SPReg && (ULONG)task->tc_SPReg >= (ULONG)task->tc_SPLower) 
        printf("free stack: %u\n", (ULONG)task->tc_SPReg - (ULONG)task->tc_SPLower);
    else
        printf("stack pointer is out of bounds\n");

    return 0;
}
BSzili commented 2 years ago

I tried changing this line: https://github.com/bebbo/libnix/blob/master/sources/misc/swapstack.c#L46 to task->tc_SPReg = a7 = upper; which fixed the stack indication in Scout, but the "out of bounds" message and crash remained, so there's probably something else in the stack swap code.

bebbo commented 2 years ago

that fix solved it for me: without that patch:

stack pointer is out of bounds: 0x649c 0 0xe49c

with that patch

free stack: 32700       0x64c8 0xe484 0xe4c8
bebbo commented 2 years ago

but IMHO the task scheduler should update the tc_SPReg field on each task switch...

BSzili commented 2 years ago

That's what I've thought as well, but Scout shown "---" for the task's stack, and it can only get the value of tc_SPReg after a task switch, so something in exec probably doesn't work as advertised. Anyway since that fixed for you I'll try again after a full rebuild.

bebbo commented 2 years ago

... I'll try again after a full rebuild.

no need for a full rebuild, Once it's live make libnix is sufficient.

BSzili commented 2 years ago

I did some more tests and it works under AmigaOS 3.1 but not under AmigaOS 3.9. I can reproduce the "out of bounds" crash under AmigaOS 3.1 too if I boot without a startup-sequence. Whether or not I start SetPatch before made no difference. Could you try running stacktest without startup-sequence to see if it still works?

bebbo commented 2 years ago

stacktest.zip It's working in

will test AmigaOs 3.9 too if I find the time to...

BSzili commented 2 years ago

Thanks for testing, I'm also using the [391773-01/391774-01] ROM. It turns out that it has nothing to do with the KS version. If I run it from a directory mounted as a volume in WinUAE it always triggers this bug without a s-s. I was able to reproduce the same issue running the program from a FAT-formatted CF card plugged into the A1200 PCMCIA slot. Interestingly if I copy it to the system volume or the system hardfile in case of UAE it works perfectly.

bebbo commented 2 years ago

Sorry, that's no longer a libnix issue. Even without startup-sequence, directory as hard drive in WinUAE, it's ok: grafik grafik