fixes [e533296a92d89044]: reset cmd in procPtr after lambda invocation; avoid reset of lambda info in frames stack by nested calls (till it is still evaluated) - fill this info and reset the artificial command only once (at parent invocation); new tests added

generic/tclProc.c

@@ -15,16 +15,7 @@
 
 #include "tclInt.h"
 #include "tclCompile.h"
-
-/*
- * Variables that are part of the [apply] command implementation and which
- * have to be passed to the other side of the NRE call.
- */
-
-typedef struct {
-    Command cmd;
-    ExtraFrameInfo efi;
-} ApplyExtraData;
+#include <assert.h>
 
 /*
  * Prototypes for static functions in this file
@@ -2631,10 +2622,8 @@ TclNRApplyObjCmd(
 {
     Interp *iPtr = (Interp *) interp;
     Proc *procPtr = NULL;
-    Tcl_Obj *lambdaPtr, *nsObjPtr;
+    Tcl_Obj *lambdaPtr;
     int result;
-    Tcl_Namespace *nsPtr;
-    ApplyExtraData *extraPtr;
 
     if (objc < 2) {
 	Tcl_WrongNumArgs(interp, 1, objv, "lambdaExpr ?arg ...?");
@@ -2659,41 +2648,55 @@ TclNRApplyObjCmd(
 	procPtr = lambdaPtr->internalRep.twoPtrValue.ptr1;
     }
 
-    /*
-     * Find the namespace where this lambda should run, and push a call frame
-     * for that namespace. Note that TclObjInterpProc() will pop it.
-     */
-
-    nsObjPtr = lambdaPtr->internalRep.twoPtrValue.ptr2;
-    result = TclGetNamespaceFromObj(interp, nsObjPtr, &nsPtr);
-    if (result != TCL_OK) {
-	return TCL_ERROR;
-    }
-
-    extraPtr = (ApplyExtraData *)TclStackAlloc(interp, sizeof(ApplyExtraData));
-    memset(&extraPtr->cmd, 0, sizeof(Command));
-    procPtr->cmdPtr = &extraPtr->cmd;
-    extraPtr->cmd.nsPtr = (Namespace *) nsPtr;
-
     /*
      * TIP#280 (semi-)HACK!
      *
-     * Using cmd.clientData to tell [info frame] how to render the lambdaPtr.
-     * The InfoFrameCmd will detect this case by testing cmd.hPtr for NULL.
-     * This condition holds here because of the memset() above, and nowhere
-     * else (in the core). Regular commands always have a valid hPtr, and
-     * lambda's never.
+     * Create procPtr->cmdPtr once. This would avoid override and usage
+     * after free of procPtr->cmdPtr by nested invocations of this lambda.
      */
+    if (!procPtr->cmdPtr) {
+	Tcl_Obj *nsObjPtr;
+	Tcl_Namespace *nsPtr;
+	Command * cmdPtr;
+	ExtraFrameInfo * efiPtr;
 
-    extraPtr->efi.length = 1;
-    extraPtr->efi.fields[0].name = "lambda";
-    extraPtr->efi.fields[0].proc = NULL;
-    extraPtr->efi.fields[0].clientData = lambdaPtr;
-    extraPtr->cmd.clientData = &extraPtr->efi;
+	/* Find the namespace where this lambda should run. */
+	nsObjPtr = lambdaPtr->internalRep.twoPtrValue.ptr2;
+	result = TclGetNamespaceFromObj(interp, nsObjPtr, &nsPtr);
+	if (result != TCL_OK) {
+	    return TCL_ERROR;
+	}
 
+	cmdPtr = (Command *)TclStackAlloc(interp,
+			sizeof(Command) + sizeof(ExtraFrameInfo));
+	efiPtr = (ExtraFrameInfo *)(cmdPtr + 1);
+	memset(cmdPtr, 0, sizeof(*cmdPtr));
+	cmdPtr->nsPtr = (Namespace *) nsPtr;
+
+	/*
+	 * Using cmdPtr->clientData to tell [info frame] how to render the lambda.
+	 * The InfoFrameCmd will detect this case by testing cmdPtr->hPtr for NULL.
+	 * This condition holds here because of the memset() above, and nowhere
+	 * else (in the core). Regular commands always have a valid hPtr, and
+	 * lambda's never.
+	*/
+
+	efiPtr->length = 1;
+	efiPtr->fields[0].name = "lambda";
+	efiPtr->fields[0].proc = NULL;
+	efiPtr->fields[0].clientData = lambdaPtr;
+	cmdPtr->clientData = efiPtr;
+
+	procPtr->cmdPtr = cmdPtr;
+	TclNRAddCallback(interp, ApplyNR2, cmdPtr, procPtr, NULL, NULL);
+    }
+
+    /*
+     * Push a call frame for that namespace. Note that TclNRInterpProcCore()
+     * will pop it (NRE-based).
+     */
     result = TclPushProcCallFrame(procPtr, interp, objc, objv, 1);
     if (result == TCL_OK) {
-	TclNRAddCallback(interp, ApplyNR2, extraPtr, NULL, NULL, NULL);
 	result = TclNRInterpProcCore(interp, objv[1], 2, &MakeLambdaError);
     }
     return result;
@@ -2705,9 +2708,12 @@ ApplyNR2(
     Tcl_Interp *interp,
     int result)
 {
-    ApplyExtraData *extraPtr = (ApplyExtraData *)data[0];
+    Proc *procPtr = (Proc *) data[1];
 
-    TclStackFree(interp, extraPtr);
+    assert(procPtr->cmdPtr == data[0]);
+    /* clear cmd of lambdaProc (released here)... */
+    procPtr->cmdPtr = NULL;
+    TclStackFree(interp, data[0]);
     return result;
 }
 

tests/apply.test

@@ -84,6 +84,19 @@ test apply-3.2 {non-existing namespace} -body {
     namespace delete ::NONEXIST
     apply $lambda x
 } -returnCodes error -result {namespace "::NONEXIST::FOR::SURE" not found}
+test apply-3.2.2 {non-existing namespace (recreated between apply-calls)} -body {
+    set res {}
+    namespace eval ::NONEXIST::FOR::SURE {}
+    set lambda {{step} {variable x; lappend x $step} ::NONEXIST::FOR::SURE}
+    lappend res [catch { apply $lambda A} x] $x
+    namespace delete ::NONEXIST
+    lappend res [catch { apply $lambda B} x] $x
+    namespace eval ::NONEXIST::FOR::SURE {}
+    lappend res [catch { apply $lambda C} x] $x
+    set res
+} -cleanup {
+    namespace delete ::NONEXIST
+} -result {0 A 1 {namespace "::NONEXIST::FOR::SURE" not found} 0 C}
 test apply-3.3 {non-existing namespace} -body {
     apply [list x {set x 1} NONEXIST::FOR::SURE] x
 } -returnCodes error -result {namespace "::NONEXIST::FOR::SURE" not found}

tests/info.test

@@ -2437,6 +2437,19 @@ test info-39.2 {Bug 4b61afd660} -setup {
     rename probe {}
 } -result 3
 
+test info-39.3 {Bug [e533296a92d89044]: no segfault, nested lambda invocation doesn't remove info from stack} -setup {
+    set ::res {}
+    proc probe {s} {
+	lappend ::res [dict filter [info frame -1] key {[cl]*m*d*}]
+	catch {apply {{s} {eval $s}} $s}
+    }
+} -body {
+    probe {catch {probe {error X}}; probe {}}
+    set ::res
+} -cleanup {
+    unset -nocomplain ::res
+} -result {{cmd {probe {catch {probe {error X}}; probe {}}}} {cmd {probe {error X}} lambda {{s} {eval $s}}} {cmd {probe {}} lambda {{s} {eval $s}}}}
+
 test info-41.0 {Bug 0de6c1d79c crash} -setup {
     interp create child
     child hide info