From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F1378A0547 for ; Tue, 15 Nov 2022 09:19:20 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EC26540DFD; Tue, 15 Nov 2022 09:19:20 +0100 (CET) Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) by mails.dpdk.org (Postfix) with ESMTP id 1846040150 for ; Tue, 15 Nov 2022 09:19:20 +0100 (CET) Received: from mail-oi1-f200.google.com (mail-oi1-f200.google.com [209.85.167.200]) (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 smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id DA015423C0 for ; Tue, 15 Nov 2022 08:19:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1668500359; bh=vu/KChqgvTXzVuF6Q4d04SAbS9ImcJCelI22IsZPdws=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=CdGsSW/smYnJmk6+ajodVeq0oRfgswSn9aZit/361/Jj0GkCv74ukOFpaMr53Qr4q koFadtxO4RZhdnSwp6rKwvw5Mio00YHVZoEGxc3bIWxymqo/WxBNhP8OobYy61nc3Y I6VDulsSa0/BXNsaFHaDiuWlSLwERCuEJTT/2fZEBulY1DPiBKEwq/AhGBr6GqHi2s 88EUbY7tPP0b/MZTLwGvJl306gh2C1/ByBs1/IRmA43zZ5EfkFtKD8l9iiw8rvMyun YYaPoWqD1HMyyHD8kWQ1eUW/Pfguxos4BS59JrUOqLxn4UeNaezAN1RRyJjwb5TSyg 1IGU8ma+vdBUQ== Received: by mail-oi1-f200.google.com with SMTP id k26-20020a54469a000000b0035ac1417866so3717803oic.18 for ; Tue, 15 Nov 2022 00:19:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vu/KChqgvTXzVuF6Q4d04SAbS9ImcJCelI22IsZPdws=; b=BEIAGJYudT3LpjfFaxaIN/gaus3r2oY22Vdj8qLly9w7fnhtht8jDwIW372B2ZhZCV kTq4ci8fAQn9riShUzGjE0kgXIzB3X4upa0LVKjuX0kOaBZhB/dSMOZxDoniPS0sYQcM gJ8+Uv2C/y0w24YulwjtPiFMEL2frF8zCGMXr9EYzoMOMsARYRkrjceFh+1pvXhLLbqE 4686ga5b/oRsv7M1IoVlhLbALSTeOooO58wEQRcbklanByFyzxtpU8nl8vqaSk/qcQ0V g/yzihzXznMMfTlHM8X+M3J5uUCW+jjQ/xzOa77VM67gnZf/TBR481FV8YQrXdh+HFh3 88dw== X-Gm-Message-State: ANoB5pkOV7nAYK96cGNCgmDiTxQPGDPwX20GcItSw+hkIU8KdtqPEUHX UCtfoouApyxaAGDF2Qz8ul58oi24cmiIch5EQvQvn/y/EeCRDgzEB9pl/HD7EGB9l1wZ9rNFYkN JzW8PT4f3pnTNSyUFT01CjxZbZAGMOIFNKmNX0ULA X-Received: by 2002:a05:6808:20a2:b0:354:74a1:d0a with SMTP id s34-20020a05680820a200b0035474a10d0amr2029oiw.204.1668500358412; Tue, 15 Nov 2022 00:19:18 -0800 (PST) X-Google-Smtp-Source: AA0mqf4w8fcGuDRdyxIBB3ikCrcFiaYQa7ySk7rsjqCmHraXo0Njqc+NMmbhNbBsTMJZASDo6Qqebg53BALuSJNhP5A= X-Received: by 2002:a05:6808:20a2:b0:354:74a1:d0a with SMTP id s34-20020a05680820a200b0035474a10d0amr2023oiw.204.1668500358136; Tue, 15 Nov 2022 00:19:18 -0800 (PST) MIME-Version: 1.0 References: <20221111183042.2589236-1-dmitry.kozliuk@gmail.com> In-Reply-To: <20221111183042.2589236-1-dmitry.kozliuk@gmail.com> From: Christian Ehrhardt Date: Tue, 15 Nov 2022 09:18:52 +0100 Message-ID: Subject: Re: [PATCH 19.11] eal: fix side effect in some pointer arithmetic macros To: Dmitry Kozlyuk Cc: stable@dpdk.org, =?UTF-8?Q?Morten_Br=C3=B8rup?= , Bruce Richardson , Chengwen Feng Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Fri, Nov 11, 2022 at 7:31 PM Dmitry Kozlyuk w= rote: > > [ upstream commit 1a7374c95678183aef6f40b64074752831a33d26 ] Thanks, applied to the WIP branch - expect it to be part of 19.11.14 unless some builds stumble over it. > RTE_PTR_SUB(ptr, x) and RTE_PTR_ALIGN_FLOOR() worked incorrectly > if "ptr" was an expression: > > uint32_t arr[3]; > > RTE_PTR_SUB(arr + 1, sizeof(arr[0])); > // expected: (uint32_t *)((uintptr_t)(arr + 1) - 4) =3D=3D arr > // actual: (uint32_t *)((uintptr_t) arr + 1 - 4) !=3D arr > > RTE_PTR_ALIGN_FLOOR(arr + 2, sizeof(arr[0])); > // expected: RTE_ALIGN_FLOOR((uintptr_t)(arr + 2), 4) =3D=3D &arr[2] > // actual: RTE_ALIGN_FLOOR((uintptr_t) arr + 2, 4) =3D=3D &arr[0] > > Fix the macros and extend the relevant unit test. > Convert uses of a custom test failure macro to RTE_TEST_ASSERT*(). > > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org > > Signed-off-by: Dmitry Kozlyuk > Reviewed-by: Morten Br=C3=B8rup > Acked-by: Bruce Richardson > Acked-by: Chengwen Feng > --- > app/test/test_common.c | 54 +++++++++++++++------- > lib/librte_eal/common/include/rte_common.h | 4 +- > 2 files changed, 40 insertions(+), 18 deletions(-) > > diff --git a/app/test/test_common.c b/app/test/test_common.c > index 12bd1cad90..181c8be3be 100644 > --- a/app/test/test_common.c > +++ b/app/test/test_common.c > @@ -25,27 +25,49 @@ test_macros(int __rte_unused unused_parm) > #define SMALLER 0x1000U > #define BIGGER 0x2000U > #define PTR_DIFF BIGGER - SMALLER > -#define FAIL_MACRO(x)\ > - {printf(#x "() test failed!\n");\ > - return -1;} > > uintptr_t unused =3D 0; > + uint32_t arr[3]; > > RTE_SET_USED(unused); > > - if ((uintptr_t)RTE_PTR_ADD(SMALLER, PTR_DIFF) !=3D BIGGER) > - FAIL_MACRO(RTE_PTR_ADD); > - if ((uintptr_t)RTE_PTR_SUB(BIGGER, PTR_DIFF) !=3D SMALLER) > - FAIL_MACRO(RTE_PTR_SUB); > - if (RTE_PTR_DIFF(BIGGER, SMALLER) !=3D PTR_DIFF) > - FAIL_MACRO(RTE_PTR_DIFF); > - if (RTE_MAX(SMALLER, BIGGER) !=3D BIGGER) > - FAIL_MACRO(RTE_MAX); > - if (RTE_MIN(SMALLER, BIGGER) !=3D SMALLER) > - FAIL_MACRO(RTE_MIN); > - > - if (strncmp(RTE_STR(test), "test", sizeof("test"))) > - FAIL_MACRO(RTE_STR); > + RTE_TEST_ASSERT_EQUAL((uintptr_t)RTE_PTR_ADD(SMALLER, PTR_DIFF), = BIGGER, > + "RTE_PTR_ADD"); > + RTE_TEST_ASSERT_EQUAL((uintptr_t)RTE_PTR_SUB(BIGGER, PTR_DIFF), S= MALLER, > + "RTE_PTR_SUB"); > + RTE_TEST_ASSERT_EQUAL(RTE_PTR_DIFF(BIGGER, SMALLER), PTR_DIFF, > + "RTE_PTR_DIFF"); > + RTE_TEST_ASSERT_EQUAL(RTE_MAX(SMALLER, BIGGER), BIGGER, > + "RTE_MAX"); > + RTE_TEST_ASSERT_EQUAL(RTE_MIN(SMALLER, BIGGER), SMALLER, > + "RTE_MIN"); > + > + RTE_TEST_ASSERT_EQUAL(RTE_PTR_ADD(arr + 1, sizeof(arr[0])), &arr[= 2], > + "RTE_PTR_ADD(expr, x)"); > + RTE_TEST_ASSERT_EQUAL(RTE_PTR_SUB(arr + 1, sizeof(arr[0])), &arr[= 0], > + "RTE_PTR_SUB(expr, x)"); > + RTE_TEST_ASSERT_EQUAL(RTE_PTR_ALIGN_FLOOR(arr + 2, 4), &arr[2], > + "RTE_PTR_ALIGN_FLOOR(expr, x)"); > + RTE_TEST_ASSERT_EQUAL(RTE_PTR_ALIGN_CEIL(arr + 2, 4), &arr[2], > + "RTE_PTR_ALIGN_CEIL(expr, x)"); > + RTE_TEST_ASSERT_EQUAL(RTE_PTR_ALIGN(arr + 2, 4), &arr[2], > + "RTE_PTR_ALIGN(expr, x)"); > + > + RTE_TEST_ASSERT_EQUAL( > + RTE_PTR_ALIGN_FLOOR(RTE_PTR_ADD(&arr[1], 1), 4), &arr[1], > + "RTE_PTR_ALIGN_FLOOR(x < y/2, y)"); > + RTE_TEST_ASSERT_EQUAL( > + RTE_PTR_ALIGN_FLOOR(RTE_PTR_ADD(&arr[1], 3), 4), &arr[1], > + "RTE_PTR_ALIGN_FLOOR(x > y/2, y)"); > + RTE_TEST_ASSERT_EQUAL( > + RTE_PTR_ALIGN_CEIL(RTE_PTR_ADD(&arr[1], 3), 4), &arr[2], > + "RTE_PTR_ALIGN_CEIL(x < y/2, y)"); > + RTE_TEST_ASSERT_EQUAL( > + RTE_PTR_ALIGN_CEIL(RTE_PTR_ADD(&arr[1], 1), 4), &arr[2], > + "RTE_PTR_ALIGN_CEIL(x > y/2, y)"); > + > + RTE_TEST_ASSERT(strncmp(RTE_STR(test), "test", sizeof("test")) = =3D=3D 0, > + "RTE_STR"); > > return 0; > } > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/= common/include/rte_common.h > index fc938eadcd..abe6bc9d15 100644 > --- a/lib/librte_eal/common/include/rte_common.h > +++ b/lib/librte_eal/common/include/rte_common.h > @@ -172,7 +172,7 @@ static void __attribute__((destructor(RTE_PRIO(prio))= , used)) func(void) > /** > * subtract a byte-value offset from a pointer > */ > -#define RTE_PTR_SUB(ptr, x) ((void*)((uintptr_t)ptr - (x))) > +#define RTE_PTR_SUB(ptr, x) ((void *)((uintptr_t)(ptr) - (x))) > > /** > * get the difference between two pointer values, i.e. how far apart > @@ -197,7 +197,7 @@ static void __attribute__((destructor(RTE_PRIO(prio))= , used)) func(void) > * must be a power-of-two value. > */ > #define RTE_PTR_ALIGN_FLOOR(ptr, align) \ > - ((typeof(ptr))RTE_ALIGN_FLOOR((uintptr_t)ptr, align)) > + ((typeof(ptr))RTE_ALIGN_FLOOR((uintptr_t)(ptr), align)) > > /** > * Macro to align a value to a given power-of-two. The resultant value > -- > 2.33.1 > --=20 Christian Ehrhardt Senior Staff Engineer, Ubuntu Server Canonical Ltd