Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] serializer: serialize recursive structures
@ 2021-03-11  5:49 Roman Khabibov via Tarantool-patches
  2021-03-14 12:42 ` Sergey Kaplun via Tarantool-patches
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Roman Khabibov via Tarantool-patches @ 2021-03-11  5:49 UTC (permalink / raw)
  To: tarantool-patches

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:<port>
-    status: alive
-    incarnation: cdata {generation = 0ULL, version = 2ULL}
-    uuid: 00000000-0000-1000-8000-000000000001
-    payload_size: 8
-  - uri: 127.0.0.1:<port>
+  - &0
+    uri: 127.0.0.1:<port>
     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:<port>
-    status: left
-    incarnation: cdata {generation = 0ULL, version = 1ULL}
-    uuid: 00000000-0000-1000-8000-000000000002
-    payload_size: 0
-  - uri: 127.0.0.1:<port>
+- - &0
+    uri: 127.0.0.1:<port>
     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;
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-07 22:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11  5:49 [Tarantool-patches] [PATCH] serializer: serialize recursive structures Roman Khabibov via Tarantool-patches
2021-03-14 12:42 ` Sergey Kaplun via Tarantool-patches
2021-04-05 22:05   ` Roman Khabibov via Tarantool-patches
2021-04-21  9:27     ` Sergey Kaplun via Tarantool-patches
2021-04-21 13:12       ` Sergey Bronnikov via Tarantool-patches
2021-04-21 18:20         ` Sergey Kaplun via Tarantool-patches
2021-04-13 15:54 ` Sergey Bronnikov via Tarantool-patches
2021-04-15 20:39   ` Roman Khabibov via Tarantool-patches
2021-04-21 12:34 ` Sergey Bronnikov via Tarantool-patches
2021-07-07 22:42 ` Alexander Turenko via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox