[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MiNT] USERDEF stack size



Vincent Rivière wrote:
I wonder if the right hack would not be to switch to restore USP and switch to user mode before calling the USERDEFs callbacks...

Switching to user mode would be complicated and useless, but switching to USP is very simple and efficient :-) I did that and it fixed everything, my freshly compiled QED works perfectly on TOS.

I have made a proof of concept patch for the CF-Lib, you can find it as attachment. Please don't commit it, that's experimental stuff.

Basically, I have made a new AES structure named SAFE_USERBLK. It is almost like USERBLK, but will automatically switch to the user stack when necessary. It is really easy to use, I think it will be very hard to do better.

If you want to make your USERDEFs safe, here is what you have to do:

1) Use SAFE_USERBLK instead of USERBLK:
+#include "safe_userdef.h"
-static USERBLK userblk;
+static SAFE_USERBLK userblk;

2) Before assigning ub_code, be sure to assign the ub_protector member to safe_user_block_protector (a C++ constructor would have been welcome).
+	userblk.ub_protector = safe_user_block_protector;
 	userblk.ub_code = colorpop_userdef;


3) When assigning the ob_spec member of the G_USERDEF object, the address of the SAFE_USERBLK must be cast to USERBLK * because USERBLK and SAFE_USERBLK are unrelated (C++ inheritance would have been welcome).
-		poptree[i].ob_spec.userblk = &userblk;
+		poptree[i].ob_spec.userblk = (USERBLK *)&userblk;

That's all. Your USERDEFs are now protected against the small AES stack issue. Easy, isn't it ? I have patched all the USERDEFs in the CF-Lib like this (included in the attached patch), so any program using the CF-Lib will be automatically protected. Now QED works fine on TOS without any modification in its own sources or the GemLib.

How does it work ?

safe_user_block_protector() is the real USERDEF callback, called by the AES. The trick is that the member ub_protector in SAFE_USERBLK is at the same offset than ub_code in USERBLK. Then I have added an additional field in SAFE_USERBLK named ub_code at the end, to store the user callback. So when you assign ub_code (as usual) you are actually assigning the additional field! And when you assign ub_protector, you are actually assigning the real callback.

Then safe_user_block_protector() is called by the AES, it switches the stack if required and call the user callback. The user code remains unmodified, it can contain function calls to functions requiring big stack sizes like v_gtext(), etc.

First, how to determine if the stack must be switched or not ?
I have used an heuristic which seems to work well: the user stack is always somewhere after the basepage. And the AES stack is very low on the memory. So if the current stack is below the basepage, we are on the AES stack and we should try to switch to the user stack. Usually, it is available in the USP register. It is the case when drawing dialogs. Unfortunately, the main menus are drawn by an AES process named "screen manager", which has no user stack. So custom separators (like CF-Lib ones) and submenus still have to work on the small AES stack. It doesn't seem to be a problem with QED. If one day we have trouble because of that, we could easily use a statically allocated stack for that special case.

That's all ! If you think this method is correct, we could add safe_userdef.c and safe_userdef.h to the *GemLib*, so SAFE_USERBLK will be available to every user program. It will be provided but unused inside the GemLib, so there will be no hidden regression. Then the CF-Lib will be able to use it (as I did in the current patch) so it will be an immediate benefit for all the programs using the CF-Lib (like QED).
Then I will be able to build QED for ColdFire and run it on EmuTOS.

Consider this as a present - Merry Chistmas :-)

--
Vincent Rivière
diff -aurN cflib.orig/colorpop.c cflib/colorpop.c
--- cflib.orig/colorpop.c	2010-12-25 00:39:41.359375000 +0100
+++ cflib/colorpop.c	2010-12-24 23:11:14.531250000 +0100
@@ -29,6 +29,7 @@
  */
 
 #include "intern.h"
+#include "safe_userdef.h"
 #include <assert.h>
 
 
@@ -53,9 +54,9 @@
 static int max_displayable_planes;		/* Maximal darstellbare Planes in dieser Auflsung */
 static int framewidth;				/* Berechnung des Abstands zwischen Farbfeldern */
 static short fattrib[5], lattrib[6];		/* Sicherung fr Fll- und Linienattribute */
-static USERBLK userblk;				/* Ein Userblock fr alle Eintrge im Popup-Men */
-static USERBLK noncolor_userblk;		/* Userblock fr "Nichtfarbe"-Eintrag */
-static USERBLK popobj_userblk[MAX_COLORPOP];	/* Userblock-Liste fr alle Farbobjekte */
+static SAFE_USERBLK userblk;			/* Ein Userblock fr alle Eintrge im Popup-Men */
+static SAFE_USERBLK noncolor_userblk;		/* Userblock fr "Nichtfarbe"-Eintrag */
+static SAFE_USERBLK popobj_userblk[MAX_COLORPOP]; /* Userblock-Liste fr alle Farbobjekte */
 static int nocolor_sub;
 static int handle, use_3D;
 
@@ -317,11 +318,11 @@
 is_colorpop_object (OBJECT *tree, short obj)
 {
 	return (tree[obj].ob_type & 0xff) == G_USERDEF
-		&& ((tree[obj].ob_spec.userblk == &userblk)
-		    || (tree[obj].ob_spec.userblk == &noncolor_userblk)
-		    || (tree[obj].ob_spec.userblk >= popobj_userblk
+		&& ((tree[obj].ob_spec.userblk == (USERBLK *)&userblk)
+		    || (tree[obj].ob_spec.userblk == (USERBLK *)&noncolor_userblk)
+		    || (tree[obj].ob_spec.userblk >= (USERBLK *)popobj_userblk
 			&& tree[obj].ob_spec.userblk <
-			popobj_userblk + MAX_COLORPOP));
+			(USERBLK *)(popobj_userblk + MAX_COLORPOP)));
 }
 
 /* Farb-Popub-Objektfarbe setzen */
@@ -346,14 +347,15 @@
 void
 fix_colorpopobj (OBJECT *tree, short obj, short color)
 {
-	USERBLK *userblkp = popobj_userblk;
+	SAFE_USERBLK *userblkp = popobj_userblk;
 
 	while (userblkp->ub_code)
 		userblkp++;
 
 	/* FIXME: Zur Zeit funzt das wg. get_objframe() nicht mit altem Typ im hibyte. */
 	tree[obj].ob_type = G_USERDEF;
-	tree[obj].ob_spec.userblk = userblkp;
+	tree[obj].ob_spec.userblk = (USERBLK *)userblkp;
+	userblkp->ub_protector = safe_user_block_protector;
 	userblkp->ub_code = colorpop_userdef;
 	userblkp->ub_parm = F_USEROBJ | (long) color;
 
@@ -376,8 +378,10 @@
 	else
 		use_3D = FALSE;
 
+	userblk.ub_protector = safe_user_block_protector;
 	userblk.ub_code = colorpop_userdef;	/* Userblk fr die Farbfelder im Farb-Popup init. */
 	userblk.ub_parm = (long) T_COLPOP & 0xffffl;	/* -1: Farbe wird je nach Index im Baum dargestellt. */
+	noncolor_userblk.ub_protector = safe_user_block_protector;
 	noncolor_userblk.ub_code = colorpop_userdef;	/* Userblk fr das "Nichtfarb"-Feld im Farb-Popup init. */
 	noncolor_userblk.ub_parm = (long) T_NOCOLOR & 0xffffl;	/* -2: Nichtfarbe wird dargestellt. */
 
@@ -476,7 +480,7 @@
 		poptree[i].ob_width = size;
 		poptree[i].ob_height = size;
 		poptree[i].ob_type = G_USERDEF;
-		poptree[i].ob_spec.userblk = &userblk;
+		poptree[i].ob_spec.userblk = (USERBLK *)&userblk;
 		poptree[i].ob_flags = OF_SELECTABLE;
 		poptree[i].ob_state = 0;
 		
@@ -490,7 +494,7 @@
 	}
 	
 	if (show_noncolor)
-		poptree[1].ob_spec.userblk = &noncolor_userblk;
+		poptree[1].ob_spec.userblk = (USERBLK *)&noncolor_userblk;
 	
 	poptree[colors].ob_next = 0;
 	poptree[colors].ob_flags |= OF_LASTOB;
diff -aurN cflib.orig/safe_userdef.c cflib/safe_userdef.c
--- cflib.orig/safe_userdef.c	1970-01-01 01:00:00.000000000 +0100
+++ cflib/safe_userdef.c	2010-12-25 00:36:43.984375000 +0100
@@ -0,0 +1,108 @@
+/*
+ * CFLIB, a GEM library for ATARI/TOS
+ * Copyright (C) 1999, 2000 Christian Felsch
+ * 
+ * Modified for FreeMiNT CVS by Frank Naumann <fnaumann@freemint.de>
+ * 
+ * Please send suggestions, patches or bug reports to me or
+ * the MiNT mailing list.
+ * 
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ * 
+ */
+
+#include "safe_userdef.h"
+#include <compiler.h>
+#include <mint/basepage.h>
+
+static __inline__ void *
+get_current_stack (void)
+{
+	register void * retvalue;
+
+	__asm__ volatile
+	(
+		"move.l sp,%0"
+	: "=r"(retvalue)			/* outputs */
+	:					/* inputs  */
+	:					/* clobbered regs */
+	);
+
+	return retvalue;
+}
+
+static __inline__ void *
+get_user_stack (void)
+{
+	register void * retvalue;
+
+	__asm__ volatile
+	(
+		"move.l usp,%0"
+	: "=a"(retvalue)			/* outputs */
+	:					/* inputs  */
+	:					/* clobbered regs */
+	);
+
+	return retvalue;
+}
+
+static __inline__ short
+call_on_specified_stack (void* stack, short cdecl (*proc)(PARMBLK *p), PARMBLK *p)
+{
+	register short retvalue __asm__("d0");
+
+	__asm__ volatile
+	(
+		"move.l sp,a2\n\t"
+		"move.l %1,sp\n\t"
+		"move.l %3,-(sp)\n\t"
+		"move.l %2,a0\n\t"
+		"jsr    (a0)\n\t"
+		"move.l a2,sp"
+	: "=r"(retvalue)			/* outputs */
+	: "r"(stack), "r"(proc), "r"(p)		/* inputs  */
+	: __CLOBBER_RETURN("d0") "d1", "a0", "a1", "a2" /* clobbered regs */
+	  AND_MEMORY
+	);
+
+	return retvalue;
+}
+
+short cdecl
+safe_user_block_protector (PARMBLK *p)
+{
+        SAFE_USERBLK *uPtr = (SAFE_USERBLK *)p->pb_tree[p->pb_obj].ob_spec.userblk;
+	short cdecl (*proc)(PARMBLK *p) = uPtr->ub_code;
+	void *current_stack = get_current_stack ();
+	void *user_stack = get_user_stack ();
+
+	/* If the current stack is below the BASEPAGE, */
+	/* we can assume we are on the small AES stack, */
+	/* so we can use the user stack instead to get more room. */
+	/* Unfortunately, the main menus are drawn by */
+	/* the AES screen manager which has no user stack. */
+	if (current_stack <= (void *)_base && user_stack != NULL)
+	{
+		/* We can call the callback using the big user stack. */
+		return call_on_specified_stack (user_stack, proc, p);
+	}
+	else
+	{
+		/* We must call the callback on the current stack */
+		return proc (p);
+	}
+}
diff -aurN cflib.orig/safe_userdef.h cflib/safe_userdef.h
--- cflib.orig/safe_userdef.h	1970-01-01 01:00:00.000000000 +0100
+++ cflib/safe_userdef.h	2010-12-24 23:30:49.828125000 +0100
@@ -0,0 +1,43 @@
+/*
+ * CFLIB, a GEM library for ATARI/TOS
+ * Copyright (C) 1999, 2000 Christian Felsch
+ * 
+ * Modified for FreeMiNT CVS by Frank Naumann <fnaumann@freemint.de>
+ * 
+ * Please send suggestions, patches or bug reports to me or
+ * the MiNT mailing list.
+ * 
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ * 
+ */
+
+#ifndef _cfl_safe_userdef_
+#define _cfl_safe_userdef_
+
+#include "intern.h"
+
+typedef struct safe_user_block
+{
+	short __CDECL (*ub_protector)(PARMBLK *parmblock); /* Protector callback */
+	long ub_parm; /* Original parameter */
+        short cdecl (*ub_code)(PARMBLK *p); /* Original callback */
+} SAFE_USERBLK;
+
+/* The following function must be affected to every SAFE_USERBLK.ub_protector */
+short cdecl
+safe_user_block_protector (PARMBLK *p);
+
+#endif /* _cfl_safe_userdef_ */
diff -aurN cflib.orig/userdef.c cflib/userdef.c
--- cflib.orig/userdef.c	2010-12-25 00:39:41.890625000 +0100
+++ cflib/userdef.c	2010-12-24 23:25:40.343750000 +0100
@@ -43,12 +43,13 @@
 
 #include "inline.rsh"
 #include "inline.rh"
+#include "safe_userdef.h"
 
 OBJECT *cf_ascii_tab = NULL;
 OBJECT *cf_alert_box = NULL;
 
 static short handle = -1, ud_wchar, ud_hchar, ud_wbox, ud_hbox;
-static USERBLK menu_blk, popup_blk;
+static SAFE_USERBLK menu_blk, popup_blk;
 static short gnva, use_3D, mx_buttons, mx_shortcut;
 static short back_col;
 static CICONBLK *radio_cib, *check_cib;
@@ -509,17 +510,18 @@
 static void
 make_userdef (OBJECT *tree, short obj, short cdecl (*proc)(PARMBLK *p))
 {
-	USERBLK *uPtr;
+	SAFE_USERBLK *uPtr;
 
-	uPtr = cf_malloc (sizeof (USERBLK), "make_userdef", FALSE);
+	uPtr = cf_malloc (sizeof (SAFE_USERBLK), "make_userdef", FALSE);
 	if (uPtr)
 	{
+		uPtr->ub_protector = safe_user_block_protector;
 		uPtr->ub_code = proc;	/* neue Zeichenfunktion */
 		uPtr->ub_parm = tree[obj].ob_spec.index;	/* alte obSpec sichern */
 
 		/* alten Typ hochschieben und neuen eintragen */
 		tree[obj].ob_type = (tree[obj].ob_type << 8) + G_USERDEF;
-		tree[obj].ob_spec.index = (long) uPtr;	/* Userblock eintragen */
+		tree[obj].ob_spec.userblk = (USERBLK *)uPtr;	/* Userblock eintragen */
 	}
 }
 
@@ -595,7 +597,7 @@
 			{
 				tree[i].ob_type =
 					(tree[i].ob_type << 8) + G_USERDEF;
-				tree[i].ob_spec.userblk = &menu_blk;
+				tree[i].ob_spec.userblk = (USERBLK *)&menu_blk;
 			}
 		}
 	}
@@ -623,9 +625,9 @@
 				tree[i].ob_type =
 					(tree[i].ob_type << 8) + G_USERDEF;
 				if (thin_line)
-					tree[i].ob_spec.userblk = &popup_blk;
+					tree[i].ob_spec.userblk = (USERBLK *)&popup_blk;
 				else
-					tree[i].ob_spec.userblk = &menu_blk;
+					tree[i].ob_spec.userblk = (USERBLK *)&menu_blk;
 			}
 		}
 	}
@@ -684,10 +686,12 @@
 	init_inline_rsc ();
 
 	menu_blk.ub_parm = 0;
+	menu_blk.ub_protector = safe_user_block_protector;
 	menu_blk.ub_code = draw_menuline;
 	gnva = getcookie ("Gnva", NULL);
 
 	popup_blk.ub_parm = 0;
+	popup_blk.ub_protector = safe_user_block_protector;
 	popup_blk.ub_code = draw_popupline;
 }