Tcl Source Code

Consider the following:

set x -2366755200
string is integer $x
set y [expr { wide($x) }]

The value of y that results is 1928212096

The problem is that the generated code for
the expression sees the "integer" internal
representation and uses it, even though
reconverting the string rep would yield a
different value.  The particular code in
question is in the "ExprCallMathFunc"
function in tclExecute.c, but the problem
appears to be pretty pervasive.

It appears to be incorrect to promote an
"integer" to a "wide" if the "integer" has a
string representation.

IP - Comment Removed: 130.88.1.31

data_type - 210894

Logged In: YES 
user_id=79902

Oh, I see you fixed it already. :} And I see it was a simple
fix. And I see that the reason I didn't find it in testing
was endianness. Oh well.. Thanks for finding it.

Logged In: YES 
user_id=79902

And it closed another bug (I've already forgotten the
number) at the same time. Track the problem in 1027690...

Logged In: YES 
user_id=80530


Patch appears to have created
new Bug 1027690.

Logged In: YES 
user_id=79902

Patch looked just about good enough to me
Applied to HEAD and 8.4 branch

Logged In: YES 
user_id=79902

Of course that code is my code.  How do you think I know it
is a mess?  :^/

I can't check the code right now; my sandbox has other toys
in it (TIP#143 implementation as it happens) and I don't
want to get them mixed up.  If/when I get that out of the
way, I'll be able to work on this area in more detail.

Logged In: YES 
user_id=99768

Uhm, Donal, much of that appears to be *your* code. :)

Nevertheless, I took a few quick trolls through TEBC, and
didn't see any *obvious* problems - all the code that
accesses objPtr->internalRep.longValue seems to check
the type first; likewise for wideValue.

Emphasis on 'obvious' is intentional.

Logged In: YES 
user_id=79902

That patch looks comparatively likely.  What is the impact
of this sort of treatment on number handling in TEBC and its
helpers?  I know that that code is *not* very clean, so it
might get quite surprised...

File Added - 84569: 868489.patch

Logged In: YES 
user_id=99768

It occurs to me that the easiest way to mark that the
annoying overflow has occurred is simply to make it a
wide integer instead of an ordinary one.

The attached patch does the following:

Makes Tcl_GetIntFromObj and Tcl_GetLongFromObj
check for tclWideIntType as well as tclIntType, and do
direct conversion of the intenal rep from wide to int
(with a range check).  Also changes the conversion from
string to a new SetIntOrWideFromAny.

The new SetIntOrWideFromAny is just like the old
SetIntFromAny except that it checks for integer overflow
and uses a tclWideIntType instead of a tclIntType
for values that don't fit in a 'long' - avoiding inconsistent
internal reps.

The existing SetIntFromAny has its range checks tightened
to avoid setting an internal representation that is
inconsistent with the string rep (an error is thrown instead).

There are potential incompatibilities here because of the
tightened range in SetIntFromAny and the fact that
Tcl_GetIntFromObj is no longer guaranteed to leave a
specific internal representation.  It is arguable that any
code that runs afoul of either incompatibility is buggy
anyway.

New test cases provided for the obvious problem spots.

Logged In: YES 
user_id=99768

The given test case works in Tcl up until the change:
2003-04-11  Don Porter  <[email protected]>

* generic/tclCmdMZ.c (Tcl_StringObjCmd,STR_IS_INT):  Corrected
inconsistent results of [string is integer] observed on systems
where sizeof(long) != sizeof(int).  [Bug 718878]
* tests/string.test: Added tests for Bug 718878.
* doc/string.n: Clarified that [string is integer] accepts
32-bit integers.

But this change is an innocent victim; replacing [string is
integer] with [clock format] will tickle the bug in earlier
releases.

Logged In: YES 
user_id=79902

The problem is how to fix this; how to detect that we are in
an "annoying integer overflow" situation and mark the object
so that we know when converting to wide to go from the
string version and not the internal rep (which would
otherwise be far faster.)

:^/ At best, this is a huge amount of work to fix.  (New
arithmetic object types require action in ever so many
places!)  I don't think I'm likely to have the free time to
take this on in the near future, so if anyone wishes to have
a go, please feel free!

Logged In: YES 
user_id=99768

[string is integer] was just an example.  The code that first
tripped over the bug actually used [clock format].  It fails on
anything that gets into SetIntFromAny.

Logged In: YES 
user_id=79902

Why is [string is integer] setting the type of the object?