From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id DE72E6EC5F; Wed, 21 Apr 2021 15:34:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DE72E6EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1619008478; bh=I3rTY4af2CNa+tsu2qzB1mCAuj3/KBCPEsibkjyaA1E=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=mjtkWCcDbu+db2oSuMV8myknlafQ49QNAjXgfvG7ZVkdUrq9QwRnjQfv+/AcJMkSB O+vBgkWlyd00ZTe1GHUMn/HrT2fIT9B2uA44eKf/t7DS7NN8hi+Zifqv+iMsBssX/o NVATbfBATnU9KMviVq1B6E934XEdwiNdqKQv8E0M= Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B8E236EC5F for ; Wed, 21 Apr 2021 15:34:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B8E236EC5F Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lZC3w-00071J-P0; Wed, 21 Apr 2021 15:34:37 +0300 To: tarantool-patches@dev.tarantool.org, roman Khabibov References: <20210311054949.91263-1-roman.habibov@tarantool.org> Message-ID: <67243305-a17e-0923-bbb0-322c3506841c@tarantool.org> Date: Wed, 21 Apr 2021 15:34:36 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210311054949.91263-1-roman.habibov@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9DD7F0C977691F2B1028A1B9238D46FC7D6DD4EEAEDFDB30C182A05F538085040E6633DD4A6806AD97129E6CDDD2BD8476C76ACCF534CFD966061A0C00338FB57 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F2919D563845004AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F7C2A16C2A438FE58638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2977A987FBAEEB043A7BCFDAA694E3557AA867293B0326636D2E47CDBA5A96583C09775C1D3CA48CFCA5A41EBD8A3A0199FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C317B107DEF921CE79117882F4460429728AD0CFFFB425014E868A13BD56FB6657E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637A2D17A18B0C1A664EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A5ECDEAA55959CA57EF591EA20A329CDBC2866E7DE80C347CCD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34C974B02B4EA30DFB4C725D86CDAB88B7E7FB4200A2849D230BABE675EFEFE9C6E6F85505D04B43A61D7E09C32AA3244C148481447ACC8C572E6C6D476A7283C7B038C9161EF167A1729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojd3ipC3Yuge0unS/SIio48A== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822E1A06DD55AAE63169363410468DC04C7DD788429FD8613638ED9BB8B05EE7B3AFB559BB5D741EB96D19CD4E7312BAA970A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] serializer: serialize recursive structures X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hello, seems patch series below is a bit outdated, so I'll review and quote actual version in a branch. >    @TarantoolBot >    __serialize parameter >    If __serialize parameter is initialized as a function then this >    function should be pure. A pure function is a function which >    returns identical values for identical arguments (no variation >    with local static variables, non-local variables, mutable >    reference arguments or input streams). Otherwise, the behavior >    is undefined. 1. __serialize is a method, not parameter --- /dev/null +++ b/changelogs/unreleased/serializer-bug.md @@ -0,0 +1,4 @@ +## bugfix/core + +* Recursive structures appeared after __serialize call don't lead +* to segfault anymore in lua and yaml serializers (gh-5392). 2. seems issue number is wrong, proper one is #3228 3. I have reviewed a code coverage and propose to cover uncovered branches in lua_field_try_serialize(), see [1] I believe it would be enough to add more examples to proposed tests to enhance coverage. 1. https://coveralls.io/builds/38864750/source?filename=src%2Flua%2Futils.c On 11.03.2021 08:49, Roman Khabibov via Tarantool-patches wrote: > Fix bug with bus error during serializing of recursive > structures. > > Closes #3228 > --- > > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3228-visited-set > Issue: https://github.com/tarantool/tarantool/issues/3228 > > src/box/lua/call.c | 6 +- > src/box/lua/execute.c | 2 +- > src/box/lua/serialize_lua.c | 96 ++------ > src/box/lua/tuple.c | 2 +- > src/box/sql/func.c | 2 +- > src/lua/msgpack.c | 10 +- > src/lua/pickle.c | 2 +- > src/lua/utils.c | 226 ++++++++++++++++-- > src/lua/utils.h | 43 +++- > ...-3228-serializer-look-for-recursion.result | 67 ++++++ > ...228-serializer-look-for-recursion.test.lua | 17 ++ > test/swim/swim.result | 18 +- > third_party/lua-cjson/lua_cjson.c | 4 +- > third_party/lua-yaml/lyaml.cc | 88 +++---- > 14 files changed, 390 insertions(+), 193 deletions(-) > create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result > create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua > > diff --git a/src/box/lua/call.c b/src/box/lua/call.c > index 0315e720c..e4907a3f7 100644 > --- a/src/box/lua/call.c > +++ b/src/box/lua/call.c > @@ -193,7 +193,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg, > */ > for (int i = 1; i <= nrets; ++i) { > struct luaL_field field; > - if (luaL_tofield(L, cfg, NULL, i, &field) < 0) > + if (luaL_tofield(L, cfg, 0, NULL, i, &field) < 0) > return luaT_error(L); > struct tuple *tuple; > if (field.type == MP_EXT && > @@ -222,7 +222,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg, > * Inspect the first result > */ > struct luaL_field root; > - if (luaL_tofield(L, cfg, NULL, 1, &root) < 0) > + if (luaL_tofield(L, cfg, 0, NULL, 1, &root) < 0) > return luaT_error(L); > struct tuple *tuple; > if (root.type == MP_EXT && (tuple = luaT_istuple(L, 1)) != NULL) { > @@ -252,7 +252,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg, > for (uint32_t t = 1; t <= root.size; t++) { > lua_rawgeti(L, 1, t); > struct luaL_field field; > - if (luaL_tofield(L, cfg, NULL, -1, &field) < 0) > + if (luaL_tofield(L, cfg, 0, NULL, -1, &field) < 0) > return luaT_error(L); > if (field.type == MP_EXT && (tuple = luaT_istuple(L, -1))) { > tuple_to_mpstream(tuple, stream); > diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c > index 926a0a61c..76d271f5c 100644 > --- a/src/box/lua/execute.c > +++ b/src/box/lua/execute.c > @@ -328,7 +328,7 @@ lua_sql_bind_decode(struct lua_State *L, struct sql_bind *bind, int idx, int i) > bind->name = NULL; > bind->name_len = 0; > } > - if (luaL_tofield(L, luaL_msgpack_default, NULL, -1, &field) < 0) > + if (luaL_tofield(L, luaL_msgpack_default, 0, NULL, -1, &field) < 0) > return -1; > switch (field.type) { > case MP_UINT: > diff --git a/src/box/lua/serialize_lua.c b/src/box/lua/serialize_lua.c > index caa08a60f..ab612dfc2 100644 > --- a/src/box/lua/serialize_lua.c > +++ b/src/box/lua/serialize_lua.c > @@ -215,7 +215,7 @@ trace_node(struct lua_dumper *d) > struct luaL_field field; > > memset(&field, 0, sizeof(field)); > - luaL_checkfield(d->L, d->cfg, lua_gettop(d->L), &field); > + luaL_checkfield(d->L, d->cfg, 0, lua_gettop(d->L), &field); > > if (field.type < lengthof(mp_type_names)) { > if (field.type == MP_EXT) { > @@ -234,7 +234,7 @@ trace_node(struct lua_dumper *d) > > memset(&field, 0, sizeof(field)); > > - luaL_checkfield(d->L, d->cfg, top, &field); > + luaL_checkfield(d->L, d->cfg, 0, top, &field); > say_info("serializer-trace: node :\tfield type %s (%d)", > type_str, field.type); > } > @@ -339,41 +339,16 @@ emit_node(struct lua_dumper *d, struct node *nd, int indent, > static const char * > get_lua_anchor(struct lua_dumper *d) > { > - const char *s = ""; > - > - lua_pushvalue(d->L, -1); > - lua_rawget(d->L, d->anchortable_index); > - if (!lua_toboolean(d->L, -1)) { > - lua_pop(d->L, 1); > - return NULL; > - } > - > - if (lua_isboolean(d->L, -1)) { > - /* > - * This element is referenced more > - * than once but has not been named. > - */ > - char buf[32]; > - snprintf(buf, sizeof(buf), "%u", d->anchor_number++); > - lua_pop(d->L, 1); > - lua_pushvalue(d->L, -1); > - lua_pushstring(d->L, buf); > - s = lua_tostring(d->L, -1); > - lua_rawset(d->L, d->anchortable_index); > - trace_anchor(s, false); > - } else { > - /* > - * An aliased element. > - * > - * FIXME: Need an example to use. > - * > - * const char *str = lua_tostring(d->L, -1); > - */ > - const char *str = lua_tostring(d->L, -1); > - trace_anchor(str, true); > - lua_pop(d->L, 1); > + const char *anchor = NULL; > + int code = get_anchor(d->L, d->anchortable_index, &d->anchor_number, > + &anchor); > + if (code == 1) { > + trace_anchor(anchor, false); > + } else if (code == 2) { > + trace_anchor(anchor, true); > + return ""; > } > - return s; > + return anchor; > } > > static void > @@ -783,7 +758,7 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent) > return -1; > > memset(field, 0, sizeof(*field)); > - luaL_checkfield(d->L, d->cfg, lua_gettop(d->L), field); > + luaL_checkfield(d->L, d->cfg, 0, lua_gettop(d->L), field); > > switch (field->type) { > case MP_NIL: > @@ -881,49 +856,6 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent) > return emit_node(d, nd, indent, str, len); > } > > -/** > - * Find references to tables, we use it > - * to find self references in tables. > - */ > -static void > -find_references(struct lua_dumper *d) > -{ > - int newval; > - > - if (lua_type(d->L, -1) != LUA_TTABLE) > - return; > - > - /* Copy of a table for self refs */ > - lua_pushvalue(d->L, -1); > - lua_rawget(d->L, d->anchortable_index); > - if (lua_isnil(d->L, -1)) > - newval = 0; > - else if (!lua_toboolean(d->L, -1)) > - newval = 1; > - else > - newval = -1; > - lua_pop(d->L, 1); > - > - if (newval != -1) { > - lua_pushvalue(d->L, -1); > - lua_pushboolean(d->L, newval); > - lua_rawset(d->L, d->anchortable_index); > - } > - > - if (newval != 0) > - return; > - > - /* > - * Other values and keys in the table > - */ > - lua_pushnil(d->L); > - while (lua_next(d->L, -2) != 0) { > - find_references(d); > - lua_pop(d->L, 1); > - find_references(d); > - } > -} > - > /** > * Dump recursively from the root node. > */ > @@ -935,7 +867,7 @@ dump_root(struct lua_dumper *d) > }; > int ret; > > - luaL_checkfield(d->L, d->cfg, lua_gettop(d->L), &nd.field); > + luaL_checkfield(d->L, d->cfg, 0, lua_gettop(d->L), &nd.field); > > if (nd.field.type != MP_ARRAY || nd.field.size != 1) { > d->err = EINVAL; > @@ -984,7 +916,7 @@ lua_encode(lua_State *L, struct luaL_serializer *serializer, > > /* Push copy of arg we're processing */ > lua_pushvalue(L, 1); > - find_references(&dumper); > + find_references(dumper.L, dumper.anchortable_index, "before"); > > if (dump_root(&dumper) != 0) > goto out; > diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c > index 3e6f043b4..4d417ec34 100644 > --- a/src/box/lua/tuple.c > +++ b/src/box/lua/tuple.c > @@ -411,7 +411,7 @@ luamp_convert_key(struct lua_State *L, struct luaL_serializer *cfg, > return tuple_to_mpstream(tuple, stream); > > struct luaL_field field; > - if (luaL_tofield(L, cfg, NULL, index, &field) < 0) > + if (luaL_tofield(L, cfg, 0, NULL, index, &field) < 0) > luaT_error(L); > if (field.type == MP_ARRAY) { > lua_pushvalue(L, index); > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > index f15d27051..d4a7064f0 100644 > --- a/src/box/sql/func.c > +++ b/src/box/sql/func.c > @@ -262,7 +262,7 @@ port_lua_get_vdbemem(struct port *base, uint32_t *size) > return NULL; > for (int i = 0; i < argc; i++) { > struct luaL_field field; > - if (luaL_tofield(L, luaL_msgpack_default, > + if (luaL_tofield(L, luaL_msgpack_default, 0, > NULL, -1 - i, &field) < 0) { > goto error; > } > diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c > index 24b0d2ccd..412736572 100644 > --- a/src/lua/msgpack.c > +++ b/src/lua/msgpack.c > @@ -156,11 +156,11 @@ restart: /* used by MP_EXT of unidentified subtype */ > lua_pushnil(L); /* first key */ > while (lua_next(L, top) != 0) { > lua_pushvalue(L, -2); /* push a copy of key to top */ > - if (luaL_tofield(L, cfg, opts, lua_gettop(L), field) < 0) > + if (luaL_tofield(L, cfg, 0, opts, lua_gettop(L), field) < 0) > return luaT_error(L); > luamp_encode_r(L, cfg, opts, stream, field, level + 1); > lua_pop(L, 1); /* pop a copy of key */ > - if (luaL_tofield(L, cfg, opts, lua_gettop(L), field) < 0) > + if (luaL_tofield(L, cfg, 0, opts, lua_gettop(L), field) < 0) > return luaT_error(L); > luamp_encode_r(L, cfg, opts, stream, field, level + 1); > lua_pop(L, 1); /* pop value */ > @@ -181,7 +181,7 @@ restart: /* used by MP_EXT of unidentified subtype */ > mpstream_encode_array(stream, size); > for (uint32_t i = 0; i < size; i++) { > lua_rawgeti(L, top, i + 1); > - if (luaL_tofield(L, cfg, opts, top + 1, field) < 0) > + if (luaL_tofield(L, cfg, 0, opts, top + 1, field) < 0) > return luaT_error(L); > luamp_encode_r(L, cfg, opts, stream, field, level + 1); > lua_pop(L, 1); > @@ -204,7 +204,7 @@ restart: /* used by MP_EXT of unidentified subtype */ > if (type != MP_EXT) > return type; /* Value has been packed by the trigger */ > /* Try to convert value to serializable type */ > - luaL_convertfield(L, cfg, top, field); > + luaL_convertfield(L, cfg, 0, top, field); > /* handled by luaL_convertfield */ > assert(field->type != MP_EXT); > assert(lua_gettop(L) == top); > @@ -229,7 +229,7 @@ luamp_encode(struct lua_State *L, struct luaL_serializer *cfg, > } > > struct luaL_field field; > - if (luaL_tofield(L, cfg, opts, lua_gettop(L), &field) < 0) > + if (luaL_tofield(L, cfg, 0, opts, lua_gettop(L), &field) < 0) > return luaT_error(L); > enum mp_type top_type = luamp_encode_r(L, cfg, opts, stream, &field, 0); > > diff --git a/src/lua/pickle.c b/src/lua/pickle.c > index 65208b5b3..86f0f77e8 100644 > --- a/src/lua/pickle.c > +++ b/src/lua/pickle.c > @@ -80,7 +80,7 @@ lbox_pack(struct lua_State *L) > if (i > nargs) > luaL_error(L, "pickle.pack: argument count does not match " > "the format"); > - luaL_checkfield(L, luaL_msgpack_default, i, &field); > + luaL_checkfield(L, luaL_msgpack_default, 0, i, &field); > switch (*format) { > case 'B': > case 'b': > diff --git a/src/lua/utils.c b/src/lua/utils.c > index b5a6ca5b7..64404e3fc 100644 > --- a/src/lua/utils.c > +++ b/src/lua/utils.c > @@ -439,9 +439,54 @@ lua_gettable_wrapper(lua_State *L) > return 1; > } > > +/** > + * Check if node with @a idx serialized. > + */ > +static int > +is_serialized(struct lua_State *L, int anchortable_index, int idx) > +{ > + if (anchortable_index == 0) > + return 0; > + lua_pushvalue(L, idx); > + lua_rawget(L, anchortable_index); > + int is_serialized = 0; > + if (lua_isnil(L, -1) == 0) { > + lua_pushstring(L, "serialized"); > + lua_rawget(L, -2); > + is_serialized = lua_toboolean(L, -1); > + lua_pop(L, 1); > + } > + lua_pop(L, 1); > + return is_serialized; > +} > + > +/** > + * Mark node with @a idx as serialized. > + */ > +static void > +mark_as_serialized(struct lua_State *L, int anchortable_index, int idx) > +{ > + if (anchortable_index == 0) > + return; > + /* Push copy of table. */ > + lua_pushvalue(L, idx); > + lua_pushvalue(L, idx); > + lua_rawget(L, anchortable_index); > + if (lua_isnil(L, -1) == 1) { > + lua_pop(L, 1); > + lua_newtable(L); > + } > + int flags_index = lua_gettop(L); > + lua_pushstring(L, "serialized"); > + lua_pushboolean(L, true); > + lua_rawset(L, flags_index); > + lua_rawset(L, anchortable_index); > +} > + > static void > lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg, > - int idx, struct luaL_field *field) > + int anchortable_index, int idx, > + struct luaL_field *field) > { > if (!cfg->encode_load_metatables) > return; > @@ -463,12 +508,148 @@ lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg, > lua_pcall(L, 1, 1, 0); > /* replace obj with the unpacked value */ > lua_replace(L, idx); > - if (luaL_tofield(L, cfg, NULL, idx, field) < 0) > + if (luaL_tofield(L, cfg, anchortable_index, NULL, idx, > + field) < 0) > luaT_error(L); > } /* else ignore lua_gettable exceptions */ > lua_settop(L, top); /* remove temporary objects */ > } > > +void > +find_references(struct lua_State *L, int anchortable_index, const char *mode) > +{ > + int type = lua_type(L, -1); > + if (type != LUA_TTABLE || anchortable_index == 0) > + return; > + > + int node_index = lua_gettop(L); > + lua_pushvalue(L, node_index); > + > + /* Get value with flags from anchor table. */ > + lua_pushvalue(L, node_index); > + lua_rawget(L, anchortable_index); > + if (lua_isnil(L, -1) == 1) { > + lua_pop(L, 1); > + lua_newtable(L); > + } > + int flags_index = lua_gettop(L); > + > + /* Get and increment counter. */ > + lua_pushstring(L, mode); > + lua_rawget(L, flags_index); > + > + int new_count = 0; > + if (lua_isnil(L, -1) == 1) { > + new_count = 1; > + lua_pop(L, 1); > + } else { > + lua_Integer number = lua_tointeger(L, -1); > + lua_pop(L, 1); > + if (number < 2) > + new_count = ++number; > + else > + new_count = -1; > + } > + > + /* Push new count to value with flags. */ > + if (new_count == -1) { > + lua_pop(L, 2); > + assert(node_index == lua_gettop(L)); > + return; > + } > + lua_pushstring(L, mode); > + lua_pushinteger(L, new_count); > + lua_rawset(L, flags_index); > + > + /* Check if node is already serialized. */ > + bool already_serialized = false; > + if (strcmp(mode, "after") == 0) { > + lua_pushstring(L, "serialized"); > + lua_rawget(L, flags_index); > + already_serialized = lua_toboolean(L, -1); > + lua_pop(L, 1); > + } > + lua_rawset(L, anchortable_index); > + assert(node_index == lua_gettop(L)); > + if (already_serialized == true) > + return; > + > + /* Recursively process other table values. */ > + lua_pushnil(L); > + while (lua_next(L, -2) != 0) { > + /* Find references on value. */ > + find_references(L, anchortable_index, mode); > + lua_pop(L, 1); > + /* Find references on key. */ > + find_references(L, anchortable_index, mode); > + } > +} > + > +int > +get_anchor(struct lua_State *L, int anchortable_index, > + unsigned int *anchor_number, const char **anchor) > +{ > + lua_pushvalue(L, -1); > + lua_rawget(L, anchortable_index); > + if (lua_isnil(L, -1) == 1) { > + lua_pop(L, 1); > + *anchor = NULL; > + return 0; > + } > + > + lua_pushstring(L, "before"); > + lua_rawget(L, -2); > + const char *str = NULL; > + if (lua_type(L, -1) == LUA_TSTRING) > + str = lua_tostring(L, -1); > + lua_Integer number_before = lua_tointeger(L, -1); > + lua_pop(L, 1); > + lua_Integer number_after = 0; > + if (str == NULL) { > + lua_pushstring(L, "after"); > + lua_rawget(L, -2); > + if (lua_type(L, -1) == LUA_TSTRING) > + str = lua_tostring(L, -1); > + number_after = lua_tointeger(L, -1); > + lua_pop(L, 1); > + } > + > + *anchor = ""; > + int ret = 0; > + if ((number_before > 1 || number_after > 1) && str == NULL) { > + /* > + * This element is referenced more than once but > + * has not been named. > + */ > + char buf[32]; > + snprintf(buf, sizeof(buf), "%u", (*anchor_number)++); > + lua_pop(L, 1); > + lua_pushvalue(L, -1); > + > + lua_pushvalue(L, -1); > + lua_rawget(L, anchortable_index); > + if (number_before > 1) > + lua_pushstring(L, "before"); > + else > + lua_pushstring(L, "after"); > + lua_pushstring(L, buf); > + *anchor = lua_tostring(L, -1); > + lua_rawset(L, -3); > + lua_rawset(L, anchortable_index); > + ret = 1; > + } else if (str != NULL) { > + /* This is an aliased element. */ > + lua_pop(L, 1); > + *anchor = str; > + ret = 2; > + } else { > + lua_pop(L, 1); > + *anchor = NULL; > + ret = 0; > + } > + return ret; > +} > + > /** > * Call __serialize method of a table object by index > * if the former exists. > @@ -496,11 +677,13 @@ lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg, > * proceed with default table encoding. > */ > static int > -lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, > - int idx, struct luaL_field *field) > +lua_field_try_serialize(struct lua_State *L, int anchortable_index, > + struct luaL_serializer *cfg, int idx, > + struct luaL_field *field) > { > if (luaL_getmetafield(L, idx, LUAL_SERIALIZE) == 0) > return 1; > + mark_as_serialized(L, anchortable_index, idx); > if (lua_isfunction(L, -1)) { > /* copy object itself */ > lua_pushvalue(L, idx); > @@ -508,7 +691,9 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, > diag_set(LuajitError, lua_tostring(L, -1)); > return -1; > } > - if (luaL_tofield(L, cfg, NULL, -1, field) != 0) > + find_references(L, anchortable_index, "after"); > + if (luaL_tofield(L, cfg, anchortable_index, NULL, -1, > + field) != 0) > return -1; > lua_replace(L, idx); > return 0; > @@ -541,16 +726,19 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, > } > > static int > -lua_field_inspect_table(struct lua_State *L, struct luaL_serializer *cfg, > - int idx, struct luaL_field *field) > +lua_field_inspect_table(struct lua_State *L, int anchortable_index, > + struct luaL_serializer *cfg, int idx, > + struct luaL_field *field) > { > assert(lua_type(L, idx) == LUA_TTABLE); > uint32_t size = 0; > uint32_t max = 0; > > - if (cfg->encode_load_metatables) { > + if (cfg->encode_load_metatables && > + is_serialized(L, anchortable_index, idx) != 1) { > int top = lua_gettop(L); > - int res = lua_field_try_serialize(L, cfg, idx, field); > + int res = lua_field_try_serialize(L, anchortable_index, cfg, > + idx, field); > if (res == -1) > return -1; > assert(lua_gettop(L) == top); > @@ -612,14 +800,14 @@ lua_field_tostring(struct lua_State *L, struct luaL_serializer *cfg, int idx, > lua_call(L, 1, 1); > lua_replace(L, idx); > lua_settop(L, top); > - if (luaL_tofield(L, cfg, NULL, idx, field) < 0) > + if (luaL_tofield(L, cfg, 0, NULL, idx, field) < 0) > luaT_error(L); > } > > int > luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, > - const struct serializer_opts *opts, int index, > - struct luaL_field *field) > + int anchortable_index, const struct serializer_opts *opts, > + int index, struct luaL_field *field) > { > if (index < 0) > index = lua_gettop(L) + index + 1; > @@ -753,7 +941,8 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, > case LUA_TTABLE: > { > field->compact = false; > - return lua_field_inspect_table(L, cfg, index, field); > + return lua_field_inspect_table(L, anchortable_index, cfg, > + index, field); > } > case LUA_TLIGHTUSERDATA: > case LUA_TUSERDATA: > @@ -773,8 +962,8 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, > } > > void > -luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx, > - struct luaL_field *field) > +luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, > + int anchortable_index, int idx, struct luaL_field *field) > { > if (idx < 0) > idx = lua_gettop(L) + idx + 1; > @@ -789,9 +978,12 @@ luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx, > */ > GCcdata *cd = cdataV(L->base + idx - 1); > if (cd->ctypeid > CTID_CTYPEID) > - lua_field_inspect_ucdata(L, cfg, idx, field); > + lua_field_inspect_ucdata(L, cfg, > + anchortable_index, idx, > + field); > } else if (type == LUA_TUSERDATA) { > - lua_field_inspect_ucdata(L, cfg, idx, field); > + lua_field_inspect_ucdata(L, cfg, anchortable_index, idx, > + field); > } > } > > diff --git a/src/lua/utils.h b/src/lua/utils.h > index 37531676d..ef9c70dfb 100644 > --- a/src/lua/utils.h > +++ b/src/lua/utils.h > @@ -354,6 +354,29 @@ struct luaL_field { > bool compact; /* a flag used by YAML serializer */ > }; > > +/** > + * Find references to tables to look up self references in tables. > + * > + * @param L Lua stack. > + * @param anchortable_index Index of anchor table on stack. > + * @param mode When the function is called before or after dump > + function. > + */ > +void > +find_references(struct lua_State *L, int anchortable_index, const char *mode); > + > +/** > + * Generate aliases and anchor numbers for self references. > + * > + * @param L Lua stack. > + * @param anchortable_index Index of anchor table on stack. > + * @param[out] anchor_number Ptr to the number anchors. > + * @param[out] anchor Ptr to anchor string. > + */ > +int > +get_anchor(struct lua_State *L, int anchortable_index, > + unsigned int *anchor_number, const char **anchor); > + > /** > * @brief Convert a value from the Lua stack to a lua_field structure. > * This function is designed for use with Lua bindings and data > @@ -388,6 +411,7 @@ struct luaL_field { > * > * @param L stack > * @param cfg configuration > + * @param anchortable_index Index of anchor table on stack. > * @param opts the Lua serializer additional options. > * @param index stack index > * @param field conversion result > @@ -397,8 +421,8 @@ struct luaL_field { > */ > int > luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, > - const struct serializer_opts *opts, int index, > - struct luaL_field *field); > + int anchortable_index, const struct serializer_opts *opts, > + int index, struct luaL_field *field); > > /** > * @brief Try to convert userdata/cdata values using defined conversion logic. > @@ -406,18 +430,21 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, > * > * @param L stack > * @param cfg configuration > + * @param anchortable_index Index of anchor table on stack. > * @param idx stack index > * @param field conversion result > */ > void > -luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx, > - struct luaL_field *field); > +luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, > + int anchortable_index, int idx, struct luaL_field *field); > > /** > * @brief A wrapper for luaL_tofield() and luaL_convertfield() that > * tries to convert value or raise an error. > * @param L stack > * @param cfg configuration > + * @param anchortable_index Stack index of table with node visit > + and serialization info. > * @param idx stack index > * @param field conversion result > * @sa lua_tofield() > @@ -433,14 +460,14 @@ luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx, > * (tostring) -> (nil) -> exception > */ > static inline void > -luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, int idx, > - struct luaL_field *field) > +luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, > + int anchortable_index, int idx, struct luaL_field *field) > { > - if (luaL_tofield(L, cfg, NULL, idx, field) < 0) > + if (luaL_tofield(L, cfg, anchortable_index, NULL, idx, field) < 0) > luaT_error(L); > if (field->type != MP_EXT || field->ext_type != MP_UNKNOWN_EXTENSION) > return; > - luaL_convertfield(L, cfg, idx, field); > + luaL_convertfield(L, cfg, anchortable_index, idx, field); > } > > void > diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result > new file mode 100644 > index 000000000..773470c8e > --- /dev/null > +++ b/test/app/gh-3228-serializer-look-for-recursion.result > @@ -0,0 +1,67 @@ > +-- test-run result file version 2 > +test_run = require('test_run').new() > + | --- > + | ... > + > +-- > +-- gh-3228: Check the error message in the case of a __serialize > +-- function generating infinite recursion. > +-- > +setmetatable({}, {__serialize = function(a) return a end}) > + | --- > + | - [] > + | ... > +setmetatable({}, {__serialize = function(a) return {a} end}) > + | --- > + | - - [] > + | ... > +setmetatable({}, {__serialize = function(a) return {a, a} end}) > + | --- > + | - - &0 [] > + | - *0 > + | ... > +setmetatable({}, {__serialize = function(a) return {a, a, a} end}) > + | --- > + | - - &0 [] > + | - *0 > + | - *0 > + | ... > +setmetatable({}, {__serialize = function(a) return {{a, a}, a} end}) > + | --- > + | - - - &0 [] > + | - *0 > + | - *0 > + | ... > +setmetatable({}, {__serialize = function(a) return {a, 1} end}) > + | --- > + | - - [] > + | - 1 > + | ... > +setmetatable({}, {__serialize = function(a) return {{{{a}}}} end}) > + | --- > + | - - - - - [] > + | ... > +setmetatable({}, {__serialize = function(a) return {{{{1}}}} end}) > + | --- > + | - - - - - 1 > + | ... > +b = {} > + | --- > + | ... > +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = a, b_1 = b, b_2 = b} end}) > + | --- > + | - b_2: &0 [] > + | a_2: &1 > + | - *0 > + | a_1: *1 > + | b_1: *0 > + | ... > +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = {a, b}, b = b} end}) > + | --- > + | - b: &0 [] > + | a_2: > + | - &1 > + | - *0 > + | - *0 > + | a_1: *1 > + | ... > diff --git a/test/app/gh-3228-serializer-look-for-recursion.test.lua b/test/app/gh-3228-serializer-look-for-recursion.test.lua > new file mode 100644 > index 000000000..1c9b0375f > --- /dev/null > +++ b/test/app/gh-3228-serializer-look-for-recursion.test.lua > @@ -0,0 +1,17 @@ > +test_run = require('test_run').new() > + > +-- > +-- gh-3228: Check the error message in the case of a __serialize > +-- function generating infinite recursion. > +-- > +setmetatable({}, {__serialize = function(a) return a end}) > +setmetatable({}, {__serialize = function(a) return {a} end}) > +setmetatable({}, {__serialize = function(a) return {a, a} end}) > +setmetatable({}, {__serialize = function(a) return {a, a, a} end}) > +setmetatable({}, {__serialize = function(a) return {{a, a}, a} end}) > +setmetatable({}, {__serialize = function(a) return {a, 1} end}) > +setmetatable({}, {__serialize = function(a) return {{{{a}}}} end}) > +setmetatable({}, {__serialize = function(a) return {{{{1}}}} end}) > +b = {} > +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = a, b_1 = b, b_2 = b} end}) > +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = {a, b}, b = b} end}) > diff --git a/test/swim/swim.result b/test/swim/swim.result > index bfc83c143..539131677 100644 > --- a/test/swim/swim.result > +++ b/test/swim/swim.result > @@ -1322,16 +1322,13 @@ m_list > incarnation: cdata {generation = 0ULL, version = 1ULL} > uuid: 00000000-0000-1000-8000-000000000002 > payload_size: 0 > - - uri: 127.0.0.1: > - status: alive > - incarnation: cdata {generation = 0ULL, version = 2ULL} > - uuid: 00000000-0000-1000-8000-000000000001 > - payload_size: 8 > - - uri: 127.0.0.1: > + - &0 > + uri: 127.0.0.1: > status: alive > incarnation: cdata {generation = 0ULL, version = 2ULL} > uuid: 00000000-0000-1000-8000-000000000001 > payload_size: 8 > + - *0 > ... > e_list > --- > @@ -1374,16 +1371,13 @@ fiber.sleep(0) > -- Two events - status update to 'left', and 'drop'. > m_list > --- > -- - uri: 127.0.0.1: > - status: left > - incarnation: cdata {generation = 0ULL, version = 1ULL} > - uuid: 00000000-0000-1000-8000-000000000002 > - payload_size: 0 > - - uri: 127.0.0.1: > +- - &0 > + uri: 127.0.0.1: > status: left > incarnation: cdata {generation = 0ULL, version = 1ULL} > uuid: 00000000-0000-1000-8000-000000000002 > payload_size: 0 > + - *0 > ... > e_list > --- > diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c > index 38e999870..67a9762bc 100644 > --- a/third_party/lua-cjson/lua_cjson.c > +++ b/third_party/lua-cjson/lua_cjson.c > @@ -354,7 +354,7 @@ static void json_append_object(lua_State *l, struct luaL_serializer *cfg, > comma = 1; > > struct luaL_field field; > - luaL_checkfield(l, cfg, -2, &field); > + luaL_checkfield(l, cfg, 0, -2, &field); > if (field.type == MP_UINT) { > strbuf_append_char(json, '"'); > json_append_uint(cfg, json, field.ival); > @@ -384,7 +384,7 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg, > int current_depth, strbuf_t *json) > { > struct luaL_field field; > - luaL_checkfield(l, cfg, -1, &field); > + luaL_checkfield(l, cfg, 0, -1, &field); > switch (field.type) { > case MP_UINT: > return json_append_uint(cfg, json, field.ival); > diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc > index 9c3a4a646..72f269e6c 100644 > --- a/third_party/lua-yaml/lyaml.cc > +++ b/third_party/lua-yaml/lyaml.cc > @@ -500,38 +500,23 @@ usage_error: > static int dump_node(struct lua_yaml_dumper *dumper); > > static yaml_char_t *get_yaml_anchor(struct lua_yaml_dumper *dumper) { > - const char *s = ""; > - lua_pushvalue(dumper->L, -1); > - lua_rawget(dumper->L, dumper->anchortable_index); > - if (!lua_toboolean(dumper->L, -1)) { > - lua_pop(dumper->L, 1); > - return NULL; > - } > - > - if (lua_isboolean(dumper->L, -1)) { > - /* this element is referenced more than once but has not been named */ > - char buf[32]; > - snprintf(buf, sizeof(buf), "%u", dumper->anchor_number++); > - lua_pop(dumper->L, 1); > - lua_pushvalue(dumper->L, -1); > - lua_pushstring(dumper->L, buf); > - s = lua_tostring(dumper->L, -1); > - lua_rawset(dumper->L, dumper->anchortable_index); > - } else { > - /* this is an aliased element */ > + const char *anchor = NULL; > + if (get_anchor(dumper->L, dumper->anchortable_index, &dumper->anchor_number, > + &anchor) == 2) { > yaml_event_t ev; > - const char *str = lua_tostring(dumper->L, -1); > - if (!yaml_alias_event_initialize(&ev, (yaml_char_t *) str) || > + if (!yaml_alias_event_initialize(&ev, (yaml_char_t *) anchor) || > !yaml_emitter_emit(&dumper->emitter, &ev)) > luaL_error(dumper->L, OOM_ERRMSG); > - lua_pop(dumper->L, 1); > + return (yaml_char_t *)""; > } > - return (yaml_char_t *)s; > + return (yaml_char_t *) anchor; > } > > -static int dump_table(struct lua_yaml_dumper *dumper, struct luaL_field *field){ > +static int dump_table(struct lua_yaml_dumper *dumper, struct luaL_field *field, > + yaml_char_t *anchor) { > yaml_event_t ev; > - yaml_char_t *anchor = get_yaml_anchor(dumper); > + if (anchor == NULL) > + anchor = get_yaml_anchor(dumper); > > if (anchor && !*anchor) return 1; > > @@ -556,10 +541,12 @@ static int dump_table(struct lua_yaml_dumper *dumper, struct luaL_field *field){ > yaml_emitter_emit(&dumper->emitter, &ev) != 0 ? 1 : 0; > } > > -static int dump_array(struct lua_yaml_dumper *dumper, struct luaL_field *field){ > +static int dump_array(struct lua_yaml_dumper *dumper, struct luaL_field *field, > + yaml_char_t *anchor) { > unsigned i; > yaml_event_t ev; > - yaml_char_t *anchor = get_yaml_anchor(dumper); > + if (anchor == NULL) > + anchor = get_yaml_anchor(dumper); > > if (anchor && !*anchor) > return 1; > @@ -621,8 +608,18 @@ static int dump_node(struct lua_yaml_dumper *dumper) > bool unused; > (void) unused; > > + yaml_char_t *anchor = NULL; > + > + /* > + * Keep anchor to print it if luaL_checkfield() push new value > + * on stack after serialization. > + */ > + if (lua_istable(dumper->L, -1) == 1) > + anchor = get_yaml_anchor(dumper); > + > int top = lua_gettop(dumper->L); > - luaL_checkfield(dumper->L, dumper->cfg, top, &field); > + luaL_checkfield(dumper->L, dumper->cfg, dumper->anchortable_index, top, > + &field); > switch(field.type) { > case MP_UINT: > snprintf(buf, sizeof(buf) - 1, "%" PRIu64, field.ival); > @@ -647,9 +644,9 @@ static int dump_node(struct lua_yaml_dumper *dumper) > len = strlen(buf); > break; > case MP_ARRAY: > - return dump_array(dumper, &field); > + return dump_array(dumper, &field, anchor); > case MP_MAP: > - return dump_table(dumper, &field); > + return dump_table(dumper, &field, anchor); > case MP_STR: > str = lua_tolstring(dumper->L, -1, &len); > if (yaml_is_null(str, len) || yaml_is_bool(str, len, &unused) || > @@ -745,35 +742,6 @@ static int append_output(void *arg, unsigned char *buf, size_t len) { > return 1; > } > > -static void find_references(struct lua_yaml_dumper *dumper) { > - int newval = -1, type = lua_type(dumper->L, -1); > - if (type != LUA_TTABLE) > - return; > - > - lua_pushvalue(dumper->L, -1); /* push copy of table */ > - lua_rawget(dumper->L, dumper->anchortable_index); > - if (lua_isnil(dumper->L, -1)) > - newval = 0; > - else if (!lua_toboolean(dumper->L, -1)) > - newval = 1; > - lua_pop(dumper->L, 1); > - if (newval != -1) { > - lua_pushvalue(dumper->L, -1); > - lua_pushboolean(dumper->L, newval); > - lua_rawset(dumper->L, dumper->anchortable_index); > - } > - if (newval) > - return; > - > - /* recursively process other table values */ > - lua_pushnil(dumper->L); > - while (lua_next(dumper->L, -2) != 0) { > - find_references(dumper); /* find references on value */ > - lua_pop(dumper->L, 1); > - find_references(dumper); /* find references on key */ > - } > -} > - > int > lua_yaml_encode(lua_State *L, struct luaL_serializer *serializer, > const char *tag_handle, const char *tag_prefix) > @@ -819,7 +787,7 @@ lua_yaml_encode(lua_State *L, struct luaL_serializer *serializer, > dumper.anchortable_index = lua_gettop(L); > dumper.anchor_number = 0; > lua_pushvalue(L, 1); /* push copy of arg we're processing */ > - find_references(&dumper); > + find_references(L, dumper.anchortable_index, "before"); > dump_document(&dumper); > if (dumper.error) > goto error;