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 E86EF527404; Sat, 15 Jul 2023 18:41:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E86EF527404 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1689435674; bh=P6L11qJeqBSV6Vbc2DyxaUYS1ldsDIlT6n7W4qsgkAk=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=mZkEfED7+unsWhma3jc5hmcY/3GaOC1OIPdEVX3bSTV6HVd2mgl8NMYQCKr28izsL 13Bb8uUSOBrxPG2ohtzRvuW+VE/BT0+6Lr+x+HwNPPr71/QnVWTQKYQiyozCMpj4/S kAMExYOplZMvl6nil6+ntg0/9kM3ZaFSmaLoQ92I= 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 E38FA527404 for ; Sat, 15 Jul 2023 18:41:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E38FA527404 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1qKhOS-0005iw-55; Sat, 15 Jul 2023 18:41:12 +0300 Date: Sat, 15 Jul 2023 18:36:49 +0300 To: Maxim Kokryashkin Message-ID: References: <20230710122818.22221-1-m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230710122818.22221-1-m.kokryashkin@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD97569A0FE902DCB3DF525A235365A19A2D5B01E069C4E5BA91867C24CE74E72BB5FE18E9CBFBE7C07826B72AD6A955D92043D874E1A5D774204DBDD03C8A231821D371A049BD054D4 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE755BE8F535441E38CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC20F58FBAB79054EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BE5CCB53A13BC8DBA488AD43C0A3EFBD4A788FCEE1BFA5D40CC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC86D3A1509E1113711F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDCE939D40DBB93CA75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A5BF82820C4BB320FE95ABE54BED09B207B873F593E12F5716F87CCE6106E1FC07E67D4AC08A07B9B01DAA61796BF5227BCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF1BBE027FF7E77C4402ADD1573BD6FACC195942E2FBB0445CC142C959A779583EC150D9F1A63664457BBDB0A562C82D59C037DCF50AFDDBEA979DE1B57BE7A3CEA74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojTPX5czuwuGF8DV74LInALQ== X-DA7885C5: 1A4ED4CFFA021E6329F4BF13D6757FE923E7B800A26CC1518E1D998D42DA1F9A262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986BB5493A39A7483C9EE8838DA391CC77780FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v3] sysprof: fix crash during FFUNC stream 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maxim! Thanks for the fixes! Generally LGTM, but please consider my comments below. On 10.07.23, Maxim Kokryashkin wrote: > Sometimes, the Lua stack can be inconsistent during > the FFUNC execution, which may lead to a sysprof > crash during the stack unwinding. > > This patch replaces the `top_frame` property of `global_State` > with `lj_sysprof_topframe` structure, which contains `top_frame` > and `ffid` properties. `ffid` property makes sense only when the > LuaJIT VM state is set to `FFUNC`. That property is set to the > ffid of the fast function that VM is about to execute. > In the same time, `top_frame` property is not updated now, so > the top frame of the Lua stack can be streamed based on the ffid, > and the rest of the Lua stack can be streamed as usual. > > Also, this patch fixes build with plain makefile, by adding Nit: I suggest to rephrase it like "original Makefile" or Makefile.original. Feel free to ignore. > the `LJ_HASSYSPROF` flag support to it. > > Resolves tarantool/tarantool#8594 > --- > Changes in v3: > - Fixed comments as per review by Sergey > > Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8594-sysprof-ffunc-crash > PR: https://github.com/tarantool/tarantool/pull/8737 > src/Makefile.original | 3 ++ > src/lj_obj.h | 7 +++- > src/lj_sysprof.c | 26 ++++++++++++--- > src/vm_x64.dasc | 22 +++++++++++-- > src/vm_x86.dasc | 31 ++++++++++++++--- > .../gh-8594-sysprof-ffunc-crash.test.lua | 33 +++++++++++++++++++ > 6 files changed, 109 insertions(+), 13 deletions(-) > create mode 100644 test/tarantool-tests/gh-8594-sysprof-ffunc-crash.test.lua > > diff --git a/src/Makefile.original b/src/Makefile.original > index aedaaa73..e21a0e56 100644 > --- a/src/Makefile.original > +++ b/src/Makefile.original > diff --git a/test/tarantool-tests/gh-8594-sysprof-ffunc-crash.test.lua b/test/tarantool-tests/gh-8594-sysprof-ffunc-crash.test.lua > new file mode 100644 > index 00000000..e5cdeb07 > --- /dev/null > +++ b/test/tarantool-tests/gh-8594-sysprof-ffunc-crash.test.lua > @@ -0,0 +1,33 @@ > +local tap = require('tap') > +local test = tap.test('gh-8594-sysprof-ffunc-crash'):skipcond({ > + ['Sysprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and > + jit.arch ~= 'x64', > + ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux', > +}) > + > +test:plan(1) > + > +jit.off() > +-- XXX: Run JIT tuning functions in a safe frame to avoid errors > +-- thrown when LuaJIT is compiled with JIT engine disabled. > +pcall(jit.flush) > + > +local TMP_BINFILE = '/dev/null' > + > +local res, err = misc.sysprof.start{ > + mode = 'C', > + interval = 3, > + path = TMP_BINFILE, > +} > +assert(res, err) > + > +for i = 1, 1e5 do > + tostring(i) > +end Within these (interval/iterations) changes I hardly can see assertion failure on master branch for my laptop (1/10 cases). | >>> git log -n1 --no-decorate --oneline | 8e46d601 test: fix flaky | >>> LUA_PATH="src/?.lua;test/tarantool-tests/?.lua;;" src/luajit test/tarantool-tests/gh-8594-sysprof-ffunc-crash.test.lua | TAP version 13 | 1..1 | ok - sysprof finished successfully I don't know how to resolve this problem with our CI at death's door... OTOH, we may consider that this value is enough for our CI, so I'll see the problem there. So, I'll agree with Sergey's opinion about this flaky test. > + > +res, err = misc.sysprof.stop() > +assert(res, err) > + > +test:ok(true, 'sysprof finished successfully') > + > +os.exit(test:check() and 0 or 1) > -- > 2.40.1 > -- Best regards, Sergey Kaplun