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 4FC65644CF9; Fri, 29 Sep 2023 16:38:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4FC65644CF9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1695994731; bh=WzfrIjk+PxpnQNJkAVMu2NuCJeVDnGIvpkdK3mShtLE=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=BXwqyfujZh5zID5I7+xRcLC+DCZm74SzGasWn4ptPF1BRkUkfUOGQSwxNbFarAobW vdCPlt7+fC5J0ZKKEkpwevtsxD5y7ydpGsNG48KtrfeqjXsmW/+TWPhcXw8ZS7K+Jl 9x/eDQIJ+kQLXYZtAOt7VmuM+sKq7U+SEh+q0sCQ= Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 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 8E88B5FC8F2 for ; Fri, 29 Sep 2023 16:38:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8E88B5FC8F2 Received: by mail-lf1-f50.google.com with SMTP id 2adb3069b0e04-50435a9f800so21397354e87.2 for ; Fri, 29 Sep 2023 06:38:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695994728; x=1696599528; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YKcTOglnsbqT6mn/FFqr+iHdVGXFlhoHJjL+7Lzgzss=; b=dNixmnygjvG1y3S7w96On58rfYTFXWmFe06GconoDwQPRRfotQIaGQbqNGzbBSn46s 3wda4I3d/QU7y0h1g4KK/oWNLF4fLlMfdgPHbVKnuqIRVeafmmlznWXgRS//P7vsPN6T ogoBHZDTxjImBlGB5/GjLIzJX9wWF7OIZFfprKEyKcW/fnpkiTJLDmHJATrQoyqj00oR nLqaSJhJUerB0A7sFM9h5a+SiXsXUv6gOSU7J1Ol7LX02OLqyJQg2ad54nRIZu8HSnHN mSKfIoGR+zNMqOj890JAgpHJhaMxevl/1yfMwWUE4/VogZDeosfLWn5ABpf4DU814+3T FH8w== X-Gm-Message-State: AOJu0YypqN6mD6IZCqErp/PhOVUca/awL5JvoCzFumvq71UDs6kmw929 vapbmb4KVtT1yhqEmQdDUMIU5hXx5Nc= X-Google-Smtp-Source: AGHT+IFxZAEoA8odN51cXjJ3IETR/Wwzml08VT2um1NdapAzILN7R8qUJaXjq1HuvMn8sgf/0h+w0A== X-Received: by 2002:a19:ca59:0:b0:500:7f51:d129 with SMTP id h25-20020a19ca59000000b005007f51d129mr3396980lfj.34.1695994727150; Fri, 29 Sep 2023 06:38:47 -0700 (PDT) Received: from fckxorg.mail.msk ([2a00:1148:b0ba:16:a3e8:bdc1:dbed:dbc8]) by smtp.gmail.com with ESMTPSA id o1-20020ac24341000000b004ff973cb14esm3498306lfl.108.2023.09.29.06.38.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Sep 2023 06:38:46 -0700 (PDT) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Date: Fri, 29 Sep 2023 16:38:29 +0300 Message-ID: <20230929133843.535217-1-m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit v2] Fix snapshot PC when linking to BC_JLOOP that was a BC_RET*. 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Reported by Arseny Vakhrushev. Fix contributed by Peter Cawley. As specified in the comment in `lj_record_stop`, all loops must set `J->pc` to the next instruction. However, the chunk of logic in `lj_trace_exit` expects it to be set to `BC_JLOOP` itself if it used to be a `BC_RET`. This wrong pc results in the execution of random data that goes after `BC_JLOOP` in the case of restoration from the snapshot. This patch fixes that behavior by adapting the loop recording logic to this specific case. Maxim Kokryashkin: * added the description and the test for the problem Part of tarantool/tarantool#8825 --- Changes in v2: - Fixed comments as per review by Sergey Kaplun Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-624-jloop-snapshot-pc PR: https://github.com/tarantool/tarantool/pull/9166 src/lj_record.c | 9 ++- src/lj_snap.c | 3 + .../lj-624-jloop-snapshot-pc.test.lua | 81 +++++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 test/tarantool-tests/lj-624-jloop-snapshot-pc.test.lua diff --git a/src/lj_record.c b/src/lj_record.c index 48a5481b..3bdc6134 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -570,10 +570,10 @@ static LoopEvent rec_iterl(jit_State *J, const BCIns iterins) } /* Record LOOP/JLOOP. Now, that was easy. */ -static LoopEvent rec_loop(jit_State *J, BCReg ra) +static LoopEvent rec_loop(jit_State *J, BCReg ra, int skip) { if (ra < J->maxslot) J->maxslot = ra; - J->pc++; + J->pc += skip; return LOOPEV_ENTER; } @@ -2433,7 +2433,7 @@ void lj_record_ins(jit_State *J) rec_loop_interp(J, pc, rec_iterl(J, *pc)); break; case BC_LOOP: - rec_loop_interp(J, pc, rec_loop(J, ra)); + rec_loop_interp(J, pc, rec_loop(J, ra, 1)); break; case BC_JFORL: @@ -2443,7 +2443,8 @@ void lj_record_ins(jit_State *J) rec_loop_jit(J, rc, rec_iterl(J, traceref(J, rc)->startins)); break; case BC_JLOOP: - rec_loop_jit(J, rc, rec_loop(J, ra)); + rec_loop_jit(J, rc, rec_loop(J, ra, + !bc_isret(bc_op(traceref(J, rc)->startins)))); break; case BC_IFORL: diff --git a/src/lj_snap.c b/src/lj_snap.c index 2dc281cb..b50ecfb2 100644 --- a/src/lj_snap.c +++ b/src/lj_snap.c @@ -115,6 +115,9 @@ static MSize snapshot_framelinks(jit_State *J, SnapEntry *map, uint8_t *topslot) #else MSize f = 0; map[f++] = SNAP_MKPC(J->pc); /* The current PC is always the first entry. */ + lj_assertJ(!J->pt || + (J->pc >= proto_bc(J->pt) && + J->pc < proto_bc(J->pt) + J->pt->sizebc), "bad snapshot PC"); #endif while (frame > lim) { /* Backwards traversal of all frames above base. */ if (frame_islua(frame)) { diff --git a/test/tarantool-tests/lj-624-jloop-snapshot-pc.test.lua b/test/tarantool-tests/lj-624-jloop-snapshot-pc.test.lua new file mode 100644 index 00000000..e0c1fa81 --- /dev/null +++ b/test/tarantool-tests/lj-624-jloop-snapshot-pc.test.lua @@ -0,0 +1,81 @@ +local tap = require('tap') +local test = tap.test('lj-624-jloop-snapshot-pc'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) +-- XXX: The test case below triggers the assertion that was +-- added in the patch if tested without the fix itself. It +-- is hard to create a stable reproducer without turning off +-- ASLR and VM randomizations, which is not suitable for testing. + +-- Reproducer below produces the following traces: +-- ---- TRACE 1 start test.lua:2 +-- 0001 KSHORT 1 2 +-- 0002 ISGE 0 1 +-- 0003 JMP 1 => 0006 +-- 0006 UGET 1 0 ; fib +-- 0007 SUBVN 2 0 0 ; 1 +-- 0008 CALL 1 2 2 +-- 0000 . FUNCF 4 ; test.lua:2 +-- 0001 . KSHORT 1 2 +-- 0002 . ISGE 0 1 +-- 0003 . JMP 1 => 0006 +-- 0006 . UGET 1 0 ; fib +-- 0007 . SUBVN 2 0 0 ; 1 +-- 0008 . CALL 1 2 2 +-- 0000 . . FUNCF 4 ; test.lua:2 +-- 0001 . . KSHORT 1 2 +-- 0002 . . ISGE 0 1 +-- 0003 . . JMP 1 => 0006 +-- 0006 . . UGET 1 0 ; fib +-- 0007 . . SUBVN 2 0 0 ; 1 +-- 0008 . . CALL 1 2 2 +-- 0000 . . . FUNCF 4 ; test.lua:2 +-- ---- TRACE 1 stop -> up-recursion +-- +-- ---- TRACE 1 exit 1 +-- ---- TRACE 2 start 1/1 test.lua:3 +-- 0004 ISTC 1 0 +-- 0005 JMP 1 => 0013 +-- 0013 RET1 1 2 +-- 0009 UGET 2 0 ; fib +-- 0010 SUBVN 3 0 1 ; 2 +-- 0011 CALL 2 2 2 +-- 0000 . JFUNCF 4 1 ; test.lua:2 +-- ---- TRACE 2 stop -> 1 +-- +-- ---- TRACE 2 exit 1 +-- ---- TRACE 3 start 2/1 test.lua:3 +-- 0013 RET1 1 2 +-- 0012 ADDVV 1 1 2 +-- 0013 RET1 1 2 +-- ---- TRACE 3 abort test.lua:3 -- down-recursion, restarting +-- +-- ---- TRACE 3 start test.lua:3 +-- 0013 RET1 1 2 +-- 0009 UGET 2 0 ; fib +-- 0010 SUBVN 3 0 1 ; 2 +-- 0011 CALL 2 2 2 +-- 0000 . JFUNCF 4 1 ; test.lua:2 +-- ---- TRACE 3 stop -> 1 +-- +-- ---- TRACE 2 exit 1 +-- ---- TRACE 4 start 2/1 test.lua:3 +-- 0013 RET1 1 2 +-- 0012 ADDVV 1 1 2 +-- 0013 JLOOP 3 3 +-- +-- During the recording of the latter JLOOP the assertion added +-- in the patch is triggered. + + +jit.opt.start('hotloop=1', 'hotexit=1') +local function fib(n) + return n < 2 and n or fib(n - 1) + fib(n - 2) +end + +fib(5) + +test:ok(true, 'snapshot pc is correct') +test:done(true) -- 2.42.0