Tcl Source Code

The case of no elements in Tcl_SetListObj when setting 
the string representation to tclEmptyStringRep fails 
to reset the length to zero.

diff -rc tcl8.3.2/generic/tclListObj.c 
tcl8.3.2.new/generic/tclListObj.c
*** tcl8.3.2/generic/tclListObj.c       Tue Aug 10 
22:45:11 1999
--- tcl8.3.2.new/generic/tclListObj.c   Tue Feb 20 
10:34:08 2001
***************
*** 265,270 ****
--- 265,271 ----
        objPtr->typePtr = &tclListType;
      } else {
        objPtr->bytes = tclEmptyStringRep;
+       objPtr->length = 0;
      }
  }

Logged In: YES 
user_id=72656

Don's good read of the docs encourages me to just take the 
patch as is.  Improving the 'empty' object in the future is 
still a possibility. In 8.4a3/8.3.3.

Logged In: YES 
user_id=80530

According to Tcl_Obj(3) (doc.Object.3):

 The bytes and the length members together hold an object's
 string representation, which is a counted or binary string
 that  may  contain  binary  data with embedded null bytes.
 bytes points to the first byte of the  string  representa-
 tion.   The  length member gives the number of bytes.  The
 byte array must always have a null after the last byte, at
 offset  length;

There is similar language in ObjectType.3.  Manipulators of
Tcl_Obj's must preserve the invariant:

(objPtr->bytes == NULL) ||
(objPtr->bytes[objPtr->length] == '\0')

Tcl_SetListObj() fails to do that.  The contributed
patch corrects the failure.

I'm punting back to the List Object maintainers to 
determine whether the best fix is the contributed
patch, or having Tcl_SetListObj() return a Tcl_Obj
that actually has (objPtr->typePtr == &tclListType).

Logged In: YES 
user_id=72656

OK, it looks like there are a few things here that could be 
fixed.  There is a bug, but I think that setting length to 
0 is just a band-aid.  In SetStringFromAny, the reason it 
uses the objPtr->length is that bytes != NULL, WITHOUT 
checking for bytes == tclEmptyStringRep (a pseudo-
equivalent) which other procs in tclStringObj.c make sure 
to do (also in tclObj.c).  So, you could fix 
SetStringFromAny to account for this just as 
Tcl_SetObjLength and others do.

However, what might be more interesting, and has come up in 
discussion before, is why an empty list isn't a Tcl list 
obj.  The following is not true:

   Tcl_SetListObj(objPtr, objc, objv);
   /* This fails for objc == 0 */
   assert(objPtr->type == &tclListType);

From basic tests, you wouldn't save much time trying to 
avoid that, because any tclListType must have a valid list 
rep (which means an alloc for an empty list).

It seems odd in any case that Tcl_SetListObj would just set 
the bytes to tclEmptyStringRep when that means anything 
done to the object will cause it to become a string.

All in all, it might be easiest to set the length to 0, and 
note that that is equivalent to TclInitStringRep.  It looks 
to be intentional (and in the spirit of Tcl) that empty 
objects have no particular type to start, and don't get one 
until you do something with it.

File Added - 4710: segfault.c

Logged In: YES 
user_id=80530

I'm re-opening this and attaching a small C program that
demonstrates that this bug can cause segfaults.  I think
the contributed fix should be accepted.

In segfault.c, after Tcl_SetListObj() returns, 
objPtr->bytes == tclEmptyStringRep, but objPtr->length != 0.
Then Tcl_AppendToObj() calls SetStringFromAny() which writes
to objPtr->bytes[objPtr->length], assuming that length has
a value that makes this safe.  Instead memory is corrupted.

segfault.c has been written so the corrupted memory changes
the value of refArray in tclPreserve.c so the next time
Tcl_Preserve() is called => SIGSEGV!

This *could* be fixed with a rather complicated fix to
SetStringFromAny(), but the simple fix contributed here
is the better choice.  Also, everywhere else in the core
when ->bytes is assigned to tclEmptyStringRep, ->length is
also assigned to 0, even if ->typePtr == NULL, notably in
Tcl_NewObj().  I don't see any reason why Tcl_SetListObj()
should be exceptional.

Logged In: YES 
user_id=72656

This presumes that someone is accessing the length value 
without taking into account that typePtr is NULL.  That 
would be a no-no.  If that occurs somewhere, I'd fix that, 
not add this unnecessary reset of length.