From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DF7B7A10DA for ; Wed, 31 Jul 2019 10:16:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 38DC31C0DC; Wed, 31 Jul 2019 10:16:44 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 5D5341C0D6 for ; Wed, 31 Jul 2019 10:16:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jul 2019 01:16:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,329,1559545200"; d="scan'208";a="174085284" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga007.fm.intel.com with ESMTP; 31 Jul 2019 01:16:40 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX152.ger.corp.intel.com (163.33.192.66) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 31 Jul 2019 09:16:39 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.164]) by irsmsx155.ger.corp.intel.com ([169.254.14.201]) with mapi id 14.03.0439.000; Wed, 31 Jul 2019 09:16:39 +0100 From: "Ananyev, Konstantin" To: Aaron Conole , "dev@dpdk.org" CC: "De Lara Guarch, Pablo" Thread-Topic: [PATCH] librte_acl: fix undefined behavior Thread-Index: AQHVRx9CqdL5XdPsqUqzJ2HuXWENCqbkWHEA Date: Wed, 31 Jul 2019 08:16:38 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580168A5FE40@irsmsx105.ger.corp.intel.com> References: <20190730213920.17225-1-aconole@redhat.com> In-Reply-To: <20190730213920.17225-1-aconole@redhat.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTlmMzgxYjctZDI1My00ZDZmLWE0ZmUtOGRkYWYxZjk0Nzc0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUXhSXC9kUFZGcmdlZ0xIenZlYUtHS3BqUDdHZlpoclUrTG95ZDdGSUFFb09kT1ZheVAyeGRHSHdleW1CdXk4MHYifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] librte_acl: fix undefined behavior X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Aaron, >=20 > Left-shift of an integer constant is represented as 'int' type, but a lef= t > shift of 1 by 31 bits in 'int' is undefined. Use the U suffix to force > a representation as unsigned. >=20 > Caught while running with ubsan under gcc. >=20 > Fixes: dc276b5780c2 ("acl: new library") > Cc: Konstantin Ananyev > Signed-off-by: Aaron Conole > --- > I could have changed the sizeof(bits_t) * 8 in the bitset.bits as well du= ring > the cleanup, but chose not to to keep the change minimal. But it seems that you did change it in some places: =20 > - 1 << (n % (sizeof(bits_t) * 8)); > + 1U << (n % (sizeof(bits_t) * CHAR_BIT)); While in others kept unchanged: > - (1 << (n % (sizeof(bits_t) * 8)))) { > + (1U << (n % (sizeof(bits_t) * 8)))) { Was that intended? Konstantin >=20 > lib/librte_acl/acl_bld.c | 6 +++--- > lib/librte_acl/acl_gen.c | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) >=20 > diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c > index b82191f42..9d27c0a5a 100644 > --- a/lib/librte_acl/acl_bld.c > +++ b/lib/librte_acl/acl_bld.c > @@ -320,7 +320,7 @@ acl_add_ptr_range(struct acl_build_context *context, > for (n =3D 0; n < UINT8_MAX + 1; n++) > if (n >=3D low && n <=3D high) > bitset.bits[n / (sizeof(bits_t) * 8)] |=3D > - 1 << (n % (sizeof(bits_t) * 8)); > + 1U << (n % (sizeof(bits_t) * CHAR_BIT)); >=20 > return acl_add_ptr(context, root, node, &bitset); > } > @@ -343,7 +343,7 @@ acl_gen_mask(struct rte_acl_bitset *bitset, uint32_t = value, uint32_t mask) > if ((n & mask) =3D=3D value) { > range++; > bitset->bits[n / (sizeof(bits_t) * 8)] |=3D > - 1 << (n % (sizeof(bits_t) * 8)); > + 1U << (n % (sizeof(bits_t) * CHAR_BIT)); > } > } > return range; > @@ -972,7 +972,7 @@ build_trie(struct acl_build_context *context, struct = rte_acl_build_rule *head, > sizeof(*end->mrt)); >=20 > for (m =3D context->cfg.num_categories; 0 !=3D m--; ) { > - if (rule->f->data.category_mask & (1 << m)) { > + if (rule->f->data.category_mask & (1U << m)) { > end->mrt->results[m] =3D rule->f->data.userdata; > end->mrt->priority[m] =3D rule->f->data.priority; > } else { > diff --git a/lib/librte_acl/acl_gen.c b/lib/librte_acl/acl_gen.c > index 35a0140b4..81dec3aa6 100644 > --- a/lib/librte_acl/acl_gen.c > +++ b/lib/librte_acl/acl_gen.c > @@ -133,7 +133,7 @@ acl_node_fill_dfa(const struct rte_acl_node *node, > for (n =3D 0; n < RTE_ACL_DFA_SIZE; n++) { >=20 > if (bits->bits[n / (sizeof(bits_t) * CHAR_BIT)] & > - (1 << (n % (sizeof(bits_t) * CHAR_BIT)))) { > + (1U << (n % (sizeof(bits_t) * CHAR_BIT)))) { >=20 > dfa[n] =3D resolved ? child->node_index : x; > ranges +=3D (last_bit =3D=3D 0); > @@ -175,7 +175,7 @@ acl_count_sequential_groups(struct rte_acl_bitset *bi= ts, int zero_one) > } > for (n =3D 0; n < QRANGE_MIN; n++) { > if (bits->bits[n / (sizeof(bits_t) * 8)] & > - (1 << (n % (sizeof(bits_t) * 8)))) { > + (1U << (n % (sizeof(bits_t) * 8)))) { > if (zero_one =3D=3D 1 && last_bit !=3D 1) > ranges++; > last_bit =3D 1; > -- > 2.21.0