Re: [edk2][what type is an enum, can UEFI really specify one?] Re: XhciSched.c:916: warning: ‘AsyncUrb’ may be used uninitialized

Subject: Re: [edk2][what type is an enum, can UEFI really specify one?] Re: XhciSched.c:916: warning: ‘AsyncUrb’ may be used uninitialized

From: "Sun, Rui" <rui.sun@intel.com>

To: "edk2-devel@lists.sourceforge.net" <edk2-devel@lists.sourceforge.net>

Date: 2012-04-13 11:40:41

Andrew,

Theoretically, it is a binary interoperability issue. For example, if the c=
ompiler for the caller treats an enumerated type as CHAR and the compiler f=
or the caller treats the enumerated type as Integer, then there would be a =
problem. I agree with you that practically it is not a so bad problem.

I verified that type cast works with GCC44. Let's see the input from your c=
ompiler team:)


-----Original Message-----
From: Andrew Fish [mailto:afish@apple.com] =

Sent: Thursday, April 12, 2012 11:17 PM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [what type is an enum, can UEFI really specify one?] Re=
: XhciSched.c:916: warning: =91AsyncUrb=92 may be used uninitialized

On Apr 12, 2012, at 1:57 AM, Sun, Rui wrote:

> Andrew,
> =

> You are correct. I got warnings when compiling the PCI bus driver using G=
CC44 with -Wtype-limits.
> =

> Enum type is used as parameters for some UEFI services and protocols. Do =
you think this is a binary interoperability issue as the choice of type is =
implementation-defined?
> =


I don't think the problems are too bad. When I suppressed the warning and o=
utputted assembly from clang the code seemed to be the same even when optim=
ization is turned on. I want to check with the compiler team to make sure t=
his is not a bad assumption.  It seems the statement in the UEFI spec is no=
t possible to implement with an ANSI C compiler, but the enums in UEFI are =
unsigned so the binary values are the same for a signed and unsigned type. =
 It seems the only issue is in the code that is error checking the enum ran=
ge. =


I noticed I can cast the warning away in clang.

((INT32)UnsignedEnum)

Does this work in gcc? At some point in the past this did not work for gcc =
and clang, but it looks like clang fixed the bug. =


The cast suppresses the warning, but also should ensure the compiler code g=
eneration is correct. So I think the cast is probably the right thing to do=
 in the code. =


Thanks,

Andrew


> -----Original Message-----
> From: Andrew Fish [mailto:afish@apple.com] =

> Sent: Thursday, April 12, 2012 1:17 AM
> To: edk2-devel@lists.sourceforge.net
> Subject: [edk2] [what type is an enum, can UEFI really specify one?] Re: =
XhciSched.c:916: warning: =91AsyncUrb=92 may be used uninitialized
> =

> =

> On Apr 11, 2012, at 1:48 AM, Sun, Rui wrote:
> =

>> Hi,
>> =

>> The UEFI spec defines  as type of INT32. Refer to the s=
ection 2.3.1 Data Types in the UEFI 2.3.1 Errata A spec.
>> =

> =

> It looks to me like the only way to make an enum type INT32 in ANSI C is =
to add a negative element to the enum.
> =

>> It is observed that most compilers treat Enum as INT32 except XCode. Doe=
s XCode support a command line option to specify Enum as INT32?
>> =

> =

> I think your observation is in error. It is my understanding that both gc=
c and clang pick the sign of the enum value based on a "best match" for the=
 enumerated values, per ANSI C. You are not seeing warnings on gcc since th=
ey are not turned on. Try adding -Wextra to gcc and I think you will start =
seeing the same errors as Xcode/clang. I would not be surprised to find tha=
t Visual Studio does that same thing, but choses not to warn to support leg=
acy code frameworks and include files. =

> =

> This discusion, http://arstechnica.com/civis/viewtopic.php?f=3D20&t=3D113=
0118 , about gcc seems to track with what I've seen with Xcode/clang. =

> =

> While gcc turns on the warning with -Wextra, clang has it on by default. =
See the following example (maybe some one could test on gcc): This example =
shows that the sign of enum is chosen by the compiler base on the range of =
values as per ANSI C. I think if you try the current gcc and turn on -Wextr=
a you will get similar results. =

> =

> ~/work/Compiler>cat enum.c =

> =

> typedef enum {
>  ENUM_a,
>  ENUM_b,
>  ENUM_Max
> } ENUM_TEST;
> =

> typedef enum {
>  SENUM_a,
>  SENUM_b,
>  SENUM_Max,
>  SENUM_FORCE_SIGNED =3D -1
> } ENUM_TEST_SIGNED;
> =

> int
> TestSignedEnumCode (
>  ENUM_TEST_SIGNED  SignedEnum,
>  )
> {
>  if ((SignedEnum < 0) || (SignedEnum >=3DENUM_Max)) {
>    return -1;
>  }
>  return 0;
> }
> =

> int
> TestUnsingedEnumCode (
>  ENUM_TEST         UnsignedEnum
>  )
> {
>  if ((UnsignedEnum < 0) || (UnsignedEnum >=3DSENUM_Max)) {
>    return -1;
>  }
>  return 0;
> ~/work/Compiler>clang -S enum.c =

> enum.c:31:21: warning: comparison of unsigned enum expression < 0 is alwa=
ys false
>      [-Wtautological-compare]
>  if ((UnsignedEnum < 0) || (UnsignedEnum >=3DSENUM_Max)) {
>       ~~~~~~~~~~~~ ^ ~
> 1 warning generated.
> =

> ~/work/Compiler>clang -S enum.c -arch i386 -Oz -Wno-tautological-compare
> ~/work/Compiler>cat enum.s
> 	.section	__TEXT,__text,regular,pure_instructions
> 	.globl	_TestSignedEnumCode
> _TestSignedEnumCode:                    ## @TestSignedEnumCode
> ## BB#0:
> 	pushl	%ebp
> 	movl	%esp, %ebp
> 	movl	$-1, %ecx
> 	cmpl	$1, 8(%ebp)
> 	movl	$0, %eax
> 	cmoval	%ecx, %eax
> 	popl	%ebp
> 	ret
> =

> 	.globl	_TestUnsingedEnumCode
> _TestUnsingedEnumCode:                  ## @TestUnsingedEnumCode
> ## BB#0:
> 	pushl	%ebp
> 	movl	%esp, %ebp
> 	movl	$-1, %ecx
> 	cmpl	$1, 8(%ebp)
> 	movl	$0, %eax
> 	cmoval	%ecx, %eax
> 	popl	%ebp
> 	ret
> =

> =

> .subsections_via_symbols
> =

> =

>> -----Original Message-----
>> From: Andrew Fish [mailto:afish@apple.com] =

>> Sent: Monday, April 02, 2012 5:49 AM
>> To: edk2-devel@lists.sourceforge.net
>> Subject: Re: [edk2] XhciSched.c:916: warning: =91AsyncUrb=92 may be used=
 uninitialized
>> =

>> =

>> On Apr 1, 2012, at 2:45 PM, Jordan Justen wrote:
>> =

>>> On Sun, Apr 1, 2012 at 09:29, Andrew Fish  wrote:
>>>> I was going to test with clang, Xcode 4.3 compiler, but I hit some err=
ors.
>>>> =

>>>> Per ANSI C spec:
>>>> Each enumerated type shall be compatible with char, a signed integer t=
ype, or an unsigned integer type. The choice of type is implementation-defi=
ned,110) but shall be capable of representing the values of all the members=
 of the enumeration. The enumerated type is incomplete until after the } th=
at terminates the list of enumerator declarations.
>>>> =

>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:88:13:=
 error: comparison of unsigned enum expression < 0 is always false [-Werror=
,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>> =

>>> Would something like this work?
>>> if ((UINTN) Width >=3D EfiPciIoWidthMaximum) {
>>> =

>> =

>> Yes that works with clang, and is correct per ANSI C.
>> =

>>> -Jordan
>>> =

>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:149:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:218:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:325:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width > EfiPciIoWidthUint64) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:424:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:503:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:581:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:659:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:884:13=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Width < 0 || Width >=3D EfiPciIoWidthMaximum) {
>>>>    ~~~~~ ^ ~
>>>> /Users/fish/work/edk2TOT/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c:973:17=
: error: comparison of unsigned enum expression < 0 is always false [-Werro=
r,-Wtautological-compare]
>>>> if (Operation < 0 || Operation >=3D EfiPciIoOperationMaximum) {
>>>>    ~~~~~~~~~ ^ ~
>>>> 10 errors generated.
>>>> =

>>>> Andrew Fish
>>>> =

>>>> =

>>>> =

>>>> =

>>>> =

>>>> On Mar 31, 2012, at 7:18 PM, Tian, Feng wrote:
>>>> =

>>>>> Which tool chain and build option are you using?
>>>>> =

>>>>> Our build system says it's passed.
>>>>> =

>>>>> and I checked code, it's impossible for this case.
>>>>> =

>>>>> -----Original Message-----
>>>>> From: Sergey Isakov [mailto:isakov-sl@bk.ru]
>>>>> Sent: Saturday, March 31, 2012 23:35
>>>>> To: edk2-devel@lists.sourceforge.net
>>>>> Subject: [edk2] XhciSched.c:916: warning: =91AsyncUrb=92 may be used =
uninitialized
>>>>> =

>>>>> Dear sirs,
>>>>> Is it possible to exclude warnings before commit into trunk?
>>>>> Sergey
>>>>> =

>>>>> =

>>>>> ---------------------------------------------------------------------=
---------
>>>>> This SF email is sponsosred by:
>>>>> Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd=
2d-msazure _______________________________________________
>>>>> edk2-devel mailing list
>>>>> edk2-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>>> =

>>>>> ---------------------------------------------------------------------=
---------
>>>>> This SF email is sponsosred by:
>>>>> Try Windows Azure free for 90 days Click Here
>>>>> http://p.sf.net/sfu/sfd2d-msazure
>>>>> _______________________________________________
>>>>> edk2-devel mailing list
>>>>> edk2-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>> =

>>>> =

>>>> ----------------------------------------------------------------------=
--------
>>>> This SF email is sponsosred by:
>>>> Try Windows Azure free for 90 days Click Here
>>>> http://p.sf.net/sfu/sfd2d-msazure
>>>> _______________________________________________
>>>> edk2-devel mailing list
>>>> edk2-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>> =

>>> -----------------------------------------------------------------------=
-------
>>> This SF email is sponsosred by:
>>> Try Windows Azure free for 90 days Click Here =

>>> http://p.sf.net/sfu/sfd2d-msazure
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>> =

>> =

>> ------------------------------------------------------------------------=
------
>> This SF email is sponsosred by:
>> Try Windows Azure free for 90 days Click Here =

>> http://p.sf.net/sfu/sfd2d-msazure
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>> =

>> ------------------------------------------------------------------------=
------
>> Better than sec? Nothing is better than sec when it comes to
>> monitoring Big Data applications. Try Boundary one-second =

>> resolution app monitoring today. Free.
>> http://p.sf.net/sfu/Boundary-dev2dev
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> =

> =

> -------------------------------------------------------------------------=
-----
> Better than sec? Nothing is better than sec when it comes to
> monitoring Big Data applications. Try Boundary one-second =

> resolution app monitoring today. Free.
> http://p.sf.net/sfu/Boundary-dev2dev
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> =

> -------------------------------------------------------------------------=
-----
> For Developers, A Lot Can Happen In A Second.
> Boundary is the first to Know...and Tell You.
> Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
> http://p.sf.net/sfu/Boundary-d2dvs2
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel


---------------------------------------------------------------------------=
---
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

---------------------------------------------------------------------------=
---
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel