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 E642BA00C2; Mon, 22 Aug 2022 09:14:59 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7E2AE41132; Mon, 22 Aug 2022 09:14:59 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 6359D410FA; Mon, 22 Aug 2022 09:14:58 +0200 (CEST) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH 1/3] eal: fix pointer arithmetic with an expression argument Date: Mon, 22 Aug 2022 09:14:55 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D8728D@smartserver.smartshare.dk> In-Reply-To: <20220821205009.1317044-2-dmitry.kozliuk@gmail.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 1/3] eal: fix pointer arithmetic with an expression argument Thread-Index: Adi1n6yIADXnCxdCRJSvv2StoAEcTAAVceug References: <20220821205009.1317044-1-dmitry.kozliuk@gmail.com> <20220821205009.1317044-2-dmitry.kozliuk@gmail.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Dmitry Kozlyuk" , Cc: X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Dmitry Kozlyuk [mailto:dmitry.kozliuk@gmail.com] > Sent: Sunday, 21 August 2022 22.50 >=20 > RTE_PTR_SUB(ptr, x) and RTE_PTR_ALIGN_FLOOR() worked incorrectly > if "ptr" was an expression: >=20 > uint32_t arr[3]; >=20 > 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 >=20 > 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] >=20 > Fix the macros and extend the relevant unit test. Good catch. Serious bugs! >=20 > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org >=20 > Signed-off-by: Dmitry Kozlyuk > --- > app/test/test_common.c | 11 +++++++++++ > lib/eal/include/rte_common.h | 4 ++-- > 2 files changed, 13 insertions(+), 2 deletions(-) >=20 > diff --git a/app/test/test_common.c b/app/test/test_common.c > index ef177cecb1..4194c1208a 100644 > --- a/app/test/test_common.c > +++ b/app/test/test_common.c > @@ -31,6 +31,7 @@ test_macros(int __rte_unused unused_parm) >=20 > uintptr_t unused =3D 0; > unsigned int smaller =3D SMALLER, bigger =3D BIGGER; > + uint32_t arr[3]; >=20 > RTE_SET_USED(unused); >=20 > @@ -41,6 +42,16 @@ test_macros(int __rte_unused unused_parm) > FAIL_MACRO(RTE_PTR_ADD); > if ((uintptr_t)RTE_PTR_SUB(BIGGER, PTR_DIFF) !=3D SMALLER) > FAIL_MACRO(RTE_PTR_SUB); > + if (RTE_PTR_ADD(arr + 1, sizeof(arr[0])) !=3D &arr[2]) > + FAIL_MACRO(RTE_PTR_ADD); > + if (RTE_PTR_SUB(arr + 1, sizeof(arr[0])) !=3D &arr[0]) > + FAIL_MACRO(RTE_PTR_SUB); Very elegant test cases. :-) > + if (RTE_PTR_ALIGN_FLOOR(arr + 2, 4) !=3D &arr[2]) > + FAIL_MACRO(RTE_PTR_ALIGN_FLOOR); > + if (RTE_PTR_ALIGN_CEIL(arr + 2, 4) !=3D &arr[2]) > + FAIL_MACRO(RTE_PTR_ALIGN_CEIL); While you are at it, consider adding a few more test cases, e.g. RTE_PTR_ALIGN_FLOOR/CEIL(RTE_PTR_ADD(&arr[1], 1), 4), and RTE_PTR_ALIGN_FLOOR/CEIL(RTE_PTR_ADD(&arr[1], sizeof(uint32_t) - 1), 4) > + if (RTE_PTR_ALIGN(arr + 2, 4) !=3D &arr[2]) > + FAIL_MACRO(RTE_PTR_ALIGN); > if (RTE_PTR_DIFF(BIGGER, SMALLER) !=3D PTR_DIFF) > FAIL_MACRO(RTE_PTR_DIFF); > if (RTE_MAX(SMALLER, BIGGER) !=3D BIGGER) > diff --git a/lib/eal/include/rte_common.h > b/lib/eal/include/rte_common.h > index a96cc2a138..d517e9f75f 100644 > --- a/lib/eal/include/rte_common.h > +++ b/lib/eal/include/rte_common.h > @@ -295,7 +295,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))) >=20 > /** > * get the difference between two pointer values, i.e. how far apart > @@ -320,7 +320,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)) >=20 > /** > * Macro to align a value to a given power-of-two. The resultant = value > -- > 2.33.1 >=20 Reviewed-by: Morten Br=F8rup