[Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr()

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

decimal conversion to string in Lua used decimal_str() function.
The function is not safe to use in preemptive context like Lua,
where any attempt to push something onto the Lua stack might
trigger GC, which in turn might invoke any other code.

It is not safe because uses the static buffer, which is global and
cyclic. Newer allocations can override the old data without any
warning.

The same problem was fixed for tt_uuid_str() and uuids in
box.info in one of the previous commits.

The patch adds a new function lua_pushdecimalstr() which does not
use the static buffer. It is now used to push decimals safely on a
Lua stack.

Follow up #5632
Follow up #6050
Closes #6259
---
 .../unreleased/gh-6259-static-buf-in-lua.md   |  5 +++
 src/lua/decimal.c                             | 14 ++++++-
 src/lua/decimal.h                             |  3 ++
 .../gh-5632-6050-6259-gc-buf-reuse.test.lua   | 39 ++++++++++++++++++-
 4 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/gh-6259-static-buf-in-lua.md

diff --git a/changelogs/unreleased/gh-6259-static-buf-in-lua.md b/changelogs/unreleased/gh-6259-static-buf-in-lua.md
new file mode 100644
index 000000000..1a2f7705a
--- /dev/null
+++ b/changelogs/unreleased/gh-6259-static-buf-in-lua.md
@@ -0,0 +1,5 @@
+## bugfix/core
+
+ * `box.info.uuid`, `box.info.cluster.uuid`, and `tostring(decimal)` with any
+   decimal number in Lua sometimes could return garbage if `__gc` handlers are
+   used in user's code (gh-6259).
diff --git a/src/lua/decimal.c b/src/lua/decimal.c
index e25b3ec18..a50e4142a 100644
--- a/src/lua/decimal.c
+++ b/src/lua/decimal.c
@@ -89,6 +89,18 @@ lua_pushdecimal(struct lua_State *L)
 	return res;
 }
 
+void
+lua_pushdecimalstr(struct lua_State *L, const decimal_t *dec)
+{
+	/*
+	 * Do not use a global buffer. It might be overwritten if GC starts
+	 * working.
+	 */
+	char str[DECIMAL_MAX_STR_LEN + 1];
+	decimal_to_string(dec, str);
+	lua_pushstring(L, str);
+}
+
 /**
  * Returns true if a value at a given index is a decimal
  * and false otherwise
@@ -376,7 +388,7 @@ ldecimal_tostring(struct lua_State *L)
 	if (lua_gettop(L) < 1)
 		return luaL_error(L, "usage: decimal.tostring(decimal)");
 	decimal_t *lhs = lua_checkdecimal(L, 1);
-	lua_pushstring(L, decimal_str(lhs));
+	lua_pushdecimalstr(L, lhs);
 	return 1;
 }
 
diff --git a/src/lua/decimal.h b/src/lua/decimal.h
index b13e59247..59a71c312 100644
--- a/src/lua/decimal.h
+++ b/src/lua/decimal.h
@@ -44,6 +44,9 @@ struct lua_State;
 decimal_t *
 lua_pushdecimal(struct lua_State *L);
 
+void
+lua_pushdecimalstr(struct lua_State *L, const decimal_t *dec);
+
 void
 tarantool_lua_decimal_init(struct lua_State *L);
 
diff --git a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
index f806ba6b7..45273ed9a 100755
--- a/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
+++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
@@ -13,6 +13,7 @@ local uuid = require('uuid')
 local uri = require('uri')
 local json = require('json')
 local msgpackffi = require('msgpackffi')
+local decimal = require('decimal')
 
 local function test_uuid(test)
     test:plan(1)
@@ -223,14 +224,50 @@ local function test_info_uuid(test)
     test:ok(is_success, 'info uuid in gc')
 end
 
+local function test_decimal(test)
+    test:plan(1)
+
+    local gc_count = 100
+    local iter_count = 1000
+    local is_success = true
+
+    local d1 = decimal.new(1111111111111111111)
+    local d2 = decimal.new(2222222222222222222)
+
+    local function decimal_to_str()
+        local str1 = tostring(d1)
+        local str2 = tostring(d2)
+        local str3 = tostring(d1)
+        local str4 = tostring(d2)
+        if str1 ~= str3 or str2 ~= str4 then
+            is_success = false
+            assert(false)
+        end
+    end
+
+    local function create_gc()
+        for _ = 1, gc_count do
+            ffi.gc(ffi.new('char[1]'), function() decimal_to_str() end)
+        end
+    end
+
+    for _ = 1, iter_count do
+        create_gc()
+        decimal_to_str()
+    end
+
+    test:ok(is_success, 'decimal str in gc')
+end
+
 box.cfg{}
 
 local test = tap.test('gh-5632-6050-6259-gc-buf-reuse')
-test:plan(5)
+test:plan(6)
 test:test('uuid in __gc', test_uuid)
 test:test('uri in __gc', test_uri)
 test:test('msgpackffi in __gc', test_msgpackffi)
 test:test('json in __gc', test_json)
 test:test('info uuid in __gc', test_info_uuid)
+test:test('decimal str in __gc', test_decimal)
 
 os.exit(test:check() and 0 or 1)
-- 
2.24.3 (Apple Git-128)