Menu
▾
▴
[Tcl-bugs] [ tcl-Bugs-3588687 ] Tcl_InitStubs() Broken As Designed
|
From: SourceForge.net <no...@so...> - 2012-11-28 00:52:48
|
Bugs item #3588687, was opened at 2012-11-19 22:41 Message generated for change (Comment added) made by jenglish You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3588687&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: 39. Package Manager Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Don Porter (dgp) Assigned to: Joe English (jenglish) Summary: Tcl_InitStubs() Broken As Designed Initial Comment: A stubs-enabled extension encounters Tcl in three distinct ways. 1) It compiles against a tcl.h file from some release of Tcl. With USE_TCL_STUBS active, the particular tcl.h file determines the layout of the stubs table that the extension is compiled to assume. 2) It links against some libtclstub.a file. That libtclstub.a file in turn was compiled against some tcl.h file and from it got a value of TCL_STUBS_MAGIC compiled into it. As far as I can tell there is nothing in place to make sure that the two tcl.h files so far mentioned are the same file, or at a minimum define the same TCL_STUBS_MAGIC value. Until now it's never been an issue since we've never changed that value. 3) The extenson [load]s into some Tcl interp. The extension calls Tcl_InitStubs() passing in the interp value. T_IS pulls a table pointer out of the iPtr->stubTable field of that interp for vetting for suitability. What's important is that the table in iPtr->stubTable has a compatible layout to the one that was compiled into the extension. But the mechanism in place cannot check this. To illustrate this, consider compiling an extension containing the call Tcl_InitStubs(interp, "9.0", 0); but compiling it against a Tcl 8 header file, so all of it's other calls into the Tcl library are coded based on the assumptions of the Tcl 8 stub table layout. Next imagine linking this against libtclstubs9.a so that the TCL_STUBS_MAGIC value the Tcl_InitStubs() call will be checking will be the one for the Tcl 9 table. Then [load] in a Tcl 9 interp. All of Tcl_InitStubs() checks will pass, but we'll be left with an extension compiled to Tcl 8 layout expectations making use of a Tcl 9 stub table, and that's gonna crash hard. Now, if the extension writer would have just stuck with the recommended convention of Tcl_InitStubs(interp, TCL_VERSION, 0) he could not get in this trouble. But that is our fault for giving him that knob to tweak in the first place. Tcl_InitStubs ought not have a "version" argument for the coder to get wrong. ---------------------------------------------------------------------- >Comment By: Joe English (jenglish) Date: 2012-11-27 16:52 Message: I don't think that this part does what it claims to do: 90 /* 91 * Detect whether the extension and the stubs library were built 92 * against Tcl header files from different major versions. That's 93 * seriously broken. 94 */ If an extension was compiled against 8.5.13 headers with -DUSE_TCL_STUBS (where Tcl_InitStubs is declared as a symbol with external linkage) but mistakenly linked against novem libtclstub.a, it'll get novem's definition of Tcl_InitStubs(), which passes novem's TCL_VERSION_MAJOR to the worker routine TclInitStubs(). Suggest simply leaving Tcl_InitStubs out of novem tclStubLib.c. That way the extension will get a linker error if there is a mismatch (old headers+new libtclstub => Tcl_InitStubs not found; new headers+old libtclstub => TclInitStubs not found). The change could be backported to core-8-5-branch and core-8-6-branch, but that would just change a link-time error into a runtime one -- probably not worth doing (if there's a mismatch, it's better to detect it earlier). ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2012-11-27 13:11 Message: Branch bug-3588687 has a patch to address these problems. Would appreciate your comments if you can spare the time. I think it's not as radical a change as you might choose, but I think it fixes the worst problem identified above. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2012-11-20 12:01 Message: To further concur, the 9.0 series does not need to load any extension compiled against 8.* at all *and* the stub table can be considered to be wholly in a state of flux right now. For sure, there won't be stabilization until *at least* 9.0a1 is released, and probably not until 9.0.0 (with things slushy after 9.0b1). Now is not the time to consider the impact on extensions. Now is not the time to put in lots of compatibility goop. ---------------------------------------------------------------------- Comment By: Andreas Kupries (andreas_kupries) Date: 2012-11-20 10:07 Message: Regarding "This means that 8.x extensions cannot be used in a 9.0 interpreter." IMHO this is a good thing, and should be kept. 8 and 9 are different major versions, and we expect incompatibility at C API level, i.e. stubs tables. We expect that an extension must be re-compiled against 9.x to pick up on these changes and be loadable in 9.x. A clean break between the versions of the core. Putting code into 9.x to shoehorn an extension build against 8.x into it, that is a source of complexity in runtime checks and/or ifdeffery we can, should!, do without, at this point. We are currently in _compatibility breaking_ mode, where we are not even have fully worked out what we are breaking and how. But we know that we want to be free of constraints in our thinking, and 8.x compat work is such a constraint. IOW trying to re-create/shoehorn compatibility now is IMHO much to early in the dev cycle for 9.x. Given the moving target that is 9.x it could possibly be called a 'fool's errand' also (with apologies if that is considered insulting). ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-11-20 00:43 Message: Please consider: <http://core.tcl.tk/tcl/info/16299cb2af> I think 2 of the 3 points you raised are solved now in the "novem" branch. Feedback welcome! ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-11-20 00:10 Message: Thanks, Don, for this clear description. Regarding the second point, the TCL_STUBS_MAGIC value built-in the stubs library, that's completely right. I already did work on this in the "novem" branch which IMHO corrects that, but feedback is welcome. >What's important is that the table in iPtr->stubTable has a compatible >layout to the one that was compiled into the extension. But the >mechanism in place cannot check this Yes, that's a problem. So, let's put a mechanism in place to check this. Your last point, the version argument, the problem now is that Tcl_InitStubs(interp, "9.0", 0) returns false when called in a "8.6" interpreter. This means that 8.x extensions cannot be used in a 9.0 interpreter. I don't have an immediate solution for that. Easiest is to let extesions use Tcl_InitStubs(interp, "8.6+", 0) if they want to indicate that they can run under both 8.6 and 9.0, but that could - at best - only work on 32-bit platforms. 64-bit platforms should have a different MAGIC value, given the big reform that is being worked on now by dkf. Regards, Jan Nijtmans ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3588687&group_id=10894 |