Enum size checks in Base.h (UINT32 not ISO C compatible)

Enum size checks in Base.h (UINT32 not ISO C compatible)

[Rebecca Cran]

In commit 6440385b17def888544c2454ffba58384b929a22 
(https://github.com/tianocore/edk2/commit/6440385b17def888544c2454ffba58384b929a22) 
enum size checks were added.

However, according to gcc, ISO C restricts the size of enum values to 
int; building with -std=c11 -pedantic results in the error:

MdePkg/Include/Base.h:812:28: error: ISO C restricts enumerator values 
to range of ‘int’ [-Werror=pedantic]
   812 |   __VerifyInt32EnumValue = 0xffffffff

Replacing 0xffffffff with 0x7fffffff fixes the problem.

It looks like this might change in C23, but since the use of 
_Static_assert in Base.h we require at least C11 (and I suspect most 
compilers aren't C23 compliant) which states:

"The expression that defines the value of an enumeration constant shall 
be an integer constant expression that has a value representable as an int.
[Section 6.7.2.2]"

The UEFI Specification appears to say we require an INT32 (i.e. a signed 
int) so is the existing code wrong?

-- 
Rebecca Cran

Re: Enum size checks in Base.h (UINT32 not ISO C compatible)

[Michael D Kinney]

Hi Rebecca,

Great catch!!!  I think the static assert verifier is incorrect.

The UEFI Spec does clearly state in Section 2.3.1 that enum values
can be type INT32 or UINT32.  The use of 0xFFFFFFFF assumes use of
only UINT32.  I agree that the correct value to assign to the 32-bit
enum value for this size check should be 0x7FFFFFFFF.

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Wednesday, February 8, 2023 6:20 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: Enum size checks in Base.h (UINT32 not ISO C compatible)
> 
> In commit 6440385b17def888544c2454ffba58384b929a22
> (https://github.com/tianocore/edk2/commit/6440385b17def888544c2454ffba58384b929a22)
> enum size checks were added.
> 
> However, according to gcc, ISO C restricts the size of enum values to
> int; building with -std=c11 -pedantic results in the error:
> 
> MdePkg/Include/Base.h:812:28: error: ISO C restricts enumerator values
> to range of ‘int’ [-Werror=pedantic]
>    812 |   __VerifyInt32EnumValue = 0xffffffff
> 
> Replacing 0xffffffff with 0x7fffffff fixes the problem.
> 
> It looks like this might change in C23, but since the use of
> _Static_assert in Base.h we require at least C11 (and I suspect most
> compilers aren't C23 compliant) which states:
> 
> "The expression that defines the value of an enumeration constant shall
> be an integer constant expression that has a value representable as an int.
> [Section 6.7.2.2]"
> 
> The UEFI Specification appears to say we require an INT32 (i.e. a signed
> int) so is the existing code wrong?
> 
> --
> Rebecca Cran

Re: Enum size checks in Base.h (UINT32 not ISO C compatible)

[Rebecca Cran]

Given the comment above it, I was looking in the 2.3 specification: 
mention of UINT32 was only introduced in UEFI 2.3.1 Errata C.

The revision history states:

913 Enum definition does not match what our current compilers implement.


I'll send a patch to fix it.


-- 
Rebecca Cran


On 2/8/23 19:28, Kinney, Michael D wrote:
> Hi Rebecca,
>
> Great catch!!!  I think the static assert verifier is incorrect.
>
> The UEFI Spec does clearly state in Section 2.3.1 that enum values
> can be type INT32 or UINT32.  The use of 0xFFFFFFFF assumes use of
> only UINT32.  I agree that the correct value to assign to the 32-bit
> enum value for this size check should be 0x7FFFFFFFF.
>
> Mike
>
>> -----Original Message-----
>> From: Rebecca Cran <rebecca@bsdio.com>
>> Sent: Wednesday, February 8, 2023 6:20 PM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
>> Subject: Enum size checks in Base.h (UINT32 not ISO C compatible)
>>
>> In commit 6440385b17def888544c2454ffba58384b929a22
>> (https://github.com/tianocore/edk2/commit/6440385b17def888544c2454ffba58384b929a22)
>> enum size checks were added.
>>
>> However, according to gcc, ISO C restricts the size of enum values to
>> int; building with -std=c11 -pedantic results in the error:
>>
>> MdePkg/Include/Base.h:812:28: error: ISO C restricts enumerator values
>> to range of ‘int’ [-Werror=pedantic]
>>     812 |   __VerifyInt32EnumValue = 0xffffffff
>>
>> Replacing 0xffffffff with 0x7fffffff fixes the problem.
>>
>> It looks like this might change in C23, but since the use of
>> _Static_assert in Base.h we require at least C11 (and I suspect most
>> compilers aren't C23 compliant) which states:
>>
>> "The expression that defines the value of an enumeration constant shall
>> be an integer constant expression that has a value representable as an int.
>> [Section 6.7.2.2]"
>>
>> The UEFI Specification appears to say we require an INT32 (i.e. a signed
>> int) so is the existing code wrong?
>>
>> --
>> Rebecca Cran

Re: Enum size checks in Base.h (UINT32 not ISO C compatible)

[Rebecca Cran]

I've sent a patch (https://edk2.groups.io/g/devel/message/99903).

On 2/8/23 19:28, Kinney, Michael D wrote:
> Hi Rebecca,
> 
> Great catch!!!  I think the static assert verifier is incorrect.
> 
> The UEFI Spec does clearly state in Section 2.3.1 that enum values
> can be type INT32 or UINT32.  The use of 0xFFFFFFFF assumes use of
> only UINT32.  I agree that the correct value to assign to the 32-bit
> enum value for this size check should be 0x7FFFFFFFF.
> 
> Mike
> 
>> -----Original Message-----
>> From: Rebecca Cran <rebecca@bsdio.com>
>> Sent: Wednesday, February 8, 2023 6:20 PM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
>> Subject: Enum size checks in Base.h (UINT32 not ISO C compatible)
>>
>> In commit 6440385b17def888544c2454ffba58384b929a22
>> (https://github.com/tianocore/edk2/commit/6440385b17def888544c2454ffba58384b929a22)
>> enum size checks were added.
>>
>> However, according to gcc, ISO C restricts the size of enum values to
>> int; building with -std=c11 -pedantic results in the error:
>>
>> MdePkg/Include/Base.h:812:28: error: ISO C restricts enumerator values
>> to range of ‘int’ [-Werror=pedantic]
>>     812 |   __VerifyInt32EnumValue = 0xffffffff
>>
>> Replacing 0xffffffff with 0x7fffffff fixes the problem.
>>
>> It looks like this might change in C23, but since the use of
>> _Static_assert in Base.h we require at least C11 (and I suspect most
>> compilers aren't C23 compliant) which states:
>>
>> "The expression that defines the value of an enumeration constant shall
>> be an integer constant expression that has a value representable as an int.
>> [Section 6.7.2.2]"
>>
>> The UEFI Specification appears to say we require an INT32 (i.e. a signed
>> int) so is the existing code wrong?
>>
>> --
>> Rebecca Cran