Tcl Source Code

As reported on clt (http://groups.google.com/group/comp.lang.tcl/browse_frm/thread/32fa115f8aef9d1f/51fac01c35c3b340#51fac01c35c3b340)

mig@uh:~$ /home/CVS/tcl_SF_clean/unix/tclsh
% set foo {a {b {c {d {e 1}}}}} 
a {b {c {d {e 1}}}}
% dict update foo a t {
    dict update t b t {
     dict update t c t {
      dict update t d t {
       dict incr t e
      }
     }
    }
   } 
e 2
% set foo
Segmentation fault

Logged In: YES 
user_id=1312539
Originator: NO

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

File Added - 245160: test

Logged In: YES 
user_id=148712
Originator: YES

Could not produce a test case that causes a crash. Manual verification of the original problem (wrong stack depth on TCL_OK returns from the body) can be done with the enclosed test in a --enable-symbols=all build. 

Fixed in HEAD, leaving open at reduced prio in case someone comes up with a workable test.
File Added: test

Logged In: YES 
user_id=79902
Originator: NO

Can't argue with patch (other than with trifling stuff that's easier to fix after it is committed). Go for it.

File Added - 245155: 1786481-2.patch

Logged In: YES 
user_id=148712
Originator: YES

New patch, same logic. Fixed panic message, reordered to allow peephole optimisation in the normal case.
File Added: 1786481-2.patch

File Added - 245085: 1786481.patch

Logged In: YES 
user_id=148712
Originator: YES

This patch fixes the issue, please review. Can dict-for use something similar?

Changes in the dict-update compiler: INST_UPDATE_START now leaves the key list on the stack instead of creating a tmp var for it; INST_UPDATE_END expects to find at stacktop. The epilogue after the implicit catch for the body is reformulated, with a clearer code separation for different return codes from the body (caught vs not).

The previous code left a duplicate result on the stack after TCL_OK completion?


File Added: 1786481.patch

Logged In: YES 
user_id=148712
Originator: YES

The numArgs is incorrect. The variable 'cleanup' only has to be set before jumping to procesExceptionReturn, so the 'cleanup=2' is wrong but harmless.

HEAD now has these fixes, but still has the stack error.

Logged In: YES 
user_id=79902
Originator: NO

That's wrong. The dictUpdateEnd entry definitely should have 2 args!

It's also possible that there's a separate bug in the INST_DICT_UPDATE_END implementation. In particular, I suspect that the 'cleanup = 2;' in the last clause of it that can goto checkForCatch is wrong.

Logged In: YES 
user_id=99768
Originator: NO

I tried to chase this a little bit further and observe that line 213 of tclCompile.c has:

    {"dictUpdateEnd",  9,    -1,   2,{OPERAND_LVT4, OPERAND_AUX4}},
/* Reflect the state of local variables (described in the aux data
 * referred to by the second immediate argument) back to the state of
 * the dictionary in the variable referred to by the first immediate
 * argument. The list of keys (popped from the stack) must be the same
 * length as the list of variables.
 * Stack:  ... keyList => ... */

This caused an immediate segfault when I tried the test case with tcl_traceCompile=2
because the numOperands is 1 and almost surely should be 2.

How sure are we about stack balance?

Logged In: YES 
user_id=80530
Originator: NO


kbk... but I observe that the definition of DICT_UPDATE_END (tclCompile.c:361) has the wrong value for numOperands...
kbk... and suspect without proof that these instructions maya also have the wrong value for stackBalance or some such.

Logged In: YES 
user_id=80530
Originator: NO


In a --enable-symbols=all build:

---- dict-21.17 start
Test file error:
Bad stack top 6 at pc 201 in TclExecuteByteCode (min 0, max 5)
 executing dict update t c t {
                    dict update t d t {
                        dict incr t e
                    }
                }
TclExecuteByteCode execution failure: bad stack top

Logged In: YES 
user_id=79902
Originator: NO

Fixed with a little more duplication in the evil case.

Logged In: YES 
user_id=79902
Originator: NO

Seems to build a recursive structure?!