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 4D61A13F152D; Wed, 11 Jun 2025 12:37:01 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4D61A13F152D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1749634621; bh=MqlzgnOzkmDcqDwKxoEnIVCwrLZnH0+bKe8QOZiYreM=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=tTiAj82IYdhXE0W8tdYSI8ZDUvqVvjb1BLrHeno/ETGpkquxMcXhaP4j4qTUFZKr3 DmsBYWbKuNELI/m/5Er4DUh/tjEDEV9KF592gPDJNNxeBOS3tLm0xbwMbgUuMJAOxV +OFMz03fuFpoVD8utaFlEmyzpSSviBbAyKJeXNFY= Received: from send37.i.mail.ru (send37.i.mail.ru [89.221.237.132]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3DB7172E60 for ; Wed, 11 Jun 2025 12:36:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3DB7172E60 Received: by exim-smtp-85b97957d7-gf7pl with esmtpa (envelope-from ) id 1uPHtC-00000000EkZ-1LNX; Wed, 11 Jun 2025 12:36:58 +0300 Content-Type: multipart/alternative; boundary="------------SXu9xWG03xxSKKwERMbX8N7l" Message-ID: <382aae48-aa98-4aef-8969-4fad892fa25f@tarantool.org> Date: Wed, 11 Jun 2025 12:36:58 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <1152e27d618a4717c0f48cb77d085434eb183b18.1749550966.git.skaplun@tarantool.org> <9d1c7b94-b18d-4290-b017-27ac9e4f6ef2@tarantool.org> Content-Language: en-US In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD909B82214CC9889175395EA0A18D39C97654ADDB14BA8099C00894C459B0CD1B95BD28242C7BAB63A03ED270C30F246C507438157DCFC5D23D2F53EB5636F5139AB5D286C5C76880D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F09446BC3D835A58EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375668906014A252870FC20A3EE9A0729DD8E709AB62F24F26AEF3807CDCF2D77C214389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0ECC8AC47CD0EDEFF8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6F459A8243F1D1D44CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C224958C1606C78F2434E76E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B6A4E49BB0F3BA1413AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735B17145F0B7815491C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5F6155B5F013E38665002B1117B3ED69685F62757D7DEE847886DC9BC01168B20823CB91A9FED034534781492E4B8EEADAE4FDBF11360AC9BBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF37C5FA689D0DB9F646231FF931FF6F5E661D19D629EDB4260134311989E9B87ECFC48FC8A09AC8551D3122CD40B080D2817B6D1BE8EEE1F11020CB9451B76377A4E8894014B66EB1111DC66A97D0BFE2913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVSykAyseJQ6/3pzhXVlhKvI= X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D61405BD28242C7BAB63A03ED270C30F246C5907D5149F8E7F00D0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/2] Different fix for partial snapshot restore due to stack overflow. 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" This is a multi-part message in MIME format. --------------SXu9xWG03xxSKKwERMbX8N7l Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Sergey, LGTM On 6/10/25 19:22, Sergey Kaplun wrote: > Hi, Sergey! > Thanks for the review! > Please, consider my answer below. > > On 10.06.25, Sergey Bronnikov wrote: >> Hello, Sergey! >> >> Thanks for the patch! See a comment below. >> >> Sergey >> >> On 6/10/25 13:28, Sergey Kaplun wrote: >>> From: Mike Pall >>> >>> Reported by Junlong Li. Fixed by Peter Cawley. >>> >>> (cherry picked from commit 86e7123bb1782a5f200ba5e83b8c4f3fbad4f7bc) >>> >>> This patch is a follow-up to the previous commit, which leads to a dirty >>> read of the pseudo-valid PC set for the cframe on snapshot restoration. >>> To avoid these dirty reads, this patch sets the PC to the outer frame >>> as well before possible error throwing. >>> >>> Sergey Kaplun: >>> * added the description and the test for the problem >>> >>> Part of tarantool/tarantool#11278 >>> --- >>> src/lj_snap.c | 3 +- >>> src/lj_trace.c | 4 +- >>> ...-1196-stack-overflow-snap-restore.test.lua | 65 +++++++++++++++++++ >>> 3 files changed, 68 insertions(+), 4 deletions(-) >>> create mode 100644 test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua >>> >>> diff --git a/src/lj_snap.c b/src/lj_snap.c >>> index 8d7bd868..4cfae579 100644 >>> --- a/src/lj_snap.c >>> +++ b/src/lj_snap.c > > >>> diff --git a/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua >>> new file mode 100644 >>> index 00000000..942d1f82 >>> --- /dev/null >>> +++ b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua >>> @@ -0,0 +1,65 @@ >>> +local tap = require('tap') >>> + >>> +-- Test file to demonstrate LuaJIT dirty reads after stack >>> +-- overflow during restoration from the snapshot. >>> +-- The test fails before the patch under Valgrind. >> Please specify valgrind option that is required for reproducing the bug. > This just mean the default valgrind settings (without any flags). With this command-line the bug is reproduced: cmake -S . -B build -DLUAJIT_USE_VALGRIND=ON -DLUAJIT_USE_ASAN=OFF -DLUAJIT_USE_SYSMALLOC=ON -DLUAJIT_ENABLE_GC64=ON  -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DCMAKE_BUILD_TYPE=Debug cd build && ctest -V -R test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua -V > >> Cannot reproduce with command line below with reverted patch: >> >> VALGRIND_OPTS="--leak-check=no --malloc-fill=0x00 --free-fill=0x00" >> ctest -V -R test/tarantool-tests/lj-1196-partial-snap-restore.test.lua -V > This doesn't fail due to --leadk-check=no flag. > Also, I suppose this should fail under ASan. > >>> +-- >>> +-- luacheck: push no max_comment_line_length >>> +-- >>> +-- Seealso:https://github.com/LuaJIT/LuaJIT/issues/1196, >>> +--https://www.freelists.org/post/luajit/Invalid-read-found-by-valgrind. >>> +-- >>> +-- luacheck: pop >>> + > > >>> +test:done(true) --------------SXu9xWG03xxSKKwERMbX8N7l Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi, Sergey,

LGTM

On 6/10/25 19:22, Sergey Kaplun wrote:
Hi, Sergey!
Thanks for the review!
Please, consider my answer below.

On 10.06.25, Sergey Bronnikov wrote:

Hello, Sergey!

Thanks for the patch! See a comment below.

Sergey

On 6/10/25 13:28, Sergey Kaplun wrote:

From: Mike Pall <mike>

Reported by Junlong Li. Fixed by Peter Cawley.

(cherry picked from commit 86e7123bb1782a5f200ba5e83b8c4f3fbad4f7bc)

This patch is a follow-up to the previous commit, which leads to a dirty
read of the pseudo-valid PC set for the cframe on snapshot restoration.
To avoid these dirty reads, this patch sets the PC to the outer frame
as well before possible error throwing.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11278
---
  src/lj_snap.c                                 |  3 +-
  src/lj_trace.c                                |  4 +-
  ...-1196-stack-overflow-snap-restore.test.lua | 65 +++++++++++++++++++
  3 files changed, 68 insertions(+), 4 deletions(-)
  create mode 100644 test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua

diff --git a/src/lj_snap.c b/src/lj_snap.c
index 8d7bd868..4cfae579 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c

<snipped>


diff --git a/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua
new file mode 100644
index 00000000..942d1f82
--- /dev/null
+++ b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua
@@ -0,0 +1,65 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT dirty reads after stack
+-- overflow during restoration from the snapshot.
+-- The test fails before the patch under Valgrind.

Please specify valgrind option that is required for reproducing the bug.

This just mean the default valgrind settings (without any flags).

With this command-line the bug is reproduced:

cmake -S . -B build -DLUAJIT_USE_VALGRIND=ON -DLUAJIT_USE_ASAN=OFF -DLUAJIT_USE_SYSMALLOC=ON -DLUAJIT_ENABLE_GC64=ON  -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DCMAKE_BUILD_TYPE=Debug

cd build && ctest -V -R test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua -V




Cannot reproduce with command line below with reverted patch:

VALGRIND_OPTS="--leak-check=no --malloc-fill=0x00 --free-fill=0x00" 
ctest -V -R test/tarantool-tests/lj-1196-partial-snap-restore.test.lua -V

This doesn't fail due to --leadk-check=no flag.
Also, I suppose this should fail under ASan.



        

          
+--
+-- luacheck: push no max_comment_line_length
+--
+-- See also:https://github.com/LuaJIT/LuaJIT/issues/1196,
+--https://www.freelists.org/post/luajit/Invalid-read-found-by-valgrind.
+--
+-- luacheck: pop
+


        

      

      
<snipped>



      

        

          
+test:done(true)


        

      

      

    

  
  


--------------SXu9xWG03xxSKKwERMbX8N7l--