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 5F61D503745; Fri, 9 Jun 2023 12:14:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5F61D503745 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1686302045; bh=5Aos3xU7LWzNpMjtAyHgz0d+B/YX8QKoDyS7WxWeBgQ=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=bfc2RhD+bxgiz3at9sjZNbTgH+OfbM26YPeNQaVjdagatGR0BpkC5aOd+1vueZc1k lD0Hqw0/yQoruWy4lQAxNJtAbaeF6hiItBWHhmU/8v/saaPS5xiNsA50MuekMal5HD BhLTi3Q/VYKIwfD28qL9t9gBs7/2qdMmXiHNlRBk= Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) (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 7975449900F for ; Fri, 9 Jun 2023 12:14:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7975449900F Received: by mail-lf1-f44.google.com with SMTP id 2adb3069b0e04-4f4b2bc1565so1891378e87.2 for ; Fri, 09 Jun 2023 02:14:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686302042; x=1688894042; 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=8yqxUD/9qiQa1bESc9tbqyAwPX1eAr8n39PAo9vG0T4=; b=TjGTLlHhabSy3KxJi0PMakSeM/WXCooemeg7zU1ULiGRkzT1OOxdeocoq4ivzvdWd8 62z0YfrbZRDVP/UY46BSdvW1kTDdZq37V6REWTMyT9ggb+3QsxH35LvZnsZH1rjGYFZ0 OIbGGFRpUQ2zs0qCC0WFKMpMHLfid8RXue/svBw/GfPY61JbnUQ1XqzoJmskChx/UUBE em1WN9uKql1mNCd9IRoLmKSYoDK+UFVqLc0cwabG/MqT1tRdWIMtJzyMF2s+MAlfDYLj treQTs4v3PPw33gEvex03v9XRoXTDMwWyIaJmAl8bftMa808o4+8h1PZwbirXwZHN+16 tlFw== X-Gm-Message-State: AC+VfDwCMJHCM+h1xD54G8SrIBHst7h+Pk8kfF9NIWcoNp/fVDYxQR9q 67RDVp/vaAQVKgzbHnpkPkftPbtPKBm9xg== X-Google-Smtp-Source: ACHHUZ66+SouIgLJ00d6tw8UTCj0OzzHbTe13DV/GdwJKnpOPRPDyZA7SjDn67As/KRgOEwf2zm4qg== X-Received: by 2002:a19:4f4e:0:b0:4f6:40e1:4a92 with SMTP id a14-20020a194f4e000000b004f640e14a92mr554065lfk.13.1686302042045; Fri, 09 Jun 2023 02:14:02 -0700 (PDT) Received: from localhost.localdomain (95-24-8-160.broadband.corbina.ru. [95.24.8.160]) by smtp.gmail.com with ESMTPSA id a2-20020ac25202000000b004f4b42e2d7dsm474197lfl.230.2023.06.09.02.14.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Jun 2023 02:14:01 -0700 (PDT) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Date: Fri, 9 Jun 2023 12:13:56 +0300 Message-Id: <20230609091358.1133736-1-m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit v2] x64: Fix 64 bit shift code generation. 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 Philipp Kutin. Fix contributed by Peter Cawley. (cherry-picked from commit 03a7ebca4f6819658cdaa12ba3af54a17b8035e9) In a situation where a variable shift left bitwise rotation that has a 64-bit result is recorded on an x86 64-bit processor and the result is supposed to end up in the `rcx` register, that value could be written into the `ecx` instead, thus being truncated into 32 bits. This patch fixes the described behavior, so now that value is written into the `rcx`. Resulting assembly changes from the following before the patch: | rol rsi, cl | mov ecx, esi to the following after the patch: | rol rsi, cl | mov rcx, rsi Importantly, the same behavior is impossible with the right rotation on machines with BMI2 support because there is a BMI2 instruction for it, so it is handled differently. Maxim Kokryashkin: * added the description and the test for the problem Part of tarantool/tarantool#8516 --- Changes in v2: - Fixed comments as per review by Sergey Kaplun PR: https://github.com/tarantool/tarantool/pull/8727 Branch: https://github.com/tarantool/luajit/tree/fckxorg/fix-bit-shift-generation src/lj_asm_x86.h | 2 +- .../fix-bit-shift-generation.test.lua | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/fix-bit-shift-generation.test.lua diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h index e6c42c6d..63d332ca 100644 --- a/src/lj_asm_x86.h +++ b/src/lj_asm_x86.h @@ -2328,7 +2328,7 @@ static void asm_bitshift(ASMState *as, IRIns *ir, x86Shift xs, x86Op xv) dest = ra_dest(as, ir, rset_exclude(RSET_GPR, RID_ECX)); if (dest == RID_ECX) { dest = ra_scratch(as, rset_exclude(RSET_GPR, RID_ECX)); - emit_rr(as, XO_MOV, RID_ECX, dest); + emit_rr(as, XO_MOV, REX_64IR(ir, RID_ECX), dest); } right = irr->r; if (ra_noreg(right)) diff --git a/test/tarantool-tests/fix-bit-shift-generation.test.lua b/test/tarantool-tests/fix-bit-shift-generation.test.lua new file mode 100644 index 00000000..9f14a9e3 --- /dev/null +++ b/test/tarantool-tests/fix-bit-shift-generation.test.lua @@ -0,0 +1,50 @@ +local tap = require('tap') +local test = tap.test('fix-bit-shift-generation'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +local NITERATIONS = 4 +local NTESTS = NITERATIONS * 2 + +test:plan(NTESTS) + +local ffi = require('ffi') +local bit = require('bit') +local rol = bit.rol +local shl = bit.lshift + +ffi.cdef('int snprintf(char *str, size_t n, const char *format, ...);') + +-- Buffer size is adjsuted to fit `(1 << 36)`, +-- which has exactly 11 digits. +local BUFFER_SIZE = 12 +local bufs = {} +for i = 1, NTESTS do + bufs[i] = ffi.new('char[?]', BUFFER_SIZE) +end + +local result = {} +jit.opt.start('hotloop=1') + +for i = 1, NITERATIONS do + -- Rotation is performed beyond the 32-bit size, for truncation + -- to become noticeable. Sprintf is used to ensure that the + -- result of rotation goes into the `rcx`, corresponing to + -- the x86_64 ABI. + result[i] = ffi.C.snprintf(bufs[i], BUFFER_SIZE, '%llu', rol(1ULL, i + 32)) + -- Resulting assembly for the `rol` instruction above changes + -- from the following before the patch: + -- | rol rsi, cl + -- | mov ecx, esi + -- + -- to the following after the patch: + -- | rol rsi, cl + -- | mov rcx, rsi +end + +for i = 1, NITERATIONS do + test:ok(result[i] > 1, '64-bit value was not truncated') + test:ok(tonumber(ffi.string(bufs[i])) == shl(1ULL, i + 32), 'valid rol') +end + +os.exit(test:check() and 0 or 1) -- 2.40.1