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 DEC8743DAD; Tue, 2 Apr 2024 01:49:53 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 81F394025D; Tue, 2 Apr 2024 01:49:53 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id D346740151; Tue, 2 Apr 2024 01:49:51 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id E1EE0208D086; Mon, 1 Apr 2024 16:49:50 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com E1EE0208D086 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1712015390; bh=W47d8M19mHKS37dGBIu5ldaxWGA+GwBPtYu72sm6LXg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JGwdvoJW1KR4uwjQojNWhWeyUn1N0aJpED+8H6c/Qn8UdJ3A+ELtV5MGpcWnSSKQ9 N8XDLliZcWlzyrUtQDWSp3ePxwNL+M7+zyD+BioziIeplFk9Ev+5bM7iNL7KByPNYW QPaapiBJC10X8BnKyDPQCnOzcKtJ4XUSe1eVXhFY= Date: Mon, 1 Apr 2024 16:49:50 -0700 From: Tyler Retzlaff To: Stephen Hemminger Cc: bugzilla@dpdk.org, dev@dpdk.org Subject: Re: [DPDK/core Bug 1409] arparse library assumes enum are 64 bit Message-ID: <20240401234950.GA1488@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20240401172014.GA5321@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20240401123408.1c05958c@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240401123408.1c05958c@hermes.local> User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Mon, Apr 01, 2024 at 12:34:08PM -0700, Stephen Hemminger wrote: > On Mon, 1 Apr 2024 10:20:14 -0700 > Tyler Retzlaff wrote: > > > On Sat, Mar 30, 2024 at 02:58:41AM +0000, bugzilla@dpdk.org wrote: > > > https://bugs.dpdk.org/show_bug.cgi?id=1409 > > > > > > Bug ID: 1409 > > > Summary: arparse library assumes enum are 64 bit > > > Product: DPDK > > > Version: 24.03 > > > Hardware: All > > > OS: All > > > Status: UNCONFIRMED > > > Severity: normal > > > Priority: Normal > > > Component: core > > > Assignee: dev@dpdk.org > > > Reporter: stephen@networkplumber.org > > > Target Milestone: --- > > > > > > MSVC correctly flags that this line in rte_argparse.h is incorrect: > > > RTE_ARGPARSE_ARG_RESERVED_FIELD = RTE_GENMASK64(63, 48), > > > > > > The problem is that enum values are just an alias for int, and it can be 32 > > > bits. > > > > > > Taken from the current C Standard (C99): > > > http://www.open std.org/JTC1/SC22/WG14/www/docs/n1256.pdf > > > > > > 6.7.2.2 Enumeration specifiers > > > [...] > > > Constraints > > > The expression that defines the value of an enumeration constant shall be an > > > integer constant expression that has a value representable as an int. > > > [...] > > > Each enumerated type shall be compatible with char, a signed integer type, or > > > an unsigned integer type. The choice of type is implementation-defined, but > > > shall be capable of representing the values of all the members of the > > > enumeration. > > > > > > Since rte_argparse only uses 10 bits now. The suggested fix here is to: > > > 1. Assume 32 bits > > > 2. Get rid of the reserved field - reserved fields are bad idea > > > > > > > as some additional information i was aware of this issue and had already > > discussing it internally with the visual studio engineers. we reviewed > > relevant parts of C11 standard we believe there are 2 points of > > interest. > > > > the C11 standard does appear to direct the implementation to select an > > integer wide enough to hold the 64-bit enum value but it is only > > reasonable to do so when the target has a native 64-bit type. i.e. if > > your target has no 64-bit integer than the size of the above constant > > expression will be truncated. > > > > the MSVC compiler requires an extra command line argument to provide > > standard C conformant behavior (/Zc:enumTypes). when used with a C++ TU > > MSVC does select a 64-bit type for the prescribed constant expression > > but does not correctly select a 64-bit type with a C TU (this matters if > > we are exposing this enum type in a public header consumed by C++) > > > > i'm in the process of requesting that /Zc:enumTypes be brought into > > alignment with how it functions with C++ TU. > > > > * /Zc:enumTypes should result in "the same" type being selected for > > C or C++ TU, whatever that type may be. > > > > * /Zc:enumTypes in a C TU should select a 64-bit integer type for the > > above example value when the target supports 64-bit integer natively. > > > > as the current released compiler obviously does not conform to the above > > we may apply a conditionally compiled workaround that declares a 64-bit > > integer with an identifier matching the name of the un-scoped enum > > value. > > All well and good, but the assumption of 64 bit enum's breaks on > 32 bit builds which DPDK still has. The library didn't need the bits. > Just deleting the unused reserved field and changing the shifts to > be 32 fixes the issue. agreed. thanks for raising it.