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 00DC46EC55; Wed, 28 Jul 2021 00:26:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 00DC46EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1627421208; bh=FjpKD4uizemQ0U6yElMj193JiXrN9nttU1rVG1IIG9s=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Tn58Rkzjv6jNnDrJ8z7xopqFYJcW1ye1vwM+KZCeGnJ0vL65nrD03x4hufz4dYFNs IOTopo3ddK6mvK1mNhhI+eVaxnBH0hEfO1U4lPYXAgIXYshcF9BdHRDDT75o36Q2qv zYz5IBAUDxxeSiZ7wYE+aOon+QetR8CH3zhaLgx0= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (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 553696EC6E for ; Wed, 28 Jul 2021 00:24:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 553696EC6E Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1m8UYn-0005in-L5; Wed, 28 Jul 2021 00:24:22 +0300 To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergos@tarantool.org Date: Tue, 27 Jul 2021 23:24:15 +0200 Message-Id: <0deaaafed27a864820d91181c5d5f8c0c7bdcf70.1627420835.git.v.shpilevoy@tarantool.org> X-Mailer: git-send-email 2.24.3 (Apple Git-128) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD941C43E597735A9C351B198F4576AC7B21928AAE70459C21B182A05F5380850405B59663A204F6591459C8AD53BFFAEE2BD68D32D2A7AD2E0F2B1679F72491240 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E8204D72912C702FEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006371331F31406ECCA328638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8EAFA723B88DDC1CEB341BC94A52B5D91117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC60CDF180582EB8FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD1828451B159A507268D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B613439FA09F3DCB32089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C7BEA09003D200E083A9140E3CD7F99635ED29DCCE6C023529C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EFA5DF9383870C0FED699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D347D10A9FCB2A62DFED357432E1995BF5D86E93849BC5711B30335EB77395DA4A1B0E4E2BEDD51911D1D7E09C32AA3244C53390C0C753B71F459E1CBB23993AE7E05AB220A9D022EBCFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojiF1u9eOpfTRwlLx2m/q9Gg== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5DA331499812CCE1011D1D99E1535FB7F53841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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)