From: SourceForge.net <no...@so...> - 2007-09-26 02:20:10
|
Bugs item #1786481, was opened at 2007-09-01 17:23 Message generated for change (Comment added) made by sf-robot You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1786481&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: 15. Dict Object Group: development: 8.5a7 >Status: Closed Resolution: Fixed Priority: 4 Private: No Submitted By: miguel sofer (msofer) Assigned to: Donal K. Fellows (dkf) Summary: dict causes segfault Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: SourceForge Robot (sf-robot) Date: 2007-09-25 19:20 Message: 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). ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2007-09-11 07:55 Message: 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 ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2007-09-11 07:46 Message: 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. ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2007-09-11 07:19 Message: 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 ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2007-09-10 20:39 Message: 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 ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2007-09-10 14:48 Message: 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. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2007-09-10 13:29 Message: 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. ---------------------------------------------------------------------- Comment By: Kevin B KENNY (kennykb) Date: 2007-09-10 09:43 Message: 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? ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2007-09-10 09:41 Message: 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. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2007-09-10 09:31 Message: 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 ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2007-09-08 15:38 Message: Logged In: YES user_id=79902 Originator: NO Fixed with a little more duplication in the evil case. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2007-09-08 15:05 Message: Logged In: YES user_id=79902 Originator: NO Seems to build a recursive structure?! ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1786481&group_id=10894 |