Tk Source Code

Several text-37.* tests fail on OS X with the deployment target only:

==== text-37.1 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete begin end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{break}} {{text 1} {break} {text 2} {break}} {{break}}
---- Result should have been (exact matching):
{{break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {}}} {{break {}}}
==== text-37.1 FAILED



==== text-37.2 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete 2.0 end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{text 1} {break} {break}} {{text 1} {break} {text 2} {break}} {{text 1} {break} {break}}
---- Result should have been (exact matching):
{{text 1 {}} {break {n1}} {break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {}}} {{text 1 {}} {break {n1}} {break {}}}
==== text-37.2 FAILED



==== text-37.3 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete 1.end end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{text 1} {break}} {{text 1} {break} {text 2} {break}} {{text 1} {break}}
---- Result should have been (exact matching):
{{text 1 {}} {break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {}}} {{text 1 {}} {break {}}}
==== text-37.3 FAILED



==== text-37.4 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete 2.end end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{text 1} {break} {text 2} {break}} {{break}} {{text 1} {break} {text 2} {break}}
---- Result should have been (exact matching):
{{text 1 {}} {break {n1}} {text 2 {}} {break {}}} {{break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {}}}
==== text-37.4 FAILED



==== text-37.5 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete begin end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{break}} {{text 1} {break} {text 2} {break} {break}} {{break}}
---- Result should have been (exact matching):
{{break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {n2}} {break {}}} {{break {}}}
==== text-37.5 FAILED



==== text-37.6 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete 2.0 end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{text 1} {break} {break}} {{text 1} {break} {text 2} {break} {break}} {{text 1} {break} {break}}
---- Result should have been (exact matching):
{{text 1 {}} {break {n1}} {break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {n2}} {break {}}} {{text 1 {}} {break {n1}} {break {}}}
==== text-37.6 FAILED



==== text-37.7 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete 1.end end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{text 1} {break}} {{text 1} {break} {text 2} {break} {break}} {{text 1} {break}}
---- Result should have been (exact matching):
{{text 1 {}} {break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {n2}} {break {}}} {{text 1 {}} {break {}}}
==== text-37.7 FAILED



==== text-37.8 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete 2.end end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{text 1} {break} {text 2} {break}} {{text 1} {break} {text 2} {break} {break}} {{text 1} {break} {text 2} {break}}
---- Result should have been (exact matching):
{{text 1 {}} {break {n1}} {text 2 {}} {break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {n2}} {break {}}} {{text 1 {}} {break {n1}} {text 2 {}} {break {}}}
==== text-37.8 FAILED



==== text-37.9 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete begin end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{break}} {{break}} {{break}}
---- Result should have been (exact matching):
{{break {}}} {{break {n}}} {{break {}}}
==== text-37.9 FAILED



==== text-37.10 deletion - clean widget FAILED
==== Contents of test case:

    lappend res [.t inspect -chars -tag]
    .t delete begin end
    lappend res [.t inspect -chars -tag]
    catch {
        .t edit undo
        lappend res [.t inspect -chars -tag]
    }
    set res

---- Result was:
{{break}} {{break}}
---- Result should have been (exact matching):
{{break {}}} {{break {}}}
==== text-37.10 FAILED



==== text-37.12 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete begin end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{break}} {{text 1} {break}} {{break}}
---- Result should have been (exact matching):
{{break {}}} {{text 1 {n}} {break {n}}} {{break {}}}
==== text-37.12 FAILED



==== text-37.13 deletion - newline behavior - undo/redo behavior FAILED
==== Contents of test case:

    .t delete begin end
    lappend res [.t inspect -chars -tag]
    .t edit undo
    lappend res [.t inspect -chars -tag]
    .t edit redo
    lappend res [.t inspect -chars -tag]
    set res

---- Result was:
{{break}} {{text 1} {break}} {{break}}
---- Result should have been (exact matching):
{{break {}}} {{text 1 {}} {break {n}}} {{break {}}}
==== text-37.13 FAILED

Fix committed to revised_text branch.

Thanks Francóis! Doesn't need to wait for TIP #585, since a fallback in tkInt.h is easily done. Just bring the TIP up for voting (whenever you like) and I have no doubt it will be accepted soon.

Thanks! Yes, that's true. It works because the caching mecanism in Tcl_GetIndexFromObj*() uses the internal representation. When that is freed, there is no caching any longer.

I was not aware of Tcl_FreeIntRep(), it's undocumented. I have open a ticket for this.

A workaround is to use Tcl_FreeIntRep() on the object when Tcl_GetIndexFromObj*() returns OK.

These sorts of concerns are why neither ensembles nor TclOO use that API for word-to-index resolution. (In the latter case, what's actually done is a great deal more sophisticated; ensembles are a better and more relevant comparison.)

The difference in production vs debug mode is probably due to the details of memory layout. Whether or not it works is pure happenstance.

I have published TIP #585 to promote the INDEX_TEMP_TABLE flag of Tcl_GetIndexFromObj*() to the public interface.

I have revisited this ticket and have found an interesting story.

On macOS and Linux (the latter is a new observation), tests to text-38.* fail for debug (aka symbols or devel) builds but not for release (aka deploy) builds. On Windows these tests do not fail, for both debug and release builds.

Short test script that demonstrates the issue (and is equivalent to the failing tests):

    package require Tk
    text .t 
    .t dump -tag 1.0 end
    .t inspect -chars -tag

With the development (aka debug, or with symbols) target, the output is the right one:

  {{break} {}}

With the deployment (aka release) target, the output is WRONG:

  {break}

What is happening is that when calling .t dump -tag 1.0 end, TextDumpCmd() first calls GetDumpFlags().

GetDumpFlags(), from the list of all available options optStrings, builds an array of allowed options myOptStrings, "allowed" options being those that should be considered from optStrings.

Then, Tcl_GetIndexFromObjStruct() is invoked to get the index of each option given to the dump command, here it is only -tag. Index value 6 is returned, which is correct.

The program flow goes on, so far, so good, until the test script invokes .t inspect -chars -tag.

This calls TextInspectCmd() which in turn calls GetDumpFlags() again.

GetDumpFlags() does the same job of selecting allowed options again, but this time the list of allowed options myOptStrings that is built is different from the previous call because the allowed and complete flags received are not the same (but dflt is the same in this specific case).

Tcl_GetIndexFromObjStruct() is then called to get in turn the index in myOptStrings of each option given to the inspect command, here it is -chars and -tags. The return value for -chars is correct (value is 2). But the return value for the second option, -tags, is incorrect: it returns 6 instead of 20. 6 then corresponds to TK_DUMP_DISPLAY_TEXT whereas 20 corresponds to the expected TK_DUMP_TAG. The what flag becomes completely wrong and TextInspectCmd() outputs the result for -chars only the TK_DUMP_DISPLAY_TEXT flag being ignored because it's not an inspect flag (but a checksum flag).

So the issue is that Tcl_GetIndexFromObjStruct() returns an incorrect index when called a second time for the same option name. This is easily explained: Tcl_GetIndexFromObjStruct() expects an array of static entries myOptStrings, that must not change between invocations (and this is documented). But the revised text code in GetDumpFlags() is doing something incompatible: the myOptStrings array is not a collection of static objects! A fundamental assumption of Tcl_GetIndexFromObjStruct() is therefore violated.

What Tcl_GetIndexFromObjStruct() returns is a cached value! This explains why 6 is returned: it's the cached index in the myOptStrings array from .t dump -tag 1.0 end.

I think that to fix this the easiest is to disable caching in Tcl_GetIndexFromObjStruct(), by passing the INDEX_TEMP_TABLE flag to this function. I have confirmed that the problem is fixed if instead of 0 we pass 2 (the value of INDEX_TEMP_TABLE) in the call here.

Unfortunately, INDEX_TEMP_TABLE is not a Tcl public flag. Could we make it public in 8.7? I definitely think we have a real use case here, with a dynamic array of options legitimately passed to Tcl_GetIndexFromObjStruct().

Note to self: What remains in the dark at this time, and will need to be understood, is why we don't see the issue with debug builds.

Assigning to gcramer on his request.

These tests were renamed from 37.* to 38.* in [40829bd1] (for good reasons), nevertheless they are still failing on macOS, for the deployment version only.

The short test script triggers the issue that makes all these tests fail:

    package require Tk
    text .t 
    .t dump -tag 1.0 end
    .t inspect -chars -tag

With the development (aka debug, or with symbols) target, the answer is the correct one:

  {{break} {}}

With the deployment (aka release) target, the answer is WRONG:

  {break}

On Windows the answer is the correct one, with both the debug and the release version.

Still happening as of today.

Note there was a slight mistake in the test script, which should be:

    package require Tk
    text .t 
    .t dump -tag 1.0 end
    .t inspect -chars -tag

(The dash was missing in the last line in front of "tag").

Minimal demo script:

    package require Tk
    text .t 
    .t dump -tag 1.0 end
    .t inspect -chars tag

Running this on OS X with the deployment compilation target, the output is:

    {break}

The correct output however is:

    {{break} {}}

Running this on OS X with the development compilation target, or on Windows (with debug or release versions), the correct output is obtained.

All the tests I have reported as failing in the present ticket do pass if all commands:

    .t dump -tag xxxxx

are commented out in previous tests (text-24.13, -24.14, -24.15, -24.16).

Out of the box, the deployment target is compiled with (see unix/configure:5420):

    CFLAGS_OPTIMIZE="-Os"

Problem disappears when compiling the deployment target with:

    CFLAGS_OPTIMIZE="-O1"

Problem is still present when compiling the deployment target with:

    CFLAGS_OPTIMIZE="-Os -fstack-protector-all"

Unfortunately -fstack-protector-all does not bring any insight about what's wrong.

I have found that the following triggers the bug:

    make -C ./macosx test TESTFLAGS="-file text.test -match \"text-37.1 text-24.13\" -verbose beps"

i.e. when text-24.13 is run before text-37.1, then we have a problem.

This looks like an interaction between dumping tags (which is tested in text-24.13) and inspecting tags (which is tested in text-37.1).

The source code has a problem, that the optimization reveals. It's not an optimization bug IMO.