Tcl Source Code

View Ticket
Login
Ticket UUID: 723a2f4ac32c306a16919568acaf0b78304dea7d
Title: return -code break/tailcall break from coroutine goes wrong at the top-level
Type: Bug Version:
Submitter: aspect Created on: 2018-07-31 13:11:32
Subsystem: 60. NRE and coroutines Assigned To: nobody
Priority: 8 Severity: Minor
Status: Pending Last Modified: 2019-08-27 17:33:03
Resolution: Fixed Closed By: nobody
    Closed on:
Description:
Depending on the set-up, this seems to be sensitive either to the context the coroutine is created in or the context it is called in.  If both have at least one frame on the stack, it works "as expected".


% proc p0 {} {yield; return -code break}
% coroutine c0 p0
% while 1 c0
invoked "break" outside of a loop
    while executing
"p0"
    invoked from within
"c0"

% apply {{} {coroutine c0 p0}}
% while 1 c0

% proc p1 {} {yield; tailcall break}
% coroutine c1 p1
% while 1 c1
invoked "break" outside of a loop
    while executing
"break"
    invoked from within
"c1"

% coroutine c1 p1
% apply {{} {while 1 c1}}


Another fun one:

$ cat itop.tcl
if 1 {coroutine c0 try {yield; break}}
while 1 c0
puts "ok1"

coroutine c0 try {yield; break}
while 1 c0
puts "ok2"

$ tclsh itop.tcl 
ok1
invoked "break" outside of a loop
    while executing
"try {yield; break}"
    invoked from within
"c0"
    ("while" body line 1)
    invoked from within
"while 1 c0"
    (file "itop.tcl" line 6)


(noticed by pooryorick on the chat yesterday)
User Comments: sebres added on 2019-08-27 17:33:03:

as for knownBug: it looks like interp-26.* (result code transmission: invoke hidden) indeed handled the same difficulty.


sebres added on 2018-08-13 13:49:28:
Which "knownBug" test in interp.test have you meant exactly, Don?

sebres added on 2018-08-13 13:34:18:
Regarding the TclNRInvoke, why you think that the way with level increment with following decrement (via NRPostInvoke) is better as it currently made in my branch?

Or rather did you meant we should just unify the handling to get one standard procedure for this?

sebres added on 2018-08-13 12:53:19:

> Which test(s) do you see triggering a segfault?

None anymore (so I cannot reproduce it right now, possibly because previously I tried to run newest test-cases with upstream-version before [9198c16407], so actually it could be the case tailcall-14.1-bc or similar... After upgrade to newest 8.6th version, clean and build, nothing evil happens (just the failed test-cases as expected).

> tests engineered to operate at toplevel in normal test suite operations.

Yes, that are indeed only this test-cases that are affected now. But I wanted to provide more test-cases (to cover also other levels resp. return constellations, that working now but could going to be broken tomorrow).

As regards the interp alias and TclNRInvoke, I'll take a look inside...
Thank you Don.


dgp added on 2018-08-10 17:53:07:
Take note of the routine TclNRInvoke() and the way it
seems to have handled the same difficulty.

dgp added on 2018-08-07 14:00:44:
There's a related anomaly that has been around a long time.

% interp create slave
slave
% interp alias {} b {} break
b
% while 1 b
% interp alias {} b slave break
b
% while 1 b
invoked "break" outside of a loop
% catch {slave eval break}
3
% catch b
1

This is related to some of the "knownBug" tests in interp.test

dgp added on 2018-08-06 16:16:55:
Which test(s) do you see triggering a segfault?

I see just 4 of the new tests failing on the unpatched
core-8-6-branch tip:

coroutine-9.1b-nc
coroutine-9.2b-nc
tailcall-15.1b-nc
tailcall-15.2b-nc

Should I see something different?

It appears those are the only tests engineered to
operate at toplevel in normal test suite operations.

sebres added on 2018-08-03 16:18:19:

Fixed now in branch sebres-bug-723a2f4ac3.

Additionally this fixes SEGFAULTS by direct return/break out of coroutine after yield back into coroutine (I've increased priority of ticket to High).
Without this fix (with unpatched original 8.6) the new test-cases of this branch segfaulting within coroutine.test.
So fixed here by provide of TCL_EVAL_NOERR to Tcl_NREvalObj inside TclNRCoroutineObjCmd, because return with code out of coro (after yield into coro) does not allow normally any modification on call-stack info in the top-level at all.


dgp added on 2018-08-02 12:43:17:
There are two bugs here. A fix for the problem
with coroutine is now on branch bug-723a2f4ac3 .
Please test and comment.

The second 'bug' is with tailcall, and is best demo'd
without any confounding coroutine:

% proc p {} {tailcall break}
% while 1 break
% while 1 p
invoked "break" outside of a loop

It's not quite so clear to me what's correct
for this case.

dgp added on 2018-07-31 18:10:01:
Without a complete deep dive, this suggests the
implementation of [coroutine] is missing a call
to Tcl_AllowExceptions().

The bass-ackward nature of return code handling
at the toplevel is a topic for another day. Would
be great if Tcl 9 did not continue this brain damage.