To-Be-Closed For-In-Loop Variable has Problems with Tailcalls

[Xmilia Hermit]

Hi,

I think I just found a bug regarding to-be-closed variables. For some
reason the forth to-be-closed variable of a for-in loop has problems to
close with a tailcall as can be seen in the following example.

local function pairs_with_close()
     return next, {1}, nil, setmetatable({}, {
         __close = function()
             print("This should be called")
         end
     })
end

local function tail()
     return 1, 2, 3, setmetatable({}, {
         __close = function()
             print("Why is this called?")
         end
     })
end

local function test()
     for l in pairs_with_close() --[[io.lines("test.lua")]] do
         return tail()
     end
end

test()


I would expect the close method from pairs_with_close to be called but
somehow the close method in the tail function is invoked.

Regards,
Xmilia

[Gé Weijers]

On Wed, Mar 31, 2021 at 10:45 AM Xmilia Hermit <[hidden email]> wrote:

Hi,

I think I just found a bug regarding to-be-closed variables. For some
reason the forth to-be-closed variable of a for-in loop has problems to
close with a tailcall as can be seen in the following example.

 

local function test()
     for l in pairs_with_close() --[[io.lines("test.lua")]] do
         return tail()

The problem is that 'tail' is no longer a tail call. In the general case the file can only be closed after 'tail' returns, like in this convoluted example:

local f = assert(io.open("/usr/share/dict/words"))

local function read_next()
    return f:read("l")
end

for w in read_next, nil, nil, f do
    print(w)
    if (w == "cheese") then
        return tail(f)
    end
end

The program reads a dictionary until it finds the word "cheese", then calls a function 'tail' with the file.

(Would I code it like that? No, but it's legal.)

The simpler case:

local function test()
    local x <close> = some_object_with_a_close_metamethod()
    return tail()
end

Here 'tail' is not a tail call either, because the __close metamethod of 'x' has to be called after 'tail' returns.

[Xmilia Hermit]

On 31.03.2021 at 23:14 Gé Weijers wrote:

The problem is that 'tail' is no longer a tail call. In the general case the file can only be closed after 'tail' returns, like in this convoluted example:

Correct me if I'm wrong but at https://github.com/lua/lua/blob/master/lparser.c#L1814 the tailcall is inserted. For that fs->bl->insidetbc needs to be zero. It is set at https://github.com/lua/lua/blob/master/lparser.c#L1710 in checktoclose called from localstat but not in the for loop case at https://github.com/lua/lua/blob/master/lparser.c#L1602. In for loop case the fs->bl->insidetbc is never set and therfore the call is a tailcall.

Second, could you explain why commenting `return tail()` out in my previous example yields
> ./lua test.lua
This should be called

while the output with the `return tail()` is
> ./lua test.lua
Why is this called?

shouldn't they be the same?

Regards,
Xmilia

[Andrew Gierth]

>>>>> "Xmilia" == Xmilia Hermit <[hidden email]> writes:

 Xmilia> Hi,

 Xmilia> I think I just found a bug regarding to-be-closed variables.
 Xmilia> For some reason the forth to-be-closed variable of a for-in
 Xmilia> loop has problems to close with a tailcall as can be seen in
 Xmilia> the following example.

Yes, this is definitely a bug. I think the problem is that the compiler
is not recognizing that return tail() should not actually be a tail call
(since a toclose variable is in scope) and is compiling it as if it is,
which leaves the stack and its to-close marks in an inconsistent state.

Specifically the test() bytecode is:

function <l2.lua:17,21> (10 instructions at 0x800a32280)
0 params, 7 slots, 2 upvalues, 5 locals, 0 constants, 0 functions
        1       [18]    GETUPVAL        0 0     ; pairs_with_close
        2       [18]    CALL            0 1 5   ; 0 in 4 out
        3       [18]    TFORPREP        0 3     ; to 7
        4       [19]    GETUPVAL        5 1     ; tail
        5       [19]    TAILCALL        5 1 0   ; 0 in
        6       [19]    RETURN          5 0 0   ; all out
        7       [18]    TFORCALL        0 1
        8       [18]    TFORLOOP        0 5     ; to 4
        9       [20]    CLOSE           0
        10      [21]    RETURN          0 1 0   ; 0 out

This looks like a simple oversight in forlist() in lparser.c, which is
calling markupval on the close variable but not setting bl->insidetbc to
signal that a close var is in scope.

--
Andrew.

[Roberto Ierusalimschy]

>  Xmilia> I think I just found a bug regarding to-be-closed variables.
>  [...]
>
> Yes, this is definitely a bug. I think the problem is that the compiler
> is not recognizing that return tail() should not actually be a tail call
> [...]

Many thanks to Xmilia for the report and to Andrew for the detailed
analysis.

-- Roberto