On Jan 20, 2013, at 2:51 PM, Joe Vernaci <jvern...@gmail.com> wrote:
> Hi Andrew,
>
> That's fine, it's ambiguities that lead to problems like this. When I wrote
> "must be type (signed) int" I was only referring to enumeration constants
> within the enumeration definition.
I agree but when we were working through this issue to update the UEFI spec I
noticed it was not uncommon for folks to confuse this with the size of the enum
type.
> 6.7.2.2 sec 2 states "The expression that deļ¬nes the value of an enumeration
> constant shall be an integer constant expression that has a value
> representable as an int." I implied the missing unsigned as implying signed,
> again the ambiguities in the spec. And from my example below:
> joevext@angryext:~/temp/enumtest$ clang -Weverything main.c
> main.c:22:5: warning: ISO C restricts enumerator values to range of 'int'
> (4294967295 is too large) [-pedantic]
> e_inv_neg_1 = 0xffffffff // <-- invalid but same value as (-1)
> I guess I was wrong since I was using ISO and ANSI as mostly the same.
>
> However you are correct that the enum type can be INT32 or UINT32 (I did
> check the UEFI spec and saw that listed) which I think is where the problem
> with the C spec is. If an enum constant can only be defined with as an int
> but can be typed as a signed or unsigned int where is the limit? Should it
> be all possible enum constant values or all possible enum type values.
>
> You also bring up a good point about the enum size. For instance a simple
> mistake like
> typedef enum {
> foo = 0,
> max_foo = 0xfffffffff // has an extra 'f'
> } bar;
>
> could cause calls to fail since gcc/clang would now treat the enum as a 64
> bits.
>
> I'm not sure if putting any extra restrictions on the EFI ABI other than the
> already listed INT32/UINT32 since as of right now I don't think there is a
> way to enforce it with a simple compiler option or pragma statement. I think
> the only viable way forward is to just specifically recast enums when doing
> ranged constant compares which from a C language standpoint makes sense.
>
The INT32/UINT32 was the language that got added to match what the compilers
were actually doing. As you point out it is important that all the enum values
fit in 32-bits as if they get bigger that will cause a compatibility issue.
> --Joe
>
> On Sun, Jan 20, 2013 at 4:51 PM, Andrew Fish <af...@apple.com> wrote:
>
> On Jan 20, 2013, at 12:18 PM, Joe Vernaci <jvern...@gmail.com> wrote:
>
>> Hi Nikolai,
>>
>> I think your recast as a UINT32 is the correct change. From the UEFI spec
>> 6.2 when allocating "MemoryType values in the range 0x80000000..0xFFFFFFFF
>> are reserved for use by UEFI OS loaders that are provided by operating
>> system vendors. The only illegal memory type values are those in the range
>> EfiMaxMemoryType..0x7FFFFFFF."
>>
>> ANSI and ISO C state that named enumeration constants must be type (signed)
>> int but may be physically stored as either signed int or unsigned int which
>> is implementation specific (ISO 9998:201x 6.7.2.2). Some people interpret
>> this as enums must always be signed int. However the norm seems to be treat
>> it as unsigned unless a negative constant is used in the enum definition.
>> If your compiler is treating this enum as a signed int then yes PoolType
>> will always be less than 0x7fffffff. If you have not changed the
>> EFI_MEMORY_TYPE definition using a negative I'm not sure why it is doing
>> this.
>>
>
> Sorry to get all pedantic but the enum type it's self is an integer type so
> the size and sign are not defined in ANSI C. I recently got the UEFI spec
> updated to point out the enum can be either INT32 or UINT32 when passed as an
> argument. The behavior between compilers on sign is different, luckily it
> seems that int (32-bits) is the common size. It turns out Visual Studio will
> not support values > 32-bit and gcc/clang will. From talking to our compiler
> guys the integer promotion rules of C make an INT32 or UINT32 interchangeable
> when passed as an argument.
>
> So from an UEFI spec point of view it is not safe to assume the sign of a
> enumerated type when making comparisons.
>
> In retrospect we should not have defined arguments to functions in the UEFI
> spec as a typedef of type enum, and we should have just made it a UINT32 (or
> INT32). So the current language in UEFI is not pedantic ANSI C, but it works
> for all the compilers we know about today. Or if you think about us putting
> restrictions on enum is part of the ABI defined by UEFI, just like the
> calling convention.
>
> Thanks,
>
> Andrew Fish
>
>> Here is a simple example:
>> #include <stdio.h>
>>
>> typedef enum {
>> e_s_neg_1 = -1, // <-- valid
>> e_s_pos_1 = 1,
>> e_s_pos_2 = 2
>> } e_s;
>>
>> typedef enum {
>> e_u_pos_1 = 1,
>> e_u_pos_2 = 2
>> } e_u;
>>
>> typedef enum {
>> e_und_0,
>> e_und_pos_1
>> } e_und;
>>
>> typedef enum {
>> e_inv_pos_1 = 1,
>> e_inv_pos_2 = 2,
>> e_inv_neg_1 = 0xffffffff // <-- invalid but same value as (-1)
>> } e_inv;
>>
>> void func_s(e_s e) {
>> if (e <= 0x7fffffff) { // <-- should use test/jns or cmp/jg
>> printf("func_s: 0x%08x\n", e);
>> }
>> }
>>
>> void func_u(e_u e) {
>> if (e <= 0x7fffffff) { // <-- should use test/js or cmp/ja
>> printf("func_u: 0x%08x\n", e);
>> }
>> }
>>
>> void func_und(e_und e) {
>> if (e <= 0x7fffffff) { // <-- should use test/js or cmp/ja
>> printf("func_und: 0x%08x\n", e);
>> }
>> }
>>
>> void func_inv(e_inv e) {
>> if (e <= 0x7fffffff) { // <-- should use test/js or cmp/ja
>> printf("func_inv: 0x%08x\n", e);
>> }
>> }
>>
>> int main(int argc, char **argv) {
>> func_s(-1); // <-- only function that should print
>> func_u(-1);
>> func_und(-1);
>> func_inv(-1);
>> return 0;
>> }
>>
>> --Joe
>>
>> On Sat, Jan 19, 2013 at 1:19 AM, Nikolai Saoukh <n...@otdel-1.org> wrote:
>> I just added handcrafted toolchain (ELFCLANG) to Conf/tools_def.txt.
>> And tried to compile DuetPkg -- DEBUG & IA32. Command lines follows
>>
>> "clang" -ffreestanding -m32 -fshort-wchar -fno-strict-aliasing -Wall
>> -c -include
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/AutoGen.h
>> -D
>> STRING_ARRAY_NAME=DxeCoreStrings -o
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Pool.obj
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/DxeMain
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Dispatcher
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Event
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/FwVol
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/FwVolBlock
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Mem
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Gcd
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Hand
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Library -I/tiano/edk2/
>> MdeModulePkg/Core/Dxe/Misc -I/tiano/edk2/MdeModulePkg/Core/Dxe/Image
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/SectionExtraction
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe -I/tiano/edk2/Build/
>> DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/DEBUG
>> -I/tiano/edk2/MdePkg -I/tiano/edk2/MdePkg/Include
>> -I/tiano/edk2/MdePkg/Include/Ia32 -I/tiano/edk2/MdeModulePkg
>> -I/tiano/edk2/MdeModulePkg/Include
>> /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Pool.c
>> /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Pool.c:188:49: warning:
>> comparison of constant 2147483647 with expression of type
>> 'EFI_MEMORY_TYPE' is always true [-Wtautological-constant-o
>> ut-of-range-compare]
>> if ((PoolType >= EfiMaxMemoryType && PoolType <= 0x7fffffff) ||
>> ~~~~~~~~ ^ ~~~~~~~~~~
>> 1 warning generated.
>> "echo" Symbol renaming not needed for
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Pool.obj
>> Symbol renaming not needed for
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Pool.obj
>> "clang" -ffreestanding -m32 -fshort-wchar -fno-strict-aliasing -Wall
>> -c -include
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/AutoGen.h
>> -DSTRING_ARRAY_NAME=DxeCoreStrings -o
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Page.obj
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/DxeMain
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Dispatcher
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Event
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/FwVol
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/FwVolBlock
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Mem
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Gcd
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Hand
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Library
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Misc
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/Image
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe/SectionExtraction
>> -I/tiano/edk2/MdeModulePkg/Core/Dxe
>> -I/tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/DEBUG
>> -I/tiano/edk2/MdePkg -I/tiano/edk2/MdePkg/Include
>> -I/tiano/edk2/MdePkg/Include/Ia32 -I/tiano/edk2/MdeModulePkg
>> -I/tiano/edk2/MdeModulePkg/Include
>> /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Page.c
>> /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Page.c:528:40: warning:
>> comparison of constant 2147483647 with expression of type
>> 'EFI_MEMORY_TYPE' is always true
>> [-Wtautological-constant-out-of-range-compare]
>> if (Type >= EfiMaxMemoryType && Type <= 0x7fffffff) {
>> ~~~~ ^ ~~~~~~~~~~
>> /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Page.c:1101:53: warning:
>> comparison of constant 2147483647 with expression of type
>> 'EFI_MEMORY_TYPE' is always true
>> [-Wtautological-constant-out-of-range-compare]
>> if ((MemoryType >= EfiMaxMemoryType && MemoryType <= 0x7fffffff) ||
>> ~~~~~~~~~~ ^ ~~~~~~~~~~
>> 2 warnings generated.
>> "echo" Symbol renaming not needed for
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Page.obj
>> Symbol renaming not needed for
>> /tiano/edk2/Build/DuetPkgIA32/DEBUG_ELFCLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Page.obj
>>
>>
>> On Wed, Jan 16, 2013 at 6:21 AM, Tian, Feng <feng.t...@intel.com> wrote:
>> > Hi, Saoukh
>> >
>> >
>> >
>> > What tool-chain and build option are you using? Are you changing
>> > tools_def.txt in Conf directory?
>> >
>> > Could you paste the entire command line of building this .c file to us?
>> >
>> >
>> >
>> > Thanks
>> >
>> > Feng
>> >
>> >
>> >
>> > From: Nikolai Saoukh [mailto:n...@otdel-1.org]
>> > Sent: Tuesday, January 15, 2013 22:27
>> >
>> >
>> > To: edk2-devel@lists.sourceforge.net
>> > Subject: [edk2] tautological-constant-out-of-range-compare
>> >
>> >
>> >
>> > This particular case was discussed already
>> >
>> >
>> >
>> > /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Pool.c:188:49: error: comparison of
>> > constant 2147483647 with expression of type 'EFI_MEMORY_TYPE' is always
>> > true
>> > [-Werror,-Wtautological-constant-out-of-range-compare]
>> >
>> > if ((PoolType >= EfiMaxMemoryType && PoolType <= 0x7fffffff) ||
>> >
>> > ~~~~~~~~ ^ ~~~~~~~~~~
>> >
>> >
>> >
>> > /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Page.c:528:40: error: comparison of
>> > constant 2147483647 with expression of type 'EFI_MEMORY_TYPE' is always
>> > true
>> > [-Werror,-Wtautological-constant-out-of-range-compare]
>> >
>> > if (Type >= EfiMaxMemoryType && Type <= 0x7fffffff) {
>> >
>> > ~~~~ ^ ~~~~~~~~~~
>> >
>> >
>> >
>> > .................
>> >
>> >
>> >
>> > "i686-pc-mingw32-objcopy" --redefine-sym _memcpy=_CopyMem --redefine-sym
>> > _memset=_SetMem
>> > /tiano/edk2/Build/DuetPkgIA32/DEBUG_PECLANG/IA32/MdeModulePkg/Core/Dxe/DxeMain/OUTPUT/Mem/Pool.obj
>> >
>> > /tiano/edk2/MdeModulePkg/Core/Dxe/Mem/Page.c:1101:53: error: comparison of
>> > constant 2147483647 with expression of type 'EFI_MEMORY_TYPE' is always
>> > true
>> > [-Werror,-Wtautological-constant-out-of-range-compare]
>> >
>> > if ((MemoryType >= EfiMaxMemoryType && MemoryType <= 0x7fffffff) ||
>> >
>> > ~~~~~~~~~~ ^ ~~~~~~~~~~
>> >
>> >
>> >
>> >
>> >
>> > I guess it should be written like this
>> >
>> >
>> >
>> > if ((PoolType >= EfiMaxMemoryType && ((UINT32)PoolType <= 0x7fffffff)) ...
>> >
>> >
>> > ------------------------------------------------------------------------------
>> > Master Java SE, Java EE, Eclipse, Spring, Hibernate, JavaScript, jQuery
>> > and much more. Keep your Java skills current with LearnJavaNow -
>> > 200+ hours of step-by-step video tutorials by Java experts.
>> > SALE $49.99 this month only -- learn more at:
>> > http://p.sf.net/sfu/learnmore_122612
>> > _______________________________________________
>> > edk2-devel mailing list
>> > edk2-devel@lists.sourceforge.net
>> > https://lists.sourceforge.net/lists/listinfo/edk2-devel
>> >
>>
>> ------------------------------------------------------------------------------
>> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
>> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
>> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
>> MVPs and experts. SALE $99.99 this month only -- learn more at:
>> http://p.sf.net/sfu/learnmore_122912
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>
>> ------------------------------------------------------------------------------
>> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
>> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
>> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
>> MVPs and experts. ON SALE this month only -- learn more at:
>> http://p.sf.net/sfu/learnmore_123012_______________________________________________
>>
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
>
> ------------------------------------------------------------------------------
> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> MVPs and experts. ON SALE this month only -- learn more at:
> http://p.sf.net/sfu/learnmore_123012
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
>
> ------------------------------------------------------------------------------
> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> MVPs and experts. ON SALE this month only -- learn more at:
> http://p.sf.net/sfu/learnmore_123012_______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122412
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
