Menu
▾
▴
[Tcl-bugs] [ tcl-Bugs-3030870 ] make itcl 3.x built with pre-8.6 work in 8.6
|
From: SourceForge.net <no...@so...> - 2010-07-19 12:04:47
|
Bugs item #3030870, was opened at 2010-07-17 09:30 Message generated for change (Comment added) made by nijtmans You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3030870&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: None Group: current: 8.5.8 Status: Open Resolution: Fixed Priority: 5 Private: No Submitted By: Jan Nijtmans (nijtmans) Assigned to: Jeffrey Hobbs (hobbs) Summary: make itcl 3.x built with pre-8.6 work in 8.6 Initial Comment: Of course, this could be fixed the same way as [incr Tcl Bug #1725219] <https://sourceforge.net/tracker/index.php?func=detail&aid=1725219&group_id=13244&atid=313244> But, much easier and much less hacky is to just add dummy unused fields to the Tcl_CallFrame and CallFrame structures. Here is the patch, for Tcl 8.5 and 8.4. For Tcl 8.6 nothing changes. But when compiling Itcl 3.x, just some extra unused stack space is reserved, so whenever the same Itcl built is run under Tcl 8.6 this space can be used at will. ---------------------------------------------------------------------- >Comment By: Jan Nijtmans (nijtmans) Date: 2010-07-19 14:04 Message: >Tcl never allocates a (Tcl_)?CallFrame I should have added there: "on the stack" Tcl allocates CallFrames in two places (tclBasic.c:610 and tclNamesp.c:413), that would be the only places in the binaries that are affected by this patch. But even in those 2 places, Tcl only allocates those in order to use the Tcl_(Push|Pop)CallFrame API the same way as extensions do. So, still, all arguments hold. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-07-19 08:29 Message: >I'm sorry to say that, but this is simply not true. Itcl >compiled against 8.5.9 just runs fine when running >in 8.5.x (x<9). Let me add why, and I'l rest my case. Tcl never allocates a (Tcl_)?CallFrame, it's the extension that is supposed to do that. This means that this patch does not even change a single byte inside Tcl binaries at all! Tcl binaries with or without this patch are binary equal, so sure they are compatible. The ONLY effect this patch has, is that Itcl allocates a little more space for the Tcl_CallFrame that is given to Tcl. The change to tclInt.h was not even necessary, although sure it will raise questions when Tcl_CallFrame and CallFrame are not equal in size any more. I rest my case. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-07-18 23:36 Message: > This means that binaries built using the 8.5.9 tcl.h > header file will allocate larger Tcl_CallFrame structs. > This means those binaries will not function when [load]ed > or otherwise matched up at runtime with a Tcl library > from release 8.5.x where x < 9. I'm sorry to say that, but this is simply not true. Itcl compiled against 8.5.9 just runs fine when running in 8.5.x (x<9). > if Itcl > hasn't bothered to stay compatible with Tcl 8.5 -- now > several years old -- that's a problem for Itcl to solve. That's exactly what [incr Tcl Bug #1725219] is about, which was handled by jeff, so I would like to hear his view before going on. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2010-07-18 19:37 Message: In addition, it's of quite questionable value to add aids to assist the migration of Itcl 3.* from Tcl 8.5 to Tcl 8.6 since Tcl 8.6 is going to come with Itcl 4. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2010-07-18 19:31 Message: On a less technical, more political note, if Itcl hasn't bothered to stay compatible with Tcl 8.5 -- now several years old -- that's a problem for Itcl to solve. Not something for Tcl to twist itself into accomodating. We spent too many years with crazy Aunt Millie in init.tcl from pursuing this reasoning in the past. Enough. If Itcl is broken, then fix it, or discard it. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2010-07-18 19:29 Message: Your commit to core-8-5-branch adds a field "dummy13" to the end of the public definition of the Tcl_CallFrame struct. This means that binaries built using the 8.5.9 tcl.h header file will allocate larger Tcl_CallFrame structs. This means those binaries will not function when [load]ed or otherwise matched up at runtime with a Tcl library from release 8.5.x where x < 9. This kind of incompatibility is one we have customarily reserved for new minor releases, not patchlevels. At a minimum, reacting to such a policy change means that extensions have to rethink their Tcl_InitStubs() and [package require Tcl] habits so that they begin checking for compatible Tcl libraries all the way to the patchlevel detail. Maybe a case could be made that that's a desirable way to go, but it should be something proposed and consideredand discussed, with some feedback from some actual extension authors, not something just dropped in over the weekend. I say revert. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-07-18 07:42 Message: See the following comment in tcl.h: ========================================================================== A call frame is initialized and pushed using Tcl_PushCallFrame and popped using Tcl_PopCallFrame. Storage for a Tcl_CallFrame must be provided by the Tcl_PushCallFrame caller, and callers typically allocate them on the C call stack for efficiency. For this reason, Tcl_CallFrame is defined as a structure and not as an opaque token. However, most Tcl_CallFrame fields are hidden since applications should not access them directly; others are declared as "dummyX". ========================================================================== Example extension code: Tcl_CallFrame frame; int somevar; Tcl_PushCallFrame(interp, &frame, ....); .... Tcl_PopCallFrame(interp); The functions Tcl_PushCallFrame and Tcl_PopCallFrame are functions that look like public, but in fact they are in the private stub table. The Tcl_CallFrame structure looks like a public structure, but in fact is was never documented. As far as I know, the only extension using it is Itcl. So, is it really public?.... Just because Tcl 8.5 added two fields to the Tcl_CallFrame structure this caused [incr Tcl Bug #1725219]. Tcl 8.6 did it again with one additional field. So, yes, you are right that "Adjusting the size of any public structure is a recipe for problems". That's exactly what caused [incr Tcl Bug #1725219]. The problem is caused by overwriting variables that come immediately afther the Tcl_CallFrame structure on the stack, that that's exactly what this patch is not doing. So, what happens if the above example code is compiled against Tcl 8.4.19? The "frame" variable is allocated on the stack, and is used to hold the old callframe between the Tcl_PushCallFrame and Tcl_PopCallFrame calls. Now we are running it in Tcl 8.5 or Tcl 8.6. The Tcl_CallFrame structure got some extra fields, so the result is that the "frame" variable is not big enough any more to hold the information stored by Tcl. The effect will be that the "somevar" variable is overwritten. So, now we run it in Tcl 8.4.20 (with this patch). Yes, the Tcl_CallFrame got some extra fields. But Tcl never reads or writes those dummy fields, so the "somevar" variable will never be overwritten. Now what happens if we compile the above sample code using Tcl 8.4.20 (with this patch). The effect is that the "frame" variable gets some extra space, and the the "somevar" variable is moved farther away on the stack. When running it on Tcl 8.4.19, this extra space is not used. But when running it on Tcl 8.5 or Tcl 8.6, this space will be used by the Tcl_PushCallFrame function! But this patch prevents that the "somevar" is overwritten, so the bug is solved. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-07-18 01:00 Message: This is fully compatible, because the added fields are unused! The only effect is that in newly compiled code, a little more space is allocated than necessary. But this space is necessary when running under Tcl 8.6. It's exactly the same as (but better than) the hack used in Itcl 3.4, only it can be used by all extensions using (Tcl_)?CallFrame. Please provide an example where this causes a problem, before suggesting that I broke any compatibility. There isn't! ---------------------------------------------------------------------- Comment By: Jeffrey Hobbs (hobbs) Date: 2010-07-17 23:21 Message: This seems like a misguided patch. Adjusting the size of any public structure is a recipe for problems. If this has been used in existing code, you may well have just broken compatibility for 8.4 and 8.5. This doesn't seem like a safe enough approach. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2010-07-17 09:44 Message: fixed in Tcl 8.5 and 8.4 branch ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3030870&group_id=10894 |