Tcl Source Code

Hi *,

initially I got a core dump on AIX 5.2 in FlushChannel() of tclIO.c using Tcl 8.4.19.

I tracked it down to the memcpy() call in AppendUnicodeToUnicode() in tclStringObj.c that is getting a wrong destination pointer in case the attemptckrealloc() call (doubling available space for StringPtr) failed. Once I understood how this all comes to happen, the following simple script provokes this crash even on Linux ...

proc bar {} {
    set f [string repeat "\xDE\xAD\xBE\xEF" 1024] ;# for the crash in AppendUnicodeToUnicodeRep()
    #set f [string repeat "DEADBEEF" 1024] ;# for a crash in AppendUtfToUtfRep() that I have not investigated further
    set r  ""
    set i  0

    while {1} {
puts -nonewline stdout "$i\r"
set fl [string length $f]
if {($fl != [string length $r]) || 1} {
    append r [string range $f 0 $fl]
}
incr i $fl
    }
}
bar

I  don't quite understand why it gets the wrong address there, yet.

Note: It may happen that the crash occurs at different positions, or leaves unreadable core dumps behind. This could be avoided with setting the environment variable MALLOC_CHECK_=2 on a fairly recent Linux system, or MALLOCTYPE=debug on AIX. The crash also depends on how much free memory is available.

If you need more information, core dumps, stack traces ... just ask.

One furher thing, I polluted the sources with printf()s, the crashing call to AppendUnicodeToUnicodeRep() generated:

+++ alloced tmpString (0x2ab8abe8) of size uallocated (67126290)
+++ copying 8192 bytes from 0x20b59ff8 to 0x32b8cbf8

1. +++ ->
 fprintf( stderr, "+++ alloced tmpString (0x%lx) of size uallocated (%ld)\n", tmpString, STRING_SIZE(stringPtr->uallocated) );
 just before
 stringPtr = tmpString;

2. +++ ->
 fprintf( stderr, "+++ copying %ld bytes from 0x%lx to 0x%lx\n", appendNumChars * sizeof(Tcl_UniChar), unicode, (VOID *)(stringPtr->unicode + stringPtr->numChars));
 just before the memcpy() call.

-- Matthias Kraft

allow_comments - 1

backports complete

I doubt that ropes would help for this. Or rather they'll help for this specific example, but probably not with the real-world cases that this is distilled from. I'm very uneasy about coding to benchmarks rather than full use-cases...

Semi-on-topic side note. Reading

> The system is able to allocate the big string
> for the [string repeat] result, but then fails
> to allocate space for a second big string to
> hold the whole word.

I caught myself thinking that ROPES would have helped here, as the second big string would share the data from the first big string for its representation.

The Solaris panic happens during
the append implicit in joining the
parts of the word,

  \xDE[string repeat a [expr (1<<31)-2000]]

together.  Appears this isn't anything
wrong with Solaris, or Tcl on Solaris,
but just a quirk of exactly when the
test system runs out of memory.  The
system is able to allocate the big string
for the [string repeat] result, but then
fails to allocate space for a second big
string to hold the whole word.

While chasing this down, though, I did find
another place where string overflows are
not handled, which I will file separately.

Reproduced the odd behavior on
solaris + forte.  looking...

On Solaris, are you compiling with gcc
or with the Forte C compiler, or something else?

Ok, used your test script and observed no hard crashes, but three different behaviors:

1) if not enough memory available to execute the [string repeat] line it comes back with a Tcl error:

string size overflow, out of memory allocating 2147481649 bytes
    while executing
"string repeat a [expr (1<<31)-2000]"
    (procedure "grill" line 2)
    invoked from within
"grill"
    (file "/RDS/COMPSHARE/home//makr/crashtest.tcl" line 55)

2) otherwise it aborts in the append line:

max length for a Tcl unicode value (2147483637 chars) exceeded
Abort

3) on Solaris it aborts with:

unable to alloc 2147481651 bytes

I am not sure why Solaris is behaving strange again and I don't have time currently to investigate...

-- Matthias Kraft

Uhm ... only with my scripts. I somehow ignored that comment. But I still have the compile environments. So unless someone steps up today and wants the space on the compile servers returned, I could retest over the weekend.

-- Matthias Kraft

Thanks for the testing report.

Please clarify, did you do any testing
with the more robust test script from
my 2009-02-18 comment, or only with
your original test script?

Used HEAD as of today, compiled and tested on various AIX, HP-UX, Linux, and Solaris machines.

Looks good: It either aborts with "unable to realloc ... bytes" or with "max size for a Tcl value (2147483647 bytes) exceeded".

One exception: Linux on a s390x mainframe. Here it seems the kernel is killing the process before Tcl gets a chance to abort. IIRC that's a feature of the Linux kernel.

kind regards
-- Matthias Kraft

will do, but I am currently overloaded with paid work. It might be not before next week that I will find time ...

kind regards
-- Matthias Kraft

please give the HEAD another round
of rigorous testing.

A more robust test script:

set s \xDE[string repeat a [expr (1<<31)-2000]]
string index $s 0
while 1 {
append s a
}

Yes, there are still some overflows
of the stringPtr->numChars value
that need repair.  Thanks for the
continued testing.

Tested HEAD on Solaris 5.9 and 5.10 today. WRT this report, I still have a crash in

signal SEGV (no mapping at the fault address) in _memcpy at 0xff3b0730
0xff3b0730: _memcpy+0x0250:     std      %c1, [%o0]
Current function is AppendUnicodeToUnicodeRep
 1444               appendNumChars * sizeof(Tcl_UniChar));

as you will see below we still have an integer overrun here

(dbx) print *stringPtr
*stringPtr = {
    numChars   = 1073745920
    allocated  = 16384U
    uallocated = 32768U
    hasUnicode = 1
    unicode    = (222U, 173U)
}

uallocated is much lower than numChars. See also my comment from "2009-02-02 11:45" ...

-- Matthias Kraft

Go! :-) ... but remember that change I made to stringAttemptRealloc

-- Matthias Kraft

Before you go away, can you translate
all of that into a simple Go! or No Go!
for the backport of the code on the
current HEAD to older branches?

Okay, it is time I do something else, I got it wrong in my Linux test - modified stringAlloc instead of stringAttemptRealloc ... it is working on Linux too!

-- Matthias Kraft

Don't use my lame attempt with that macro! With it Tcl isn't even going to "make install" on Linux...

-- Matthias Kraft

With stringAttemptRealloc changed to

#define stringAttemptRealloc(ptr, numBytes) \
    (String *) ((((numBytes) == 0) || ((numBytes) > INT_MAX - STRING_SIZE(0))) \
    ? NULL \
    : attemptckrealloc((char *) ptr, (unsigned) STRING_SIZE( \
(numBytes) ? (numBytes) : sizeof(Tcl_UniChar)) ))

it seems to fail cleanly now. I go and do the retest dash over my other systems now ...

-- Matthias Kraft

peppered the code with printf()s again ... on Solaris revealed:

>>> numChars 1073733632; ualloc(numChars) 2147467264 >= uallocated 2147460096?
>>>  attempt to reallocate 4294934528
>>>   reallocate 2147476480
>>>  new buffer @ 0x2109a0, old was @ 0x2109a0
>>> copy 8192 from 0x206970 to 0x8020a9b0

>>> numChars 1073737728; ualloc(numChars) 2147475456 >= uallocated 2147476480?
>>> copy 8192 from 0x206970 to 0x8020c9b0

>>> numChars 1073741824; ualloc(numChars) 2147483648 >= uallocated 2147476480?
>>>  attempt to reallocate 0
>>>  new buffer @ 0x1f6b28, old was @ 0x2109a0
>>> copy 8192 from 0x206970 to 0x801f4b38

>>> numChars 1073745920; ualloc(numChars) 2147491840 >= uallocated 0?
>>>  attempt to reallocate 16384
>>>  new buffer @ 0x20a980, old was @ 0x1f6b28
>>> copy 8192 from 0x206970 to 0x8020a990

>>> numChars 1073750016; ualloc(numChars) 2147500032 >= uallocated 16384?
>>>  attempt to reallocate 32768
>>>  new buffer @ 0x20a980, old was @ 0x20a980
>>> copy 8192 from 0x206970 to 0x8020c990

>>> numChars 1073754112; ualloc(numChars) 2147508224 >= uallocated 32768?
>>>  attempt to reallocate 49152
>>>  new buffer @ 0x20a980, old was @ 0x20a980
>>> copy 8192 from 0x206970 to 0x8020e990

>>> numChars 1073758208; ualloc(numChars) 2147516416 >= uallocated 49152?
>>>  attempt to reallocate 65536
>>>  new buffer @ 0x20a980, old was @ 0x20a980
>>> copy 8192 from 0x206970 to 0x80210990

So there is an entry that show that we attempt to reallocate 0 bytes. The preprocessor result is ...

    numChars = stringPtr->numChars + appendNumChars;

    if ( ( ( numChars ) * sizeof ( Tcl_UniChar ) ) >= stringPtr->uallocated) {
     stringPtr->uallocated =  ( ( 2 * numChars ) * sizeof ( Tcl_UniChar ) );
tmpString =  ( String * ) ( ( ( stringPtr -> uallocated ) > 2147483647 - ( sizeof ( String ) - sizeof ( Tcl_UniChar ) + ( 0 ) ) ) ? 0 : Tcl_AttemptRealloc ( ( char * ) stringPtr , ( unsigned ) ( sizeof ( String ) - sizeof ( Tcl_UniChar ) + ( ( stringPtr -> uallocated ) ? ( stringPtr -> uallocated ) : sizeof ( Tcl_UniChar ) ) ) ) );
if (tmpString ==  0 ) {
    stringPtr->uallocated =
         ( ( numChars + appendNumChars ) * sizeof ( Tcl_UniChar ) ) 
+  1024 ;
    tmpString =  ( String * ) ( ( ( stringPtr -> uallocated ) > 2147483647 - ( sizeof ( String ) - sizeof ( Tcl_UniChar ) + ( 0 ) ) ) ? ( Tcl_Panic ( "unable to alloc %u bytes" , ( sizeof ( String ) - sizeof ( Tcl_UniChar ) + ( stringPtr -> uallocated ) ) ) , ( char * ) 0 ) : Tcl_Realloc ( ( char * ) stringPtr , ( unsigned ) ( sizeof ( String ) - sizeof ( Tcl_UniChar ) + ( ( stringPtr -> uallocated ) ? ( stringPtr -> uallocated ) : sizeof ( Tcl_UniChar ) ) ) ) );
}
stringPtr = tmpString;
 ( objPtr ) -> internalRep . otherValuePtr = ( void * ) ( stringPtr );
    }

So we attempt to realloc 0 bytes, that does not fail (and is legal, should free the memory) and even seem to return some pointer...

-- Matthias Kraft

It worked on HP-UX, AIX, and Linux, but not on Solaris. On Solaris it somehow manages to get over that INT_MAX limit and core dumps in the following memcpy() call in AppendUnicodeToUnicodeRep(). Will retry next week. Maybe the preprocessor output sheds some light on how this is possible ...

-- Matthias Kraft

Nevermind, Andreas described the fix already ... but it is not yet in HEAD!

-- Matthias Kraft

Update: The patch works on Linux x86_64 with top of core-8-5-branch. Variables can now again hold (1 << 30) - appendNumChars bytes before panic().

But: The patch as it is now, does not compile on AIX using xlC:

.../generic/tclStringObj.c", line 355.17: 1506-226 (S) The ":" operator is not allowed between "int" and "char*".

that is from an attempt to compile a patched 8.4.19. I will look into this, but maybe not today as I am working from home and the VPN is sluggish and slow today -> no fun...

-- Matthias Kraft

Sorry Don,

I was off to a workshop and have to look after other paid work now. I hope to find time for testing tomorrow or Friday...

kind regards
-- Matthias Kraft

Agreed.  Matthias????

I do not believe so. We are not stressing the memory subsystem with extra-large allocs.

no reply from matzek.

aku, is your testing enough
to confirm this is ok for
backport?

Fix: The NULL after the Tcl_Panic in STRING_NOMEM needs a cast to (char*).

Don, please review your changes. On HPUX IA64 I am now getting errors and these are in the macros you modified. When I diffed to history I saw that they became quite complex.

andreask@bigsur03:~/workbench/z> tail log/tcl-8.6/make.log
Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2720 # Illegal types found in conditional operator: 'long' and 'char *'.
            copyStringPtr = stringAlloc(STRING_UALLOC(0));
                            ^^^^^^^^^^^
Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2723 # Illegal types found in conditional operator: 'long' and 'char *'.
            copyStringPtr = stringAlloc(srcStringPtr->uallocated);
                            ^^^^^^^^^^^
Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2789 # Illegal types found in conditional operator: 'long' and 'char *'.
            stringPtr = stringAlloc(STRING_UALLOC(0));
                        ^^^^^^^^^^^
make: *** [tclStringObj.o] Error 2
andreask@bigsur03:~/workbench/z>

fix committed to HEAD.

Please test and after you
confirm success, I will backport.

ok, I think fixing the 
panic on attempt problem corrects
both issues.  When the
attemp panicked prematurely,
that cut off the "slow growth"
portion of the string growth algorithm,
which would in fact reduce the allocatable
buffer by about a factor two.

Patch on the way...

matzek, can you provide your
evidence that the commit has
dropped the max value size
in half?

Thanks for pointing out
the problem with attempt.
The new panic is in the wrong
place.  Trickier than I thought.

From a mail I sent to the tcl-core list:

I want to give two points to your attention that concern me with the fix of this bug. But first thank you very much for helping to find the problem and then fixing it promptly!

1) The fix as now implemented restricts the content of variables to INT_MAX/4 which is 512MB! To my mind this is comparatively small and should be documented somewhere. Please notice that this is also half the size, of what was possible in 8.4.19 / 8.5.6.

2) In tclStringObj.c is a lengthy documentation of the memory allocation algorithm for strings. This is now obsolete in the case when more than INT_MAX/4 memory is free and could be allocated. As in this case we panic() before even coming to the attemptckalloc().

What you should make with this information? Well, I don't know. I just wanted to make sure it doesn't get lost and someone has to learn it the hard way. I can live with the fix now.

Where is the use case that needs so much memory? Consider vfs::memchan. This is where I stumbled over the issue. I thought I use vfs::memchan to be able to transparently uncompress and evaluate the crc of archives...

trouble in STRING_SIZE() also fixed
in all branches.

ok, looked into 8.5 again: it now seems to be some kind of integer overflow, or something in this direction.

My Linux build is 64 bit, that means sizeof(size_t) is 8, while sizeof(int) is 4.

The core dump now shows:

- the memcpy() call in AppendUnicodeToUnicodeep() crashes again:
  #0  0x00007febad876dd3 in memcpy () from /lib64/libc.so.6
  #1  0x00007febae0a82d1 in AppendUnicodeToUnicodeRep (objPtr=0x625990, unicode=0x63bafc, appendNumChars=4096)
      at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1336

- the other values are:
  (gdb) print *stringPtr
  $2 = {numChars = 1073737728, allocated = 16384, uallocated = 4294967296, hasUnicode = 1, unicode = {222, 173}}
  (gdb) print numChars
  $3 = 1073741824

- $3 == 1<<30 == 1GB

- (gdb) print numChars - stringPtr->numChars
  $6 = 4096

- stringPtr->uallocated == 1<<32 == 4GB which means the attemptckrealloc() has not failed

so far my observations, maybe I have time to come back to it later today ...

Testing the patch with my customized Tcl 8.4.19 on AIX, shows that it seems to be fixed there ... that means I am running into out-of-memory now and up to that point the buffers seem to be ok ...

Sorry Don, but it is not fixed. I tested core-8.5-branch with your changes this morning and it still dumps core on Linux at the same place with the same stack trace ...

fixed in all branches

Found it.  Problem is in the STRING_UALLOC() macro:

#define STRING_UALLOC(numChars) \
        (numChars * sizeof(Tcl_UniChar))

It needs changing to:

#define STRING_UALLOC(numChars) \
        ((numChars) * sizeof(Tcl_UniChar))

sorry, I think my hexadigit counting 
was off by one.  The crash happens
reallocating for 2**28 characters.

In case it's not already obvious,
the crash happens when "numChars"
value escapes the range of a signed int.

if this is fixed reasonably quickly,
the fix will also be part of 8.4.20.

This bug is present in Tcl 8.5.6, but I have not the freedom of choice and am still stuck at 8.4.19. So if someone can fix it for 8.5 I kindly ask to provide a patch that could be applied against 8.4, too, or back ported.

Do you mean you have confidence that the bug is no longer present after 8.4 ?
What are your reasons to stick to 8.4 ?

As I now have missed the bus anyway, there is time to ask for a patch that could be ported back to 8.4 :-) ...

Stack trace on Linux x86_64 with Tcl 8.5.6:

Program terminated with signal 11, Segmentation fault.
#0  0x00007f5443b0ddd3 in memcpy () from /lib64/libc.so.6
(gdb) bt
#0  0x00007f5443b0ddd3 in memcpy () from /lib64/libc.so.6
#1  0x00007f544433f23e in AppendUnicodeToUnicodeRep (objPtr=0x624330, unicode=0x63a55c, appendNumChars=4096)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1336
#2  0x00007f544433efb3 in Tcl_AppendObjToObj (objPtr=0x624330, appendObjPtr=0x6244e0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1230
#3  0x00007f54443536e1 in TclPtrSetVar (interp=0x605790, varPtr=0x6067b0, arrayPtr=0x0, part1Ptr=0x0, part2Ptr=0x0, newValuePtr=0x6244e0, flags=516, index=1)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclVar.c:1881
#4  0x00007f54442deed5 in TclExecuteByteCode (interp=0x605790, codePtr=0x6367f0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclExecute.c:2952
#5  0x00007f5444337011 in TclObjInterpProcCore (interp=0x605790, procNameObj=0x623b20, skip=1, errorProc=0x7f544433771f <MakeProcError>)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclProc.c:1748
#6  0x00007f5444336f0b in TclObjInterpProc (clientData=0x6272f0, interp=0x605790, objc=1, objv=0x6065c0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclProc.c:1642
#7  0x00007f5444279c39 in TclEvalObjvInternal (interp=0x605790, objc=1, objv=0x6065c0, command=0x634b46 "bar\n", length=4, flags=0)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:3690
#8  0x00007f544427af60 in TclEvalEx (interp=0x605790, 
    script=0x6347d0 "set i 1234567890\nset n /dev/zero\n\nproc foo {n i} {\n    set f [open $n r]\n    fconfigure $f -translation binary -buffering full -buffersize 4096\n\n    upvar #0 data($n,raw) r\n    set r \"\"\n\n    while {$i"..., numBytes=890, flags=0, line=46) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:4338
#9  0x00007f544427a4b5 in Tcl_EvalEx (interp=0x605790, 
    script=0x6347d0 "set i 1234567890\nset n /dev/zero\n\nproc foo {n i} {\n    set f [open $n r]\n    fconfigure $f -translation binary -buffering full -buffersize 4096\n\n    upvar #0 data($n,raw) r\n    set r \"\"\n\n    while {$i"..., numBytes=890, flags=0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:4043
#10 0x00007f544430e3e1 in Tcl_FSEvalFileEx (interp=0x605790, pathPtr=0x624690, encodingName=0x0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclIOUtil.c:1814
#11 0x00007f5444316ed0 in Tcl_Main (argc=-1, argv=0x7fff4c7ba4a8, appInitProc=0x400925 <Tcl_AppInit>) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclMain.c:441
#12 0x000000000040091e in main (argc=2, argv=0x7fff4c7ba498) at /home/makr/cvs/tccvs/tcl-8.5/unix/../unix/tclAppInit.c:87

Stack trace on AIX with Tcl 8.4.19:

Segmentation fault in . at 0xf050
0x0000f050 warning: Unable to access address 0xf050 from core
(dbx) where
warning: Unable to access address 0xf050 from core
warning: Unable to access address 0xf050 from core
warning: Unable to access address 0xf04c from core
warning: Unable to access address 0xf04c from core
warning: Unable to access address 0xf050 from core
warning: Unable to access address 0xf050 from core
warning: Unable to access address 0xf04c from core
warning: Unable to access address 0xf04c from core
warning: Unable to access address 0xf050 from core
.() at 0xf050
AppendUnicodeToUnicodeRep(objPtr = 0x20996ca0, unicode = 0x20b59ff8, appendNumChars = 4096), line 1260 in "tclStringObj.c"
Tcl_AppendObjToObj(objPtr = 0x20996ca0, appendObjPtr = 0x209967c0), line 1154 in "tclStringObj.c"
TclPtrSetVar(interp = 0x20090e08, varPtr = 0x2ff21718, arrayPtr = (nil), part1 = warning: Unable to access address 0x20b24ff8 from core
(invalid char ptr (0x20b24ff8)), part2 = (nil), newValuePtr = 0x209967c0, flags = 516), line 1666 in "tclVar.c"
TclExecuteByteCode(interp = 0x20090e08, codePtr = 0x20b46eb0), line 2072 in "tclExecute.c"
TclCompEvalObj(interp = 0x20090e08, objPtr = 0x20996958), line 1107 in "tclExecute.c"
unnamed block $b1240, line 1180 in "tclProc.c"
TclObjInterpProc(clientData = 0x20aa4fe0, interp = 0x20090e08, objc = 1, objv = 0x2ff21ca8), line 1180 in "tclProc.c"
TclEvalObjvInternal(interp = 0x20090e08, objc = 1, objv = 0x2ff21ca8, command = warning: Unable to access address 0x20a73ff4 from core
(invalid char ptr (0x20a73ff4)), length = 4, flags = 0), line 3219 in "tclBasic.c"
Tcl_EvalEx(interp = 0x20090e08, script = warning: Unable to access address 0x20a73d00 from core
(invalid char ptr (0x20a73d00)), numBytes = 760, flags = 0), line 4011 in "tclBasic.c"
Tcl_FSEvalFile(interp = 0x20090e08, pathPtr = 0x204b5f70), line 1776 in "tclIOUtil.c"
Tcl_Main(argc = -1, argv = 0x2ff222c0, appInitProc = 0x2001b92c), line 295 in "tclMain.c"
main(argc = 2, argv = 0x2ff222b8), line 288 in "instsh.c"