Re: [DPDK/core Bug 1409] arparse library assumes enum are 64 bit

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

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.

ty

> -- 
> You are receiving this mail because:
> You are the assignee for the bug.