Lua stack state during 'lua_load'

[Jure Bagić]

Reference manual for 'lua_load' says:
> The function lua_dump fully preserves the Lua stack
> through the calls to the writer function,
> except that it may push some values for internal use
> before the first call,
> and it restores the stack size to its original size
> after the last call.

How about the 'kcache' table that is pushed each time
new function is being parsed (see 'open_func' in lparser.c).
Now the reader cannot assume that the Lua stack is fully preserved
as it might of pushed a new 'kcache' table.

This assumption would break C code that interacts with Lua stack while
loading chunks in text mode; loading of binary chunks looks ok,
I guess the only changes here were the addition of PF_FIXED and
new external strings (no new stack anchors).

Updating docs is easy fix,
trying to preserve stack with the addition of 'kcache' table not so easy fix.

I do have 2 fixes.
Storing 'kcache' table not on stack but in the scanner table
or (worse option in my opinion) storing the 'kcache' tables in the
'mainfunc' 'kcache' table.

Storing 'kcache' in scanner table is faster (less checks),
requires one less stack slot and less code.
The parser would reserve one stack slot above the scanner table
that would be kept as nil.
It would act as temporary anchor for 'kcache' table before the
'kcache' is inserted into the scanner table (the table itself is a key).
When in 'close_func',
before the GC check the 'kcache' would be removed from the scanner table
(set to nil).

If using 'kcache' table of the 'mainfunc' to store other 'kcache' tables,
there would be additional check in 'end_func'
> if (fs->prev != NULL)
> -- remove 'kcache' from the main 'kcache'
not only that,
there would be need for additional stack slot,
the temporary anchor + the slot for 'mainfunc' 'kcache' table,
and it would also require pointer to the 'mainfunc' 'kcache' table.

-- Jure

[Jure Bagić]

> The parser would reserve one stack slot above the scanner table
> that would be kept as nil.
> It would act as temporary anchor for 'kcache' table before the
> 'kcache' is inserted into the scanner table (the table itself is a key).

Correction,
the parser needs no extra stack slot,
the anchor would be the current stack top as it would be
removed promptly (after insertion into scanner table)
before the next call to reader.
Reserving a stack slot would not be so trivial as lexer would
need to store offset in case stack changes,
but maybe it would prevent very rare cases where 'lua_load' would
be the cause of stack overflow if reader abuses the Lua stack.
However maybe EXTRA_STACK takes care of this.

--
Jure

[Roberto Ierusalimschy]

> Reference manual for 'lua_load' says:
> > The function lua_dump fully preserves the Lua stack
> > through the calls to the writer function,
> > except that it may push some values for internal use
> > before the first call,
> > and it restores the stack size to its original size
> > after the last call.

(You are talking about lua_load, but quoted the documentation about
lua_dump. Never mind...)

> How about the 'kcache' table that is pushed each time
> new function is being parsed (see 'open_func' in lparser.c).
> Now the reader cannot assume that the Lua stack is fully preserved
> as it might of pushed a new 'kcache' table.
>

> [...]

Many thanks for the feedback. Actually, the problem is worse than you
mentioned. lua_load starts by reading a first byte to detect whether
the chunk is text or binary, so everything it pushes (including the
scanner table) is being pushed after the first call to the reader function.

-- Roberto

[Jure Bagić]

> Many thanks for the feedback. Actually, the problem is worse than you
> mentioned. lua_load starts by reading a first byte to detect whether
> the chunk is text or binary, so everything it pushes (including the
> scanner table) is being pushed after the first call to the reader function.

Oh, right...
Luckily by this bug still being present it seems nobody was relying on this
assumption anyway.

--
Jure

[Jure Bagić]

> (You are talking about lua_load, but quoted the documentation about
> lua_dump. Never mind...)

ADHD

--
Jure