[Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for embedded bytecode loader.
Sergey Bronnikov
sergeyb at tarantool.org
Mon Sep 4 19:34:01 MSK 2023
Hi, Max
On 9/1/23 13:05, Maxim Kokryashkin via Tarantool-patches wrote:
> On Thu, Aug 31, 2023 at 02:32:14PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
>> From: Sergey Bronnikov <sergeyb at tarantool.org>
<snipped>
>> +
>> +/**
>> + * Function generates a huge chunk of "bytecode" with a size bigger than
>> + * LJ_MAX_BUF. Generated chunk must enable endmark in a Lex state.
>> + */
>> +static const char *
>> +bc_reader_with_endmark(lua_State *L, void *data, size_t *size)
>> +{
>> + UNUSED(data);
>> + int bc_chunk_size = (size_t)0;
>> + static char *bc_chunk = NULL;
>> + free(bc_chunk);
> What's the point of free here? Why the buffer is static?
Because callee (aka Reader) is responsible for buffer, reader
initializes it once
and then reuse.
> The block must exist until the reader function is called again.
> To signal the end of the chunk, the reader must return NULL or set
size to zero.
> The reader function may return pieces of any size greater than zero.
1. http://www.lua.org/manual/5.1/manual.html#lua_Reader
>> +
>> + bc_chunk = malloc(bc_chunk_size);
> Malloc of zero size doesn't seem to be the thing you wanted to do.
Right. Updated:
@@ -33,26 +33,9 @@ static const char *
bc_reader_with_endmark(lua_State *L, void *data, size_t *size)
{
UNUSED(data);
- int bc_chunk_size = (size_t)0;
- static char *bc_chunk = NULL;
- free(bc_chunk);
-
- bc_chunk = malloc(bc_chunk_size);
- assert(bc_chunk != NULL);
-
- /**
- * `lua_load` automatically detects whether the chunk is text or
binary,
- * and loads it accordingly. We need a trace for bytecode input,
- * so it is necessary to deceive a check in lj_lex_setup, that
- * makes a sanity check and detects whether input is bytecode or text
- * by the first char. Put LUA_SIGNATURE[0] at the beginning of the
- * allocated region.
- */
- bc_chunk[0] = LUA_SIGNATURE[0];
-
- *size = bc_chunk_size;
+ *size = ~(size_t)0;
- return bc_chunk;
+ return NULL;
}
static int bc_loader_with_endmark(void *test_state)
>> + assert(bc_chunk != NULL);
>> +
>> + /**
>> + * `lua_load` automatically detects whether the chunk is text or binary,
>> + * and loads it accordingly. We need a trace for bytecode input,
>> + * so it is necessary to deceive a check in lj_lex_setup, that
>> + * makes a sanity check and detects whether input is bytecode or text
>> + * by the first char. Put LUA_SIGNATURE[0] at the beginning of the
>> + * allocated region.
>> + */
>> + bc_chunk[0] = LUA_SIGNATURE[0];
>> +
>> + *size = bc_chunk_size;
>> +
>> + return bc_chunk;
>> +}
>> +
>> +static int bc_loader_with_endmark(void *test_state)
>> +{
>> + lua_State *L = test_state;
>> + void *ud = NULL;
>> + int res = lua_load(L, bc_reader_with_endmark, ud, "endmark");
>> +
>> + /*
>> + * Make sure we passed the condition with lj_err_mem in the function
>> + * `lex_more`.
>> + */
>> + assert_true(res != LUA_ERRMEM);
>> +
>> + return TEST_EXIT_SUCCESS;
>> +}
>> +
>> +enum bc_emission_state {
>> + EMIT_BC,
>> + EMIT_EOF,
>> +};
>> +
>> +typedef struct {
>> + enum bc_emission_state state;
>> +} dt;
>> +
>> +/**
>> + * Function returns a bytecode chunk on the first call and NULL and size equal
>> + * to zero on the second call. Triggers the END_OF_STREAM flag in the function
>> + * `lex_more`.
>> + */
>> +static const char *
>> +bc_reader_with_eof(lua_State *L, void *data, size_t *size)
>> +{
>> + UNUSED(data);
>> + UNUSED(L);
>> + dt *test_data = (dt *)data;
>> + if (test_data->state == EMIT_EOF) {
> This section is unreachable, isn't it?
Right, fixed it.
>> + *size = 0;
>> + return NULL;
>> + }
>> +
>> + static char *bc_chunk = NULL;
>> + free(bc_chunk);
> Ditto.
>> +
>> + size_t sz = 10;
> Is there any reason for it to be exactly 10? Drop a comment.
Set it to 2 and dropped a comment.
@@ -98,10 +81,23 @@ bc_reader_with_eof(lua_State *L, void *data, size_t
*size)
static char *bc_chunk = NULL;
free(bc_chunk);
- size_t sz = 10;
+ /**
+ * Minimal size of a buffer with bytecode:
+ * signiture (1 byte) and a bytecode itself (1 byte).
+ */
+ size_t sz = 2;
bc_chunk = malloc(sz);
+ /**
+ * `lua_load` automatically detects whether the chunk is text or
binary,
+ * and loads it accordingly. We need a trace for *bytecode* input,
+ * so it is necessary to deceive a check in `lj_lex_setup`, that
+ * makes a sanity check and detects whether input is bytecode or text
+ * by the first char. Put `LUA_SIGNATURE[0]` at the beginning of the
+ * allocated region.
+ */
bc_chunk[0] = LUA_SIGNATURE[0];
*size = sz;
+ test_data->state = EMIT_EOF;
return bc_chunk;
}
<snipped>
More information about the Tarantool-patches
mailing list