From 8be766f938d6042a4a05a1ddd6dcb4ec3b99f27b Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 29 Apr 2016 17:34:14 +0000 Subject: Refactor bytecode initialization machinery. --- generic/tclCompile.c | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/generic/tclCompile.c b/generic/tclCompile.c index 002b551..8665187 100644 --- a/generic/tclCompile.c +++ b/generic/tclCompile.c @@ -2735,6 +2735,29 @@ TclInitByteCodeObj( iPtr = envPtr->iPtr; + for (i = 0; i < numLitObjects; i++) { + if (objPtr == TclFetchLiteral(envPtr, i)) { + /* + * Prevent circular reference where the bytecode intrep of + * a value contains a literal which is that same value. + * If this is allowed to happen, refcount decrements may not + * reach zero, and memory may leak. Bugs 467523, 3357771 + * + * NOTE: [Bugs 3392070, 3389764] We make a copy based completely + * on the string value, and do not call Tcl_DuplicateObj() so we + * can be sure we do not have any lingering cycles hiding in + * the intrep. + */ + int numBytes; + const char *bytes = Tcl_GetStringFromObj(objPtr, &numBytes); + Tcl_Obj *copyPtr = Tcl_NewStringObj(bytes, numBytes); + + Tcl_IncrRefCount(copyPtr); + TclReleaseLiteral((Tcl_Interp *)iPtr, objPtr); + + envPtr->literalArrayPtr[i].objPtr = copyPtr; + } + } codeBytes = envPtr->codeNext - envPtr->codeStart; objArrayBytes = envPtr->literalArrayNext * sizeof(Tcl_Obj *); exceptArrayBytes = envPtr->exceptArrayNext * sizeof(ExceptionRange); @@ -2791,29 +2814,7 @@ TclInitByteCodeObj( p += TCL_ALIGN(codeBytes); /* align object array */ codePtr->objArrayPtr = (Tcl_Obj **) p; for (i = 0; i < numLitObjects; i++) { - Tcl_Obj *fetched = TclFetchLiteral(envPtr, i); - - if (objPtr == fetched) { - /* - * Prevent circular reference where the bytecode intrep of - * a value contains a literal which is that same value. - * If this is allowed to happen, refcount decrements may not - * reach zero, and memory may leak. Bugs 467523, 3357771 - * - * NOTE: [Bugs 3392070, 3389764] We make a copy based completely - * on the string value, and do not call Tcl_DuplicateObj() so we - * can be sure we do not have any lingering cycles hiding in - * the intrep. - */ - int numBytes; - const char *bytes = Tcl_GetStringFromObj(objPtr, &numBytes); - - codePtr->objArrayPtr[i] = Tcl_NewStringObj(bytes, numBytes); - Tcl_IncrRefCount(codePtr->objArrayPtr[i]); - TclReleaseLiteral((Tcl_Interp *)iPtr, objPtr); - } else { - codePtr->objArrayPtr[i] = fetched; - } + codePtr->objArrayPtr[i] = TclFetchLiteral(envPtr, i); } p += TCL_ALIGN(objArrayBytes); /* align exception range array */ -- cgit v0.12 From 272c398b78060d7d8b575bb5aa652f37aa08d666 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 29 Apr 2016 17:52:30 +0000 Subject: Tease apart the bytecode creation machinery from the Tcl_Obj intrep machinery. --- generic/tclCompile.c | 92 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 35 deletions(-) diff --git a/generic/tclCompile.c b/generic/tclCompile.c index 8665187..8837f06 100644 --- a/generic/tclCompile.c +++ b/generic/tclCompile.c @@ -2709,33 +2709,14 @@ TclCompileNoOp( *---------------------------------------------------------------------- */ -void -TclInitByteCodeObj( - Tcl_Obj *objPtr, /* Points object that should be initialized, - * and whose string rep contains the source - * code. */ - register CompileEnv *envPtr)/* Points to the CompileEnv structure from - * which to create a ByteCode structure. */ +static void +PreventCycle( + Tcl_Obj *objPtr, + CompileEnv *envPtr) { - register ByteCode *codePtr; - size_t codeBytes, objArrayBytes, exceptArrayBytes, cmdLocBytes; - size_t auxDataArrayBytes, structureSize; - register unsigned char *p; -#ifdef TCL_COMPILE_DEBUG - unsigned char *nextPtr; -#endif - int numLitObjects = envPtr->literalArrayNext; - Namespace *namespacePtr; - int i, isNew; - Interp *iPtr; - - if (envPtr->iPtr == NULL) { - Tcl_Panic("TclInitByteCodeObj() called on uninitialized CompileEnv"); - } - - iPtr = envPtr->iPtr; + int i; - for (i = 0; i < numLitObjects; i++) { + for (i = 0; i < envPtr->literalArrayNext; i++) { if (objPtr == TclFetchLiteral(envPtr, i)) { /* * Prevent circular reference where the bytecode intrep of @@ -2753,11 +2734,36 @@ TclInitByteCodeObj( Tcl_Obj *copyPtr = Tcl_NewStringObj(bytes, numBytes); Tcl_IncrRefCount(copyPtr); - TclReleaseLiteral((Tcl_Interp *)iPtr, objPtr); + TclReleaseLiteral((Tcl_Interp *)envPtr->iPtr, objPtr); envPtr->literalArrayPtr[i].objPtr = copyPtr; } } +} + +static ByteCode * +TclInitByteCode( + register CompileEnv *envPtr)/* Points to the CompileEnv structure from + * which to create a ByteCode structure. */ +{ + register ByteCode *codePtr; + size_t codeBytes, objArrayBytes, exceptArrayBytes, cmdLocBytes; + size_t auxDataArrayBytes, structureSize; + register unsigned char *p; +#ifdef TCL_COMPILE_DEBUG + unsigned char *nextPtr; +#endif + int numLitObjects = envPtr->literalArrayNext; + Namespace *namespacePtr; + int i, isNew; + Interp *iPtr; + + if (envPtr->iPtr == NULL) { + Tcl_Panic("TclInitByteCodeObj() called on uninitialized CompileEnv"); + } + + iPtr = envPtr->iPtr; + codeBytes = envPtr->codeNext - envPtr->codeStart; objArrayBytes = envPtr->literalArrayNext * sizeof(Tcl_Obj *); exceptArrayBytes = envPtr->exceptArrayNext * sizeof(ExceptionRange); @@ -2857,15 +2863,6 @@ TclInitByteCodeObj( #endif /* TCL_COMPILE_STATS */ /* - * Free the old internal rep then convert the object to a bytecode object - * by making its internal rep point to the just compiled ByteCode. - */ - - TclFreeIntRep(objPtr); - objPtr->internalRep.twoPtrValue.ptr1 = codePtr; - objPtr->typePtr = &tclByteCodeType; - - /* * TIP #280. Associate the extended per-word line information with the * byte code object (internal rep), for use with the bc compiler. */ @@ -2878,6 +2875,31 @@ TclInitByteCodeObj( envPtr->iPtr = NULL; codePtr->localCachePtr = NULL; + return codePtr; +} + +void +TclInitByteCodeObj( + Tcl_Obj *objPtr, /* Points object that should be initialized, + * and whose string rep contains the source + * code. */ + register CompileEnv *envPtr)/* Points to the CompileEnv structure from + * which to create a ByteCode structure. */ +{ + ByteCode *codePtr; + + PreventCycle(objPtr, envPtr); + + codePtr = TclInitByteCode(envPtr); + + /* + * Free the old internal rep then convert the object to a bytecode object + * by making its internal rep point to the just compiled ByteCode. + */ + + TclFreeIntRep(objPtr); + objPtr->internalRep.twoPtrValue.ptr1 = codePtr; + objPtr->typePtr = &tclByteCodeType; } /* -- cgit v0.12 From 44deb5c463cd7905f01f7cb74ac29d0e9d90972a Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 29 Apr 2016 17:58:33 +0000 Subject: Make obj-free bytecode maker available to rest of compile-related files. --- generic/tclCompile.c | 3 ++- generic/tclCompile.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/generic/tclCompile.c b/generic/tclCompile.c index 8837f06..32a09b2 100644 --- a/generic/tclCompile.c +++ b/generic/tclCompile.c @@ -677,6 +677,7 @@ static void FreeSubstCodeInternalRep(Tcl_Obj *objPtr); static int GetCmdLocEncodingSize(CompileEnv *envPtr); static int IsCompactibleCompileEnv(Tcl_Interp *interp, CompileEnv *envPtr); +static void PreventCycle(Tcl_Obj *objPtr, CompileEnv *envPtr); #ifdef TCL_COMPILE_STATS static void RecordByteCodeStats(ByteCode *codePtr); #endif /* TCL_COMPILE_STATS */ @@ -2741,7 +2742,7 @@ PreventCycle( } } -static ByteCode * +ByteCode * TclInitByteCode( register CompileEnv *envPtr)/* Points to the CompileEnv structure from * which to create a ByteCode structure. */ diff --git a/generic/tclCompile.h b/generic/tclCompile.h index 86a9db0..af8d60f 100644 --- a/generic/tclCompile.h +++ b/generic/tclCompile.h @@ -1118,6 +1118,7 @@ MODULE_SCOPE int TclFixupForwardJump(CompileEnv *envPtr, int distThreshold); MODULE_SCOPE void TclFreeCompileEnv(CompileEnv *envPtr); MODULE_SCOPE void TclFreeJumpFixupArray(JumpFixupArray *fixupArrayPtr); +MODULE_SCOPE ByteCode * TclInitByteCode(CompileEnv *envPtr); MODULE_SCOPE void TclInitByteCodeObj(Tcl_Obj *objPtr, CompileEnv *envPtr); MODULE_SCOPE void TclInitCompileEnv(Tcl_Interp *interp, -- cgit v0.12 From 100542f06740e0120d51ef844f61d47b10ce39ef Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 29 Apr 2016 18:02:55 +0000 Subject: No longer need to create Tcl_Obj just to make some bytecode. --- generic/tclCompExpr.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/generic/tclCompExpr.c b/generic/tclCompExpr.c index 4390282..654666a 100644 --- a/generic/tclCompExpr.c +++ b/generic/tclCompExpr.c @@ -2181,7 +2181,6 @@ ExecConstantExprTree( CompileEnv *envPtr; ByteCode *byteCodePtr; int code; - Tcl_Obj *byteCodeObj = Tcl_NewObj(); NRE_callback *rootPtr = TOP_CB(interp); /* @@ -2195,14 +2194,12 @@ ExecConstantExprTree( CompileExprTree(interp, nodes, index, litObjvPtr, NULL, NULL, envPtr, 0 /* optimize */); TclEmitOpcode(INST_DONE, envPtr); - Tcl_IncrRefCount(byteCodeObj); - TclInitByteCodeObj(byteCodeObj, envPtr); + byteCodePtr = TclInitByteCode(envPtr); TclFreeCompileEnv(envPtr); TclStackFree(interp, envPtr); - byteCodePtr = byteCodeObj->internalRep.twoPtrValue.ptr1; TclNRExecuteByteCode(interp, byteCodePtr); code = TclNRRunCallbacks(interp, TCL_OK, rootPtr); - Tcl_DecrRefCount(byteCodeObj); + TclReleaseByteCode(byteCodePtr); return code; } -- cgit v0.12 From d41d08e1111a2353e98c6814043b6441e6e71b64 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 29 Apr 2016 19:29:14 +0000 Subject: Parameterize TclInitByteCodeObj to callers sense of typePtr. --- generic/tclAssembly.c | 4 +--- generic/tclCompile.c | 12 ++++++------ generic/tclCompile.h | 4 ++-- generic/tclExecute.c | 4 +--- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/generic/tclAssembly.c b/generic/tclAssembly.c index 7db5d69..4ad31d2 100644 --- a/generic/tclAssembly.c +++ b/generic/tclAssembly.c @@ -891,15 +891,13 @@ CompileAssembleObj( */ TclEmitOpcode(INST_DONE, &compEnv); - TclInitByteCodeObj(objPtr, &compEnv); - objPtr->typePtr = &assembleCodeType; + codePtr = TclInitByteCodeObj(objPtr, &assembleCodeType, &compEnv); TclFreeCompileEnv(&compEnv); /* * Record the local variable context to which the bytecode pertains */ - codePtr = objPtr->internalRep.twoPtrValue.ptr1; if (iPtr->varFramePtr->localCachePtr) { codePtr->localCachePtr = iPtr->varFramePtr->localCachePtr; codePtr->localCachePtr->refCount++; diff --git a/generic/tclCompile.c b/generic/tclCompile.c index 32a09b2..96b418c 100644 --- a/generic/tclCompile.c +++ b/generic/tclCompile.c @@ -868,7 +868,7 @@ TclSetByteCodeFromAny( #endif /*TCL_COMPILE_DEBUG*/ if (result == TCL_OK) { - TclInitByteCodeObj(objPtr, &compEnv); + (void) TclInitByteCodeObj(objPtr, &tclByteCodeType, &compEnv); #ifdef TCL_COMPILE_DEBUG if (tclTraceCompile >= 2) { TclPrintByteCodeObj(interp, objPtr); @@ -1322,11 +1322,9 @@ CompileSubstObj( TclSubstCompile(interp, bytes, numBytes, flags, 1, &compEnv); TclEmitOpcode(INST_DONE, &compEnv); - TclInitByteCodeObj(objPtr, &compEnv); - objPtr->typePtr = &substCodeType; + codePtr = TclInitByteCodeObj(objPtr, &substCodeType, &compEnv); TclFreeCompileEnv(&compEnv); - codePtr = objPtr->internalRep.twoPtrValue.ptr1; objPtr->internalRep.twoPtrValue.ptr1 = codePtr; objPtr->internalRep.twoPtrValue.ptr2 = INT2PTR(flags); if (iPtr->varFramePtr->localCachePtr) { @@ -2879,11 +2877,12 @@ TclInitByteCode( return codePtr; } -void +ByteCode * TclInitByteCodeObj( Tcl_Obj *objPtr, /* Points object that should be initialized, * and whose string rep contains the source * code. */ + const Tcl_ObjType *typePtr, register CompileEnv *envPtr)/* Points to the CompileEnv structure from * which to create a ByteCode structure. */ { @@ -2900,7 +2899,8 @@ TclInitByteCodeObj( TclFreeIntRep(objPtr); objPtr->internalRep.twoPtrValue.ptr1 = codePtr; - objPtr->typePtr = &tclByteCodeType; + objPtr->typePtr = typePtr; + return codePtr; } /* diff --git a/generic/tclCompile.h b/generic/tclCompile.h index af8d60f..f99c07c 100644 --- a/generic/tclCompile.h +++ b/generic/tclCompile.h @@ -1119,8 +1119,8 @@ MODULE_SCOPE int TclFixupForwardJump(CompileEnv *envPtr, MODULE_SCOPE void TclFreeCompileEnv(CompileEnv *envPtr); MODULE_SCOPE void TclFreeJumpFixupArray(JumpFixupArray *fixupArrayPtr); MODULE_SCOPE ByteCode * TclInitByteCode(CompileEnv *envPtr); -MODULE_SCOPE void TclInitByteCodeObj(Tcl_Obj *objPtr, - CompileEnv *envPtr); +MODULE_SCOPE ByteCode * TclInitByteCodeObj(Tcl_Obj *objPtr, + const Tcl_ObjType *typePtr, CompileEnv *envPtr); MODULE_SCOPE void TclInitCompileEnv(Tcl_Interp *interp, CompileEnv *envPtr, const char *string, int numBytes, const CmdFrame *invoker, int word); diff --git a/generic/tclExecute.c b/generic/tclExecute.c index 2a2b951..cd28a92 100644 --- a/generic/tclExecute.c +++ b/generic/tclExecute.c @@ -1565,10 +1565,8 @@ CompileExprObj( */ TclEmitOpcode(INST_DONE, &compEnv); - TclInitByteCodeObj(objPtr, &compEnv); - objPtr->typePtr = &exprCodeType; + codePtr = TclInitByteCodeObj(objPtr, &exprCodeType, &compEnv); TclFreeCompileEnv(&compEnv); - codePtr = objPtr->internalRep.twoPtrValue.ptr1; if (iPtr->varFramePtr->localCachePtr) { codePtr->localCachePtr = iPtr->varFramePtr->localCachePtr; codePtr->localCachePtr->refCount++; -- cgit v0.12