From 1bb7bf057245c23fb001032ceb3df30a99b093ee Mon Sep 17 00:00:00 2001 From: Miguel Sofer Date: Sat, 10 Nov 2007 19:01:33 +0000 Subject: * generic/tclBasic.c: * generic/tclInt.h: * unix/tclUnixInit.c: * win/tclWin32Dll.c: restore simpler behaviour for stack checking, not adaptive to stack size changes after a thread is launched. Consensus is that "nobody does that", and so it is not worth the cost. --- ChangeLog | 10 +++++++ generic/tclBasic.c | 27 +++++++------------ generic/tclInt.h | 6 ++--- unix/tclUnixInit.c | 77 +++++++++++++++++++++++++++++++----------------------- win/tclWin32Dll.c | 6 ++--- 5 files changed, 70 insertions(+), 56 deletions(-) diff --git a/ChangeLog b/ChangeLog index e7eb651..065efec 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2007-11-10 Miguel Sofer + + * generic/tclBasic.c: + * generic/tclInt.h: + * unix/tclUnixInit.c: + * win/tclWin32Dll.c: restore simpler behaviour for stack checking, + not adaptive to stack size changes after a thread is + launched. Consensus is that "nobody does that", and so it is not + worth the cost. + 2007-11-10 Kevin Kenny * win/tclWin32Dll.c: Rewrote the Windows stack checking algorithm diff --git a/generic/tclBasic.c b/generic/tclBasic.c index 2684986..53cd4fd 100644 --- a/generic/tclBasic.c +++ b/generic/tclBasic.c @@ -14,7 +14,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclBasic.c,v 1.273 2007/11/10 16:08:09 msofer Exp $ + * RCS: @(#) $Id: tclBasic.c,v 1.274 2007/11/10 19:01:34 msofer Exp $ */ #include "tclInt.h" @@ -338,15 +338,15 @@ static const OpCmdInfo mathOpCmds[] = { static int stackGrowsDown = 1; #define GetCStackParams(iPtr) \ - stackGrowsDown = TclpGetCStackParams(&((iPtr)->stackBoundPtr)) + stackGrowsDown = TclpGetCStackParams(&((iPtr)->stackBound)) -#define CheckStackSpace(iPtr, localIntPtr) \ +#define StackOverflow(iPtr, localIntPtr) \ (stackGrowsDown \ - ? ((localIntPtr) > *((iPtr)->stackBoundPtr)) \ - : ((localIntPtr) < *((iPtr)->stackBoundPtr)) \ + ? ((localIntPtr) < (iPtr)->stackBound) \ + : ((localIntPtr) > (iPtr)->stackBound) \ ) #else /* stack check disabled: make them noops */ -#define CheckStackSpace(interp, localIntPtr) 1 +#define StackOverflow(interp, localIntPtr) 0 #define GetCStackParams(iPtr) #endif @@ -3407,7 +3407,7 @@ int TclInterpReady( Tcl_Interp *interp) { - int localInt, stackOverflow; /* used for checking the stack */ + int localInt; /* used for checking the stack */ register Interp *iPtr = (Interp *) interp; /* @@ -3435,17 +3435,8 @@ TclInterpReady( * probably because of an infinite loop somewhere. */ - stackOverflow = !CheckStackSpace(iPtr, &localInt); - if (stackOverflow) { - /* - * Update the stack params in case the thread's stack was grown. - */ - - GetCStackParams(iPtr); - stackOverflow = !CheckStackSpace(iPtr, &localInt); - } - - if (stackOverflow || ((iPtr->numLevels) > iPtr->maxNestingDepth)) { + if (((iPtr->numLevels) > iPtr->maxNestingDepth) + || StackOverflow(iPtr, &localInt)) { Tcl_AppendResult(interp, "too many nested evaluations (infinite loop?)", NULL); return TCL_ERROR; diff --git a/generic/tclInt.h b/generic/tclInt.h index 1dc744c..6a72767 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -13,7 +13,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclInt.h,v 1.339 2007/11/10 16:08:10 msofer Exp $ + * RCS: @(#) $Id: tclInt.h,v 1.340 2007/11/10 19:01:35 msofer Exp $ */ #ifndef _TCLINT @@ -1855,7 +1855,7 @@ typedef struct Interp { * tclObj.c and tclThreadAlloc.c */ int *asyncReadyPtr; /* Pointer to the asyncReady indicator for * this interp's thread; see tclAsync.c */ - int **stackBoundPtr; /* Pointer to the limit stack address + int *stackBound; /* Pointer to the limit stack address * allowable for invoking a new command * without "risking" a C-stack overflow; * see TclpCheckStackSpace in the @@ -2502,7 +2502,7 @@ MODULE_SCOPE int TclParseAllWhiteSpace(CONST char *src, int numBytes); MODULE_SCOPE int TclProcessReturn(Tcl_Interp *interp, int code, int level, Tcl_Obj *returnOpts); #ifndef TCL_NO_STACK_CHECK -MODULE_SCOPE int TclpGetCStackParams(int ***stackBound); +MODULE_SCOPE int TclpGetCStackParams(int **stackBoundPtr); #endif MODULE_SCOPE int TclpObjLstat(Tcl_Obj *pathPtr, Tcl_StatBuf *buf); MODULE_SCOPE Tcl_Obj * TclpTempFileName(void); diff --git a/unix/tclUnixInit.c b/unix/tclUnixInit.c index 4ae061c..86edc38 100644 --- a/unix/tclUnixInit.c +++ b/unix/tclUnixInit.c @@ -7,7 +7,7 @@ * Copyright (c) 1999 by Scriptics Corporation. * All rights reserved. * - * RCS: @(#) $Id: tclUnixInit.c,v 1.75 2007/11/10 16:08:10 msofer Exp $ + * RCS: @(#) $Id: tclUnixInit.c,v 1.76 2007/11/10 19:01:35 msofer Exp $ */ #include "tclInt.h" @@ -78,7 +78,6 @@ typedef struct ThreadSpecificData { int *outerVarPtr; /* The "outermost" stack frame pointer for * this thread. */ - int stackDetermineResult; /* What happened when we did that? */ int *stackBound; /* The current stack boundary */ } ThreadSpecificData; static Tcl_ThreadDataKey dataKey; @@ -1028,21 +1027,21 @@ TclpFindVariable( #ifndef TCL_NO_STACK_CHECK int TclpGetCStackParams( - int ***stackBoundPtr) + int **stackBoundPtr) { - int localVar; - size_t stackSize; /* The size of the current stack. */ + int result = TCL_OK; + size_t stackSize = 0; /* The size of the current stack. */ ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); /* Most variables are actually in a * thread-specific data block to minimise the * impact on the stack. */ - + if (stackGrowsDown == -1) { /* * Not initialised! */ - stackGrowsDown = StackGrowsDown(&localVar); + stackGrowsDown = StackGrowsDown(&result); } /* @@ -1051,37 +1050,47 @@ TclpGetCStackParams( */ if (tsdPtr->outerVarPtr == NULL) { - tsdPtr->outerVarPtr = &localVar; - tsdPtr->stackDetermineResult = GetStackSize(&stackSize); + tsdPtr->outerVarPtr = &result; + result = GetStackSize(&stackSize); + if (result != TCL_OK) { + /* Can't check, assume it always succeeds */ + stackGrowsDown = 1; + tsdPtr->stackBound = NULL; + goto done; + } } - if ((stackGrowsDown && (&localVar < tsdPtr->stackBound)) - || (!stackGrowsDown && (&localVar > tsdPtr->stackBound))) { - /* - * Stack failure - if we didn't already blow up, we are within the - * safety area. Recheck with the OS in case the stack was grown. - */ + if (stackSize || (stackGrowsDown && (&result < tsdPtr->stackBound)) + || (!stackGrowsDown && (&result > tsdPtr->stackBound))) { + /* + * Either the thread's first pass or stack failure: set the params + */ - tsdPtr->stackDetermineResult = GetStackSize(&stackSize); - } + if (!stackSize) { + /* + * Stack failure: if we didn't already blow up, we are within the + * safety area. Recheck with the OS in case the stack was grown. + */ + result = GetStackSize(&stackSize); + if (result != TCL_OK) { + /* Can't check, assume it always succeeds */ + stackGrowsDown = 1; + tsdPtr->stackBound = NULL; + goto done; + } + } - if (tsdPtr->stackDetermineResult != TCL_OK) { - switch (tsdPtr->stackDetermineResult) { - case TCL_BREAK: - STACK_DEBUG(("skipping stack checks with failure\n")); - case TCL_CONTINUE: - STACK_DEBUG(("skipping stack checks with success\n")); + if (stackGrowsDown) { + tsdPtr->stackBound = (int *) ((char *)tsdPtr->outerVarPtr - + stackSize); + } else { + tsdPtr->stackBound = (int *) ((char *)tsdPtr->outerVarPtr + + stackSize); } - stackGrowsDown = 1; - tsdPtr->stackBound = NULL; - } else if (stackGrowsDown) { - tsdPtr->stackBound = (int *) ((char *)tsdPtr->outerVarPtr - - stackSize); - } else { - tsdPtr->stackBound = (int *) ((char *)tsdPtr->outerVarPtr + - stackSize); } - *stackBoundPtr = &(tsdPtr->stackBound); + + done: + *stackBoundPtr = tsdPtr->stackBound; return stackGrowsDown; } @@ -1132,6 +1141,7 @@ GetStackSize( /* * Some kind of confirmed error?! */ + STACK_DEBUG(("skipping stack checks with failure\n")); return TCL_BREAK; } if (rawStackSize > 0) { @@ -1149,12 +1159,14 @@ GetStackSize( /* * getrlimit() failed, just fail the whole thing. */ + STACK_DEBUG(("skipping stack checks with failure: getrlimit failed\n")); return TCL_BREAK; } if (rLimit.rlim_cur == RLIM_INFINITY) { /* * Limit is "infinite"; there is no stack limit. */ + STACK_DEBUG(("skipping stack checks with success: infinite limit\n")); return TCL_CONTINUE; } rawStackSize = rLimit.rlim_cur; @@ -1169,6 +1181,7 @@ GetStackSize( finalSanityCheck: #endif if (rawStackSize <= 0) { + STACK_DEBUG(("skipping stack checks with success\n")); return TCL_CONTINUE; } diff --git a/win/tclWin32Dll.c b/win/tclWin32Dll.c index 0ad344e..e7d20f5 100644 --- a/win/tclWin32Dll.c +++ b/win/tclWin32Dll.c @@ -10,7 +10,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclWin32Dll.c,v 1.51 2007/11/10 17:40:35 kennykb Exp $ + * RCS: @(#) $Id: tclWin32Dll.c,v 1.52 2007/11/10 19:01:35 msofer Exp $ */ #include "tclWinInt.h" @@ -544,7 +544,7 @@ TclWinNoBackslash( #ifndef TCL_NO_STACK_CHECK int TclpGetCStackParams( - int ***stackBoundPtr) + int **stackBoundPtr) { ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); SYSTEM_INFO si; /* The system information, used to @@ -592,7 +592,7 @@ TclpGetCStackParams( + TCL_WIN_STACK_THRESHOLD); } } - *stackBoundPtr = &(tsdPtr->stackBound); + *stackBoundPtr = tsdPtr->stackBound; return 1; } #endif -- cgit v0.12