Tcl Source Code

View Ticket
Login
Ticket UUID: 1677512
Title: lset shared intrep bug
Type: Bug Version: obsolete: 8.5a6
Submitter: dgp Created on: 2007-03-09 18:37:47
Subsystem: 14. List Object Assigned To: dgp
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2007-03-21 03:50:01
Resolution: Fixed Closed By: kennykb
    Closed on: 2007-03-20 20:49:11
Description:
The TclLsetFlat() routine appears
to suffer from the same problem
mentioned in 1675044.  

It is possible for an
unshared "list" Tcl_Obj
to have a List intrep that
it shares with another Tcl_Obj.
In TclLsetFlat, care is taken
to avoid shared List intreps only
when the Tcl_Obj itself is shared.
Otherwise the direct surgery
on the List struct might leak to
other values.

As with 1675044, it is remarkably
difficult to construct a demo script
for this possibility.  Not accomplished
so far.
User Comments: dgp added on 2007-03-18 02:00:49:

File Deleted - 220971: 



File Added - 221035: 1677512.patch

Logged In: YES 
user_id=80530
Originator: YES


Latest patch has much
code cleanup and commentary.
I think it's suitable for
commit, with any further changes
(refcount revision) to happen
post commit.  passing to kbk
for review.
File Added: 1677512.patch

dgp added on 2007-03-17 12:08:27:

File Added - 220971: revised.patch

Logged In: YES 
user_id=80530
Originator: YES


Revised patch merges the two loops
back into one, and starts to look
more like the previous implementation.
It still offers the possibility of
dropping the artificial refcount
bump of the returned value.
File Added: revised.patch

dgp added on 2007-03-17 02:13:24:
Logged In: YES 
user_id=80530
Originator: YES


Note that the rewritten
TclLsetFlat in the patch
explicitly calls
Tcl_IncrRefCount(resultPtr)
just before it returns
resultPtr, to keep compat with
the refCount conventions
TclLsetFlat has always had.

This increment no longer serves
any purpose, so it could be
dropped while changing all callers
at the same time.

dgp added on 2007-03-17 01:07:41:

File Deleted - 220723: 



File Added - 220910: 1677512.patch

Logged In: YES 
user_id=80530
Originator: YES


Updated patch resolves
the surplus copies problem.
File Added: 1677512.patch

dgp added on 2007-03-16 01:23:47:

File Added - 220723: 1677512.patch

Logged In: YES 
user_id=80530
Originator: YES


attached patch fixes the bug,
but might possibly be too
inefficient, by making surplus
copies when the variable being
[lset] holds an unshared value.
File Added: 1677512.patch

dgp added on 2007-03-15 00:50:03:
Logged In: YES 
user_id=80530
Originator: YES

spoke too soon. here's the test:

testConstraint testobj [llength [info commands testobj]]
test lset-15.1 {lset: shared intrep [Bug 1677512]} -setup {
    teststringobj set 1 {{1 2} 3}
    testobj convert 1 list
    testobj duplicate 1 2
    variable x [teststringobj get 1]
    variable y [teststringobj get 2]
    testobj freeallvars
    set l [list $y z]
    unset y
} -constraints testobj -body {
    lset l 0 0 0 5
    lindex $x 0 0
} -cleanup {
    unset -nocomplain x l
} -result 1

dgp added on 2007-03-14 02:18:35:
Logged In: YES 
user_id=80530
Originator: YES


The actual value setting
gets done by the
TclListObjSetElement()
routine, which does the
right intrep sharing safety
checks to avoid trouble.

Attachments: