[Tcl-bugs] [ tcl-Bugs-2152286 ] TCL_RETURN causes TclUpdateReturnInfo() to panic

[SourceForge.net <no...@so...>]

Bugs item #2152286, was opened at 2008-10-07 18:39
Message generated for change (Settings changed) made by dgp
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2152286&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 45. Parsing and Eval
Group: obsolete: 8.5.4
>Status: Pending
>Resolution: Invalid
Priority: 7
Private: No
Submitted By: Fadi Adib (fadib)
Assigned to: Don Porter (dgp)
Summary: TCL_RETURN causes TclUpdateReturnInfo() to panic

Initial Comment:
Please see the attached testcase (tcltest.tar.gz)

Problem Analysis:
----------------
The problem happens when a C function calls a TCL proc that calls a C function that returns TCL_RETURN, this causes returnLevel to be decremented all the way to 0 causing TclUpdateReturnInfo() to panic.

Testcase:
--------
 + (tcltest.c): main() calls CProcCmd() function which is a C function with a TCL wrapper.
 + CProcCmd() calls tclproc (tcltest.tcl) which calls CProcCmd() again.
 + CProcCmd() returns TCL_RETURN which makes TCL panic.

Workarounds:
-----------
There are two workarounds that I can use to make the testcase work (uncomment either line 25 or 28 in tcltest.c):

+ Workaround 1 (line 25): 
Explicitly increment returnLevel before returning TCL_RETURN.

+ Workaround 2 (line 28): 
Call a dummy tcl proc before returning TCL_RETURN. This works because it forces the returllevel to be incremented once and thus stops it from ever reaching 0.



----------------------------------------------------------------------

>Comment By: Don Porter (dgp)
Date: 2008-10-19 14:52

Message:

The error here is in the evaluation
of [cproc 1].  With a single argument
the branch is taken that evaluates
[tclproc].  If you examine the return
code from that Tcl_Eval(), you will
find that it is TCL_OK.  The command
[tclproc] has run successfully.

You then turn around and originate a
TCL_RETURN return code, without
dealing with the fact that an
intervening Tcl_Eval() has taken
the interp out of the reset state
it had when it entered CProcCmd.

There are a number of ways to
fix the code, depending on just
what it is this code is intended
to do.  That is not clear at all
from the example, which feels 
contrived to demo the conditions
for panic, more than directed
toward any rational purpose.  That's
fine; that's often true of bug
demos, but leaves me without any
context to choose what alternative
might make sense.

If you have a Tcl_ObjCmdProc that
will return TCL_RETURN, you should
either do that with the interp
still in its original reset state,
or do that after something you called
also returned TCL_RETURN (the "pass
along the exception" case), or take
care to call Tcl_SetReturnOptions()
to set the relevant options as needed
for your command (the "I'm originating
TCL_RETURN for my own purposes" case).

It seems to me the panic is serving
exactly the purpose intended, to call
attention to C code that's not robust
or safe so it can get corrected.

Note that your workarounds are just
tricky indirect ways to take my advice.
Explicit returnLevel increment is just
a sneaky "probe the internals" way to
accomplish what a proper Tcl_SetReturnOptions()
call would do.  The dummy proc call is just
a very indirect way to call the
Tcl_ResetResult() you need to get the
interp back to an initial state where
simple return of TCL_RETURN will cause
no trouble.

Hope that helps.  I'll keep this report
pending in case you want to comment
further or seek advice.


----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2008-10-16 17:51

Message:

A quick attempt to patch up that
flaw does not resolve the issue, 
so I'll examine further.  In the meantime,
do not confuse the coding of 
procs, which push CallFrames, and
non-proc commands, which do not.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2008-10-16 17:48

Message:

The flaw here is in the CProcCmd()
routine.  Several places in that routine
Tcl_Eval is called on some script, but
the return code is not checked.  If you
were to check it, you would find the
value to be TCL_RETURN, which is typically
an indication that you should not just
continue normally.  Your code continues
normally.

----------------------------------------------------------------------

Comment By: Fadi Adib (fadib)
Date: 2008-10-07 18:50

Message:
Please use the .zip file

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2008-10-07 18:48

Message:
That file seems to be corrupt:

mig@oli:/tmp$ tar -ztf /tmp/tcltest.tar.gz 
tar: This does not look like a tar archive
tar: Skipping to next header
tar: Error exit delayed from previous errors


----------------------------------------------------------------------

Comment By: Fadi Adib (fadib)
Date: 2008-10-07 18:47

Message:
File Added: tcltest.zip

----------------------------------------------------------------------

Comment By: Fadi Adib (fadib)
Date: 2008-10-07 18:43

Message:
File Added: tcltest.tar.gz

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2152286&group_id=10894