-- cgit v0.12 From 951e955d2c89cad1bd96d2e9ec08233d1a14f2f1 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Fri, 22 Jul 2022 06:32:40 +0000 Subject: Added testapplylambda to illustrate bug in apply when the passed argument does NOT have Lambda internal representation but the body of the lambda DOES have an internal ByteCode representation. --- generic/tclTest.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/generic/tclTest.c b/generic/tclTest.c index e3c6663..77540e2 100644 --- a/generic/tclTest.c +++ b/generic/tclTest.c @@ -342,6 +342,7 @@ static Tcl_ObjCmdProc TestInterpResolverCmd; #if defined(HAVE_CPUID) && !defined(MAC_OSX_TCL) static Tcl_ObjCmdProc TestcpuidCmd; #endif +static Tcl_ObjCmdProc TestApplyLambdaObjCmd; static const Tcl_Filesystem testReportingFilesystem = { "reporting", @@ -715,6 +716,8 @@ Tcltest_Init( NULL, NULL); Tcl_CreateObjCommand(interp, "testsetencpath", TestsetencpathObjCmd, NULL, NULL); + Tcl_CreateObjCommand(interp, "testapplylambda", TestApplyLambdaObjCmd, + NULL, NULL); if (TclObjTest_Init(interp) != TCL_OK) { return TCL_ERROR; @@ -8120,7 +8123,85 @@ TestInterpResolverCmd( } return TCL_OK; } - + +/* + *------------------------------------------------------------------------ + * + * TestApplyLambdaObjCmd -- + * + * Implements the Tcl command testapplylambda. This tests the apply + * implementation handling of a lambda where the lambda has a list + * internal representation where the second element's internal + * representation is already a byte code object. + * + * Results: + * TCL_OK - Success. Caller should check result is 42 + * TCL_ERROR - Error. + * + * Side effects: + * In the presence of the apply bug, may panic. Otherwise + * Interpreter result holds result or error message. + * + *------------------------------------------------------------------------ + */ +int TestApplyLambdaObjCmd ( + ClientData notUsed, + Tcl_Interp *interp, /* Current interpreter. */ + int objc, /* Number of arguments. */ + Tcl_Obj *const objv[]) /* Argument objects. */ +{ + Tcl_Obj *lambdaObjs[2]; + Tcl_Obj *evalObjs[2]; + Tcl_Obj *lambdaObj; + int result; + + /* Create a lambda {{} {set a 42}} */ + lambdaObjs[0] = Tcl_NewObj(); /* No parameters */ + lambdaObjs[1] = Tcl_NewStringObj("set a 42", -1); /* Body */ + lambdaObj = Tcl_NewListObj(2, lambdaObjs); + Tcl_IncrRefCount(lambdaObj); + + /* Create the command "apply {{} {set a 42}" */ + evalObjs[0] = Tcl_NewStringObj("apply", -1); + Tcl_IncrRefCount(evalObjs[0]); + /* + * NOTE: IMPORTANT TO EXHIBIT THE BUG. We duplicate the lambda because + * it will get shimmered to a Lambda internal representation but we + * want to hold on to our list representation. + */ + evalObjs[1] = Tcl_DuplicateObj(lambdaObj); + Tcl_IncrRefCount(evalObjs[1]); + + /* Evaluate it */ + result = Tcl_EvalObjv(interp, 2, evalObjs, TCL_EVAL_GLOBAL); + if (result != TCL_OK) { + Tcl_DecrRefCount(evalObjs[0]); + Tcl_DecrRefCount(evalObjs[1]); + return result; + } + /* + * So far so good. At this point, + * - evalObjs[1] has an internal representation of Lambda + * - lambdaObj[1] ({set a 42}) has been shimmered to + * an internal representation of ByteCode. + */ + Tcl_DecrRefCount(evalObjs[1]); /* Don't need this anymore */ + /* + * The bug trigger. Repeating the command but: + * - we are calling apply with a lambda that is a list (as BEFORE), + * BUT + * - The body of the lambda (lambdaObjs[1]) ALREADY has internal + * representation of ByteCode and thus will not be compiled again + */ + evalObjs[1] = lambdaObj; /* lambdaObj already has a ref count so + no need for IncrRef */ + result = Tcl_EvalObjv(interp, 2, evalObjs, TCL_EVAL_GLOBAL); + Tcl_DecrRefCount(evalObjs[0]); + Tcl_DecrRefCount(lambdaObj); + + return result; +} + /* * Local Variables: * mode: c @@ -8130,3 +8211,4 @@ TestInterpResolverCmd( * indent-tabs-mode: nil * End: */ + -- cgit v0.12 From 5f5c1bf7f1f0c4fb33e50301c87ad228cc1cc366 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Fri, 22 Jul 2022 11:10:15 +0000 Subject: Fix and test crash using apply when the passed argument does NOT have already Lambda internal representation but the body of the lambda DOES have an internal ByteCode representation. --- generic/tclProc.c | 8 ++++++++ tests/apply.test | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/generic/tclProc.c b/generic/tclProc.c index 17635e7..9677f02 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.c @@ -2457,6 +2457,14 @@ SetLambdaFromAny( argsPtr = objv[0]; bodyPtr = objv[1]; + /* + * Bugfix for testapplylambda. If we are constructing a new lambda, + * the body must be recompiled even if it is already a ByteCode object. + * Otherwise the procPtr->numCompiledLocals will not get updated causing + * a crash as local variable space is not allocated. + */ + (void) TclGetString(bodyPtr); /* Ensure string representation exists */ + TclFreeInternalRep(bodyPtr); /* * Create and initialize the Proc struct. The cmdPtr field is set to NULL diff --git a/tests/apply.test b/tests/apply.test index e2be172..32dff08 100644 --- a/tests/apply.test +++ b/tests/apply.test @@ -22,6 +22,8 @@ if {[info commands ::apply] eq {}} { } testConstraint memory [llength [info commands memory]] +testConstraint applylambda [llength [info commands testapplylambda]] + # Tests for wrong number of arguments @@ -306,6 +308,13 @@ test apply-9.3 {leaking internal rep} -setup { unset -nocomplain end i x tmp leakedBytes } -result 0 +# Tests for specific bugs +test apply-10.1 {Test for precompiled bytecode body} -constraints { + applylambda +} -body { + testapplylambda +} -result 42 + # Tests for the avoidance of recompilation # cleanup -- cgit v0.12 From 856edbc25be9401b87ea00c346b7bea728e8c0dd Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 25 Jul 2022 21:12:10 +0000 Subject: Make testapplylambda work on Windows with gcc too --- tests/apply.test | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/apply.test b/tests/apply.test index 32dff08..a5f1f8f 100644 --- a/tests/apply.test +++ b/tests/apply.test @@ -16,6 +16,8 @@ if {"::tcltest" ni [namespace children]} { package require tcltest 2.5 namespace import -force ::tcltest::* } +::tcltest::loadTestedCommands +catch [list package require -exact tcl::test [info patchlevel]] if {[info commands ::apply] eq {}} { return -- cgit v0.12 From a66ed8e23823b7ddefdb8867d3f4d5dc5dfd44d6 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 27 Jul 2022 15:56:13 +0000 Subject: fixes [4eb3a155ac] and similar segfaults: reset corresponding bodyPtr->procPtr if procPtr gets released in TclProcCleanupProc, also recompile body if its procPtr is not the same as supplied. --- generic/tclProc.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/generic/tclProc.c b/generic/tclProc.c index 59153b8..7550bfa 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.c @@ -1576,13 +1576,16 @@ TclPushProcCallFrame( * is up-to-date), the namespace must match (so variable handling * is right) and the resolverEpoch must match (so that new shadowed * commands and/or resolver changes are considered). + * Ensure the ByteCode's procPtr is the same (or it's precompiled). */ codePtr = procPtr->bodyPtr->internalRep.twoPtrValue.ptr1; if (((Interp *) *codePtr->interpHandle != iPtr) || (codePtr->compileEpoch != iPtr->compileEpoch) || (codePtr->nsPtr != nsPtr) - || (codePtr->nsEpoch != nsPtr->resolverEpoch)) { + || (codePtr->nsEpoch != nsPtr->resolverEpoch) + || ((codePtr->procPtr != procPtr) && procPtr->bodyPtr->bytes) + ) { goto doCompilation; } } else { @@ -1920,6 +1923,7 @@ TclProcCompileProc( * procPtr->numCompiledLocals if new local variables are found while * compiling. * + * Ensure the ByteCode's procPtr is the same (or it is pure precompiled). * Precompiled procedure bodies, however, are immutable and therefore they * are not recompiled, even if things have changed. */ @@ -1928,7 +1932,9 @@ TclProcCompileProc( if (((Interp *) *codePtr->interpHandle == iPtr) && (codePtr->compileEpoch == iPtr->compileEpoch) && (codePtr->nsPtr == nsPtr) - && (codePtr->nsEpoch == nsPtr->resolverEpoch)) { + && (codePtr->nsEpoch == nsPtr->resolverEpoch) + && ((codePtr->procPtr == procPtr) || !bodyPtr->bytes) + ) { return TCL_OK; } @@ -2139,6 +2145,13 @@ TclProcCleanupProc( Interp *iPtr = procPtr->iPtr; if (bodyPtr != NULL) { + /* procPtr is stored in body's ByteCode, so ensure to reset it. */ + if (bodyPtr->typePtr == &tclByteCodeType) { + ByteCode *codePtr = bodyPtr->internalRep.twoPtrValue.ptr1; + if (codePtr->procPtr == procPtr) { + codePtr->procPtr = NULL; + } + } Tcl_DecrRefCount(bodyPtr); } for (localPtr = procPtr->firstLocalPtr; localPtr != NULL; ) { -- cgit v0.12 From 2da979cd8e0bfb96bc57578f2ff2413efd393e5c Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 29 Jul 2022 10:55:37 +0000 Subject: amend for [4eb3a155ac] SF-fix: 8.7 specific implementation and warnings silencing --- generic/tclProc.c | 10 +++++----- generic/tclTest.c | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/generic/tclProc.c b/generic/tclProc.c index 311ea4e..e6b1372 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.c @@ -2162,11 +2162,11 @@ TclProcCleanupProc( if (bodyPtr != NULL) { /* procPtr is stored in body's ByteCode, so ensure to reset it. */ - if (bodyPtr->typePtr == &tclByteCodeType) { - ByteCode *codePtr = bodyPtr->internalRep.twoPtrValue.ptr1; - if (codePtr->procPtr == procPtr) { - codePtr->procPtr = NULL; - } + ByteCode *codePtr; + + ByteCodeGetInternalRep(bodyPtr, &tclByteCodeType, codePtr); + if (codePtr != NULL && codePtr->procPtr == procPtr) { + codePtr->procPtr = NULL; } Tcl_DecrRefCount(bodyPtr); } diff --git a/generic/tclTest.c b/generic/tclTest.c index 77540e2..ed5f34b 100644 --- a/generic/tclTest.c +++ b/generic/tclTest.c @@ -8145,10 +8145,10 @@ TestInterpResolverCmd( *------------------------------------------------------------------------ */ int TestApplyLambdaObjCmd ( - ClientData notUsed, + TCL_UNUSED(void*), Tcl_Interp *interp, /* Current interpreter. */ - int objc, /* Number of arguments. */ - Tcl_Obj *const objv[]) /* Argument objects. */ + TCL_UNUSED(int), /* objc. */ + TCL_UNUSED(Tcl_Obj *const *)) /* objv. */ { Tcl_Obj *lambdaObjs[2]; Tcl_Obj *evalObjs[2]; -- cgit v0.12