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. 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.
--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 <http://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
