Stack size set on icon ignored on 3.2.2.1

[CCRider]

Hi there,

Apparently, on my system, the stack size set on a program's .info is being ignored on launch.

My specific test case: AmigaGPT warns if a stack size of at least 32768 isn't set. And despite such value being set on its icon, I get the warn every time I launch it.

Now, if running from the Shell and typing stack 32768 before running the program, such warning doesn't appear.

My system is an Amiga 1200 with TF1260, KS 47.111 and WB 47.4 with no performance patches whatsoever, except for MuFastRom, MuFastChip and MuFastZero.

[PeterK]

I've tried out Scout yesterday, a system information tool. Scout also has a stack setting of 32768 in its tool icon, and that stack size really appears in the task list of Scout, too. So, I think that it's not OS 3.2.2 which is responsible for your stack problem.

I still remember that MUI 5 may have some problems in some special cases to get enough stack for some subtasks. If your AmigaGPT program uses MUI 5 then try to install StackAttack with a configuration for a larger stack.

[CCRider]

AmigaGPT doesn't use MUI, instead it relies completely on AmigaOS's GUI tools.

Maybe it's a bug on it, after all. Weird...

[Thomas Richter]

Which type of an icon is that? A project icon or a tool icon?

[PeterK]

Sorry, it took a little longer to find that case in my old emails again where the stack problem occured.

In that case it was not MUI 5 directly, not the tool directly, but it was caused by the MagicASL patch, which was injecting MUI 5 with all the stack requirements when the user tried to open an ASL requester with his tool. Since my own tool MinStack didn't support this indirect stack demand, I removed it from Aminet.

But if something similar happens with AmigaGPT, maybe not only loading MUI indirectly, then try StackAttack.

[CCRider]

Originally Posted by Thomas Richter

Which type of an icon is that? A project icon or a tool icon?

Tool icon.

[meynaf]

Does it still happen if trying to setup a stack that's bigger than the minimum requirement ?

[CCRider]

Originally Posted by meynaf

Does it still happen if trying to setup a stack that's bigger than the minimum requirement ?

Yep. I tried setting it on Workbench Prefs (to 32 KB), and nothing changed.

[klx300r]

@ CCRider


try this as it helped me quite a bit over the years with stack issues


http://aminet.net/package/util/boot/StackAttack2

[PeterK]

AmigaGPT seems indeed to have a bug. When reading the stack size from its tool icon it divides the size by 4 by mistake! So, for an entry of 32768 it just sees 8192 bytes of stack.

When I tried 140000 for the stack setting AmigaGPT didn't complain anymore, but 131072 should also work (not tested).

[CCRider]

Originally Posted by PeterK

AmigaGPT seems indeed to have a bug. When reading the stack size from its tool icon it divides the size by 4 by mistake! So, for an entry of 32768 it just sees 8192 bytes of stack.

When I tried 140000 for the stack setting AmigaGPT didn't complain anymore, but 131072 should also work (not tested).

Whoa. It seems you nailed it! Will report to the dev.

[CCRider]

Originally Posted by PeterK

AmigaGPT seems indeed to have a bug. When reading the stack size from its tool icon it divides the size by 4 by mistake! So, for an entry of 32768 it just sees 8192 bytes of stack.

When I tried 140000 for the stack setting AmigaGPT didn't complain anymore, but 131072 should also work (not tested).

Tried here, and indeed 131072 is the lower limit where the warning doesn't appear.

Unfortunately, the author doesn't seem convinced that the bug is on his side.

[PeterK]

It's not the problem that AmigaGPT won't get enough stack with a setting of 32768, it really gets what it needs (you can see that in Scout's tasklist or with other stack monitoring tools). AmigaGPT just seems to have a bug in code which checks the stack size, maybe a shift right by 2 bits accidentally?

And I did my test on OS 3.9 with Rom 3.1.4, not on OS 3.2.2.x. For a MinStack value of 50000 AmigaGPT complained about getting 12500 only. Thousands of programs would have crashed in the past if that was true and a bug of the OS.

[PeterK]

Exactly as supposed: AmigaGPT divides the stack size by 4 with a shift right 2 bits:

...
lab_b8:
   move.l  $3e(a2),d0  ; 19d0 : 202a 003e
   sub.l   $3a(a2),d0  ; 19d4 : 90aa 003a      stack size
   asr.l   #2,d0       ; 19d8 : e480           shift right by 2 !!
   cmpi.l  #$7fff,d0   ; 19da : 0c80 0000 7fff compare with 32767
   bhi.w   lab_b0      ; 19e0 : 6200 fcf8
lab_b9:
   move.l  d0,-(sp)    ; 19e4 : 2f00
   pea     lab_6c(pc)  ; 19e6 : 487a f4d9      stack warning
   bsr.l   _printf     ; 19ea : 61ff 0000 b658
   addq.l  #8,sp       ; 19f0 : 508f
...

[CCRider]

Nice findings!

Informed the dev about them... Thanks a lot!

[Nightfox]

Hey, the dev here

I don't get why it would divide by 4. In my testing it does actually work for me.

Here'e the code that does all the work:

	struct Process *currentTask = (struct Process*)FindTask(NULL);
	struct CommandLineInterface *cli = (struct CommandLineInterface *)BADDR(currentTask->pr_CLI);

	/* If we started from Workbench then we must retrieve the startup message
	 before doing anything else. The startup message also contains a lock on
	 the program directory. */
	if (cli == NULL) {
		wbStartupMessage = (struct WBStartup*)GetMsg(&currentTask->pr_MsgPort);
	}

	ULONG *upper, *lower, total;

    // For CLI tasks, stack bounds are determined differently
    if (currentTask->pr_CLI) {
        upper = (ULONG *)currentTask->pr_ReturnAddr + sizeof(ULONG);
        total = *(ULONG *)currentTask->pr_ReturnAddr;
        lower = upper - total;
    } else {
        upper = (ULONG *)currentTask->pr_Task.tc_SPUpper;
        lower = (ULONG *)currentTask->pr_Task.tc_SPLower;
        total = upper - lower;
    }

	if (total < 32768) {
		printf("Warning: The stack size of %ld bytes is too small. The minimum recommended stack size is 32768 bytes to avoid crashes.\n", total);
	}

[a/b]

Because those are ULONG pointers, not BYTE pointers.

[Thomas Richter]

[QUOTE=Nightfox;1677059]Hey, the dev here

I don't get why it would divide by 4. In my testing it does actually work for me.

Here'e the code that does all the work:

     // For CLI tasks, stack bounds are determined differently

That is not correct. For the CLI, stack bounds are determined exactly as the stack bounds for the workbench. The shell or rather RunCommand() performs a complete stack swap and the stack bounds in the task structure are the correct one. The ReturnAddr is a legacy mechanism for BCPL programs.

[PeterK]

Originally Posted by Nightfox

... I don't get why it would divide by 4. In my testing it does actually work for me.

Here'e the code that does all the work:

...
 else {
        upper = (ULONG *)currentTask->pr_Task.tc_SPUpper;
        lower = (ULONG *)currentTask->pr_Task.tc_SPLower;
        total = upper - lower;
    }

	if (total < 32768) {
		printf("Warning: The stack size of %ld bytes is too small. The minimum recommended stack size is 32768 bytes to avoid crashes.\n", total);
	}

...
lab_b8:
   move.l  $3e(a2),d0  ; 19d0 : 202a 003e
   sub.l   $3a(a2),d0  ; 19d4 : 90aa 003a      stack size
   asr.l   #2,d0       ; 19d8 : e480           shift right by 2 !!
   cmpi.l  #$7fff,d0   ; 19da : 0c80 0000 7fff compare with 32767
   bhi.w   lab_b0      ; 19e0 : 6200 fcf8
lab_b9:
   move.l  d0,-(sp)    ; 19e4 : 2f00
   pea     lab_6c(pc)  ; 19e6 : 487a f4d9      stack warning
   bsr.l   _printf     ; 19ea : 61ff 0000 b658
   addq.l  #8,sp       ; 19f0 : 508f

My ADis disassembler code is from AmigaGPT 1.4.4 for OS 3.x.

I'm not a C coder and have no experience with GCC, but maybe there is a bug in the compiler, because both tc_SPUpper and tc_SPLower are APTR pointers, so there is no reason to shift anything by 2 in any direction. Please check your includes for the correct declaration of both pointers.

And the compiler also compares the stack size with 32767 in asm and not with 32768 as in your C code, maybe just an optimization, but if a compiler makes changes like that, then the resulting asm code gets unpredictable. I can send you the complete disassembler listing, but didn't want to publish it here without your permission.

[a/b]

As I said, he is recasting upper and lower to ULONG*, so when a pointer subtraction happens the result is also normalized.
If you inc/dec (++/--) a ULONG pointer, it's increased/decreased by 4, not by 1. M68K asm works the same way (predec/postinc are the operand size, not 1).
Use void*, char*, (un)signed byte* or similar instead of ULONG* (or multiply total by sizeof(ULONG) after subtraction) to get the size in bytes.