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 AEEA9687D15; Tue, 3 Oct 2023 16:37:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AEEA9687D15 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1696340235; bh=Ece00q43riKnn9473wMNYWRt+DcAcvjjhPNEHyb213s=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=LajEzIVKXkPm3lF92/4885vlcX45Y1sLUvlAc8XsmtgXGILlW3SSWK+5EF6BUc/Rt swJ4BX2qkDvWxKH4VtQ+NENtAkf4nn2uLLeZwITCmySmUvHRqP/pdFZB7iAExWOuSB hmYOoyGYTNvuJOWI1bD52nKpozg1jRh51AvaGuzU= Received: from mail-lj1-f174.google.com (mail-lj1-f174.google.com [209.85.208.174]) (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 B0D1F5CDDE4 for ; Tue, 3 Oct 2023 16:37:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B0D1F5CDDE4 Received: by mail-lj1-f174.google.com with SMTP id 38308e7fff4ca-2c16bc71e4cso11425611fa.0 for ; Tue, 03 Oct 2023 06:37:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696340233; x=1696945033; 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=bdWHWawijilJ66Jakh5Fshf6JR0f1SOSJ11pC9FiOwg=; b=ekC2/861I5IR82VW/qNA1rTil1isXEmgjh9KJWlOKYMGdUARxjtecBhvw1uYCJGReL 6vLHjyhKcPU5OIdrjiF3+3xdMP9oZOO9L40Pj+LmuffDUc6E+/icmNOBk0q2ct0nChMA Fu+TfdT/F9pvXboetpo/y2rR+Cb+vhBlc+FHzPNcvMdN8guvIw+Sj3uDDJkU2hjHfMJJ Wjv+mSUUxr6CCPyVLmxT25m8oYxZdtWY4gIhuY4WO8MqFVIG2iFVYVcsKpO5jdH7oZ/y KKRgNjJPHsuOTF10aKlz6qMAcTOhHS67y4gRVx1bNrifapIuU0nu2hwNcj264yRXHEzR GKBg== X-Gm-Message-State: AOJu0YxkcUG/0+F0W1AhO2oEmQ4Uzygs8BaQVVnGIuJdzLWrYAZ8ivol 3eQb2ody771edRmFO1rFl8aDu7bRuSg= X-Google-Smtp-Source: AGHT+IGPRLXG3/PGTgZNyqOfMu8NM83eaUvQLFBEW57QVGR9UhbMG9MmCeLzE4xkZefmZ6iHQBSlSw== X-Received: by 2002:a2e:2c16:0:b0:2bf:ff17:811e with SMTP id s22-20020a2e2c16000000b002bfff17811emr11443728ljs.14.1696340232410; Tue, 03 Oct 2023 06:37:12 -0700 (PDT) Received: from localhost.localdomain ([185.205.79.32]) by smtp.gmail.com with ESMTPSA id p18-20020a2e7412000000b002c029a4b681sm244843ljc.15.2023.10.03.06.37.10 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Tue, 03 Oct 2023 06:37:11 -0700 (PDT) To: tarantool-patches@dev.tarantool.org, sergeyb@tarantool.org, skaplun@tarantool.org, m.kokryashkin@tarantool.org Date: Tue, 3 Oct 2023 16:37:03 +0300 Message-Id: <20231003133705.5700-1-max.kokryashkin@gmail.com> X-Mailer: git-send-email 2.39.3 (Apple Git-145) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit v2] Mark CONV as non-weak, to prevent elimination of its side-effect. 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: Maksim Kokryashkin via Tarantool-patches Reply-To: Maksim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall An unused guarded CONV int.num cannot be omitted in general. (cherry-picked from commit 881d02d3117838acaf4fb844332c8e33cc95c8c5) In some cases, an unused `CONV int.num` omission in `DUALNUM` mode may lead to a guard absence, resulting in invalid control flow branching and undefined behavior. For a comprehensive example of the described situation, please refer to the comment in `test/tarantool-tests/mark-conv-non-weak.test.lua`. Maxim Kokryashkin: * added the description and the test for the problem Part of tarantool/tarantool#9145 --- Branch: https://github.com/tarantool/luajit/tree/fckxorg/mark-conv-non-weak PR: https://github.com/tarantool/tarantool/pull/8871 Changes in v2: - Fixed comments as per review by Sergey Kaplun src/lj_ir.h | 2 +- .../mark-conv-non-weak.test.lua | 115 ++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/mark-conv-non-weak.test.lua diff --git a/src/lj_ir.h b/src/lj_ir.h index 46af54e4..e3ba6ff3 100644 --- a/src/lj_ir.h +++ b/src/lj_ir.h @@ -132,7 +132,7 @@ _(XBAR, S , ___, ___) \ \ /* Type conversions. */ \ - _(CONV, NW, ref, lit) \ + _(CONV, N , ref, lit) \ _(TOBIT, N , ref, ref) \ _(TOSTR, N , ref, lit) \ _(STRTO, N , ref, ___) \ diff --git a/test/tarantool-tests/mark-conv-non-weak.test.lua b/test/tarantool-tests/mark-conv-non-weak.test.lua new file mode 100644 index 00000000..f54f30ba --- /dev/null +++ b/test/tarantool-tests/mark-conv-non-weak.test.lua @@ -0,0 +1,115 @@ +local tap = require('tap') +local test = tap.test('mark-conv-non-weak'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +local data = {0.1, 0, 0.1, 0, 0 / 0} +local sum = 0 + +jit.opt.start('hotloop=2', 'hotexit=2') + +-- XXX: The test fails before the patch only +-- for `DUALNUM` mode. All of the IRs below are +-- produced by the corresponding LuaJIT build. + +-- When the trace is recorded, the IR +-- is the following before the patch: +---- TRACE 1 IR +-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ] +-- 0001 u8 XLOAD [0x100dac521] V +-- 0002 int BAND 0001 +12 +-- 0003 > int EQ 0002 +0 +-- 0004 > int SLOAD #8 T +-- .... SNAP #1 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ] +-- 0005 > num SLOAD #3 T +-- 0006 num CONV 0004 num.int +-- 0007 + num ADD 0006 0005 +-- 0008 > fun SLOAD #4 T +-- 0009 > tab SLOAD #5 T +-- 0010 > int SLOAD #6 T +-- 0011 > fun EQ 0008 ipairs_aux +-- 0012 + int ADD 0010 +1 +-- 0013 int FLOAD 0009 tab.asize +-- 0014 > int ABC 0013 0012 +-- 0015 p64 FLOAD 0009 tab.array +-- 0016 p64 AREF 0015 0012 +-- 0017 >+ num ALOAD 0016 +-- .... SNAP #2 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ] +-- 0018 ------ LOOP ------------ +-- 0019 u8 XLOAD [0x100dac521] V +-- 0020 int BAND 0019 +12 +-- 0021 > int EQ 0020 +0 +-- 0022 > int CONV 0017 int.num +-- .... SNAP #3 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ] +-- 0023 + num ADD 0017 0007 +-- 0024 + int ADD 0012 +1 +-- 0025 > int ABC 0013 0024 +-- 0026 p64 AREF 0015 0024 +-- 0027 >+ num ALOAD 0026 +-- 0028 num PHI 0017 0027 +-- 0029 num PHI 0007 0023 +-- 0030 int PHI 0012 0024 +---- TRACE 1 stop -> loop + +---- TRACE 1 exit 0 +---- TRACE 1 exit 3 +-- +-- And the following after the patch: +---- TRACE 1 IR +-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ] +-- 0001 u8 XLOAD [0x102438521] V +-- 0002 int BAND 0001 +12 +-- 0003 > int EQ 0002 +0 +-- 0004 > int SLOAD #8 T +-- .... SNAP #1 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ] +-- 0005 > num SLOAD #3 T +-- 0006 num CONV 0004 num.int +-- 0007 + num ADD 0006 0005 +-- 0008 > fun SLOAD #4 T +-- 0009 > tab SLOAD #5 T +-- 0010 > int SLOAD #6 T +-- 0011 > fun EQ 0008 ipairs_aux +-- 0012 + int ADD 0010 +1 +-- 0013 int FLOAD 0009 tab.asize +-- 0014 > int ABC 0013 0012 +-- 0015 p64 FLOAD 0009 tab.array +-- 0016 p64 AREF 0015 0012 +-- 0017 >+ num ALOAD 0016 +-- .... SNAP #2 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ] +-- 0018 ------ LOOP ------------ +-- 0019 u8 XLOAD [0x102438521] V +-- 0020 int BAND 0019 +12 +-- 0021 > int EQ 0020 +0 +-- 0022 > int CONV 0017 int.num +-- .... SNAP #3 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ] +-- 0023 + num ADD 0017 0007 +-- 0024 + int ADD 0012 +1 +-- 0025 > int ABC 0013 0024 +-- 0026 p64 AREF 0015 0024 +-- 0027 >+ num ALOAD 0026 +-- 0028 num PHI 0017 0027 +-- 0029 num PHI 0007 0023 +-- 0030 int PHI 0012 0024 +---- TRACE 1 stop -> loop + +---- TRACE 1 exit 0 +---- TRACE 1 exit 2 +-- +-- Before the patch, the `0022 > int CONV 0017 int.num` +-- instruction is omitted due to DCE, which results in the +-- third side exit being taken, instead of the second, +-- and, hence, incorrect summation. After the patch, `CONV` +-- is left intact and is not omitted; it remains as a guarded +-- instruction, so the second side exit is taken and sum is +-- performed correctly. + +for _, val in ipairs(data) do + if val == val then + sum = sum + val + end +end + +test:ok(sum == sum, 'NaN check was not omitted') +test:done(true) -- 2.39.3 (Apple Git-145)