#5106 Tcl8.5 + incrTcl3.4 [variable] crash

obsolete: 8.5.12
closed-fixed
miguel sofer
8
2012-09-11
2012-09-04
Brian Griffin
No

I've recently tracked down a crash that was due to faulty coding in an incrTcl methods, however, I don't know if this is a known "problem" with incrTcl, or a bug in the 8.5.8+34 combination, or just the way things are. I'm also wondering if there shouldn't be a runtime check for this condition.

So what is the problem?

itcl::class Dog {
variable bite
method bark {}
}

itcl::body Dog::bark {} {
variable bite
set bite false
puts "Bark!"
}

The crash is due to the unnecessary "variable" statement in the itcl::body. It ends up corrupting the heap. (Thanks for TCL_MEM_DEBUG!)

$ tclsh8.5
% package require Itcl
3.4
% memory validate on
% itcl::class Dog {
variable bite
method bark {}
}
% itcl::body Dog::bark {} {
variable bite
set bite false
puts "Bark!"
}
% Dog fido
fido
% fido bark
hi guard byte 0 is 0x62 b
total mallocs 114496
total frees 103026
current packets allocated 11470
current bytes allocated 482362
maximum packets allocated 15640
maximum bytes allocated 630832
high guard failed at 81ba7f8, .../tcl/unix/../generic/tclHash.c 344
8 bytes allocated at (.../incrTcl/itcl/generic/itcl_migrate.c 130)
Memory validation failure
Abort(coredump)

Discussion

1 2 > >> (Page 1 of 2)
  • I could not reproduce it with 8.5.11 and itcl-3-branch on windows. How often is it reproducible?

     
  • Don Porter
    Don Porter
    2012-09-04

    The failure has been present since the Tcl checkin
    http://core.tcl.tk/tcl/ci/c49c117219
    which is "modifs to help itcl adapt to VarReform"

    For many Tcl checkins prior to that, current Itcl
    source either cannot build or cannot [load].

    Appears that this bug is an indication that some
    details of adjusting to Tcl "VarReform" just never
    got done right.

     
  • Don Porter
    Don Porter
    2012-09-04

    Rolling back Itcl to the same era,
    http://core.tcl.tk/itcl/info/3f5de3c16d
    the same problem is present. This is
    not a recent Itcl regression.

     
  • Don Porter
    Don Porter
    2012-09-04

    On the bright side, whatever the problem is
    has apparently been resolved in Itcl 4.0b8.
    No troubles there.

     
  • miguel sofer
    miguel sofer
    2012-09-04

    Don: could you please check one rev back for itcl, ie [8d02855363]? That one should compile ok against 8.5 headers. I'm trying to check if it isn't something that I introduced when doing the unholy adaptation for compiling on 8.4 headers and running properly on 8.4 and 8.5

     
  • Don Porter
    Don Porter
    2012-09-05

    • priority: 5 --> 9
     
  • Don Porter
    Don Porter
    2012-09-05

    The memory corruption happens in the execution of
    the INST_VARIABLE instruction in tclExecute.c

    The "call" TclSetVarNamespaceVar(otherPtr) corrupts
    the memory. This is really a macro, and the expansion
    includes the line:

    ((VarInHash *)(otherPtr))->refCount++;

    This is actually writing one byte past allocated memory,
    so this tells me otherPtr points to a struct that is actually
    smaller in size than a VarInHash struct. I think this must
    be some Itcl declared struct that the Itcl custom resolvers
    allocate.

    I hope that's enough information for msofer to understand
    the problem at a more conceptual level and recommend a fix.

     
  • Don Porter
    Don Porter
    2012-09-05

    Looks like the root of the problem is casting a pointer to
    a Var struct to the type of VarInHash struct, and then using
    it to get at extension fields that are not there. As a guess,
    we're simply not supposed to reach this code in this scenario?
    Some guard somewhere is failing in its duty?

     
  • IIRC, the 8.5 series assumes in a few key spots that all variables are either a VarInHash or a member of the LVT. In particular, it assumes that TclObjLookupVarEx(…,TCL_NAMESPACE_ONLY,…) always returns a VarInHash (or NULL, of course). It sounds like Itcl is generating them in another way. I guess we ought to check if the VAR_IN_HASHTABLE flag is set on the otherPtr (i.e., use TclIsVarInHash(otherPtr) as the guard).

    One question though: should that be combined into the guard within TclSetVarNamespaceVar? Or should it be exposed in the INST_VARIABLE code? If the latter, I think TclVariableObjCmd has the identical problem. (It should be testable by invoking [variable bite] like [set v variable;$v bite], so as to defeat compilation.)

     
  • There's also the possibility that Itcl 3 might have the wrong flags set (i.e., setting the VAR_IN_HASHTABLE flag when it was never allocated as a VarInHash). If that's the case, it's got to be slated as an Itcl bug for the sake of our sanity.

     
1 2 > >> (Page 1 of 2)