[Tcl-bugs] [ tcl-Bugs-3564735 ] Tcl8.5 + incrTcl3.4 [variable] crash

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Bugs item #3564735, was opened at 2012-09-04 08:38
Message generated for change (Comment added) made by dgp
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3564735&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: 21. [namespace]
Group: current: 8.5.12
Status: Open
Resolution: None
Priority: 9
Private: No
Submitted By: Brian Griffin (bgriffin)
Assigned to: miguel sofer (msofer)
Summary: Tcl8.5 + incrTcl3.4 [variable] crash

Initial Comment:
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)

----------------------------------------------------------------------

>Comment By: Don Porter (dgp)
Date: 2012-09-06 12:03

Message:
Branch bug-3564735 now contains a patch
that armors Tcl against memory corruption in
this scenario.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-06 09:08

Message:

Here's a demonstration of a related flaw:

package require Itcl
itcl::class Dog {
method bark {}
}
itcl::body Dog::bark {} {
variable bite
}
Dog fido
fido bark

can't upvar from variable to itself
    while executing
"variable bite"
    (object "::fido" method "::Dog::bark" body line 2)
    invoked from within
"fido bark"

These two together indicate some ways in which
the Itcl custom resolvers aren't quite meeting core
expectations.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-06 06:53

Message:
Tcl_VariableObjCmd() suffers the same problem.

The (Var *) returned by TclObjLookupVarEx has
a flags field of 0.

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2012-09-05 16:22

Message:
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.

----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2012-09-05 16:19

Message:
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.)

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-05 12:45

Message:
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?

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-05 12:39

Message:
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.

----------------------------------------------------------------------

Comment By: miguel sofer (msofer)
Date: 2012-09-04 12:50

Message:
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

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-04 10:21

Message:

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

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-04 10:11

Message:
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.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2012-09-04 10:07

Message:

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.

----------------------------------------------------------------------

Comment By: Serg G. Brester (sebres)
Date: 2012-09-04 09:13

Message:
I could not reproduce it with 8.5.11 and itcl-3-branch on windows. How
often is it reproducible?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3564735&group_id=10894

[Tcl-bugs] [ tcl-Bugs-3564735 ] Tcl8.5 + incrTcl3.4 [variable] crash

The Tool Command Language implementation

[Tcl-bugs] [ tcl-Bugs-3564735 ] Tcl8.5 + incrTcl3.4 [variable] crash