* Re: [PATCH] Fix for short-enums comparison bug [not found] <x490ee6l2e.fsf@janus.pgt.com> @ 1999-02-10 23:47 ` Jeffrey A Law [not found] ` < 4691.918719200@hurl.cygnus.com > 1999-02-28 22:53 ` Jeffrey A Law 0 siblings, 2 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-10 23:47 UTC (permalink / raw) To: Charles G Waldman; +Cc: egcs In message <x490ee6l2e.fsf@janus.pgt.com>you write: > The following message is a courtesy copy of an article > that has been posted to gnu.gcc,gnu.gcc.bug as well. Thanks. > I provide below a one-line patch which fixes the bug; however I'd like > to provide a little bit of explanation about the nature of the bug and > how I found it. My apologies if this is a bit long. No need to apologize, it is precisely the kind of analysis that makes my job of reviewing patches a whole lot easier :-) > Here's a very simple example which illustrates the bug in question: > > typedef enum {A=0, B, C, D} T; > main(){ > T x; > for (x=A; x<=D; ++x) > printf("%d ", (int)x); > putchar('\n'); > } > > If you compile this with no compiler flags set and run it will print > > 0 1 2 3 > > as expected. If you compile with -fshort-enums it will loop > indefinitely, printing all values from 0-255 over and over. I'm not a language lawyer -- we really need one to determine what the behavior of that code should be. I don't have an ANSI C standard here, nor do I have a copy of the ISO C9X standard. Can someone who does dig into them and determine what happens for an assignment out of the range of an enumerated type and what assumptions the compiler can/can not make about the values an enumerated variable can have? > Try running this with and without the -fshort-enums flag. If the > intent of -fshort-enums is to really restrict the value of an > enumerated type to lie within the min and max of that type, a warning > should be produced when we set x equal to 10. But no such warning is > produced, just bizarre results (the program prints 10<=3 if > -fshort-enums is used). Actually, whether or not a warning must be produced is a standards issue. Even if the standards don't demand it, a warning would be nice. Someone would have to grovel around the front-end though to produce the warning. Once we've left the language front end it's too late. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 4691.918719200@hurl.cygnus.com >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 4691.918719200@hurl.cygnus.com > @ 1999-02-11 6:55 ` Gavin Romig-Koch [not found] ` < 14018.60839.325601.426391@cetus.cygnus.com > 1999-02-28 22:53 ` Gavin Romig-Koch 0 siblings, 2 replies; 26+ messages in thread From: Gavin Romig-Koch @ 1999-02-11 6:55 UTC (permalink / raw) To: law, Charles G Waldman; +Cc: egcs Jeffrey A Law writes: > > Here's a very simple example which illustrates the bug in question: > > > > typedef enum {A=0, B, C, D} T; > > main(){ > > T x; > > for (x=A; x<=D; ++x) > > printf("%d ", (int)x); > > putchar('\n'); > > } > > > > If you compile this with no compiler flags set and run it will print > > > > 0 1 2 3 > > > > as expected. If you compile with -fshort-enums it will loop > > indefinitely, printing all values from 0-255 over and over. > I don't have an ANSI C standard here, nor do I have a copy of the ISO C9X > standard. Can someone who does dig into them and determine what happens for > an assignment out of the range of an enumerated type and what assumptions > the compiler can/can not make about the values an enumerated variable > can have? c89 and c9x say that enumerations have to act like some other integer type. Which integer type is implementation defined, and can vary from enumeration to enumeration, but it has to be capable of representing all the values in the enumeration. It looks like GCC is choosing unsigned char for this particular enum with -fshort-enums, which is ok standard-wise. Given this though, I don't see why this example shouldn't work the same as it does without -fshort-enums. > > Try running this with and without the -fshort-enums flag. If the > > intent of -fshort-enums is to really restrict the value of an > > enumerated type to lie within the min and max of that type, a warning > > should be produced when we set x equal to 10. But no such warning is > > produced, just bizarre results (the program prints 10<=3 if > > -fshort-enums is used). > Actually, whether or not a warning must be produced is a standards issue. Even > if the standards don't demand it, a warning would be nice. Someone would have > to grovel around the front-end though to produce the warning. Once we've > left the language front end it's too late. IMO, the intent of -fshort-enums is only to have the compiler choose a smaller integral type for the representation of an enum, and not to change the kinds of diagnostics produced. Warnings about creating enum values outside the range of the enum, and/or outside the range of the representation of the enum should be a separate option. One or both of these options may already exist in gcc, but I don't know without looking. -gavin... ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 14018.60839.325601.426391@cetus.cygnus.com >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 14018.60839.325601.426391@cetus.cygnus.com > @ 1999-02-11 7:25 ` Joern Rennecke 1999-02-28 22:53 ` Joern Rennecke 1999-02-11 8:12 ` Charles G Waldman 1999-02-11 9:33 ` Jeffrey A Law 2 siblings, 1 reply; 26+ messages in thread From: Joern Rennecke @ 1999-02-11 7:25 UTC (permalink / raw) To: Gavin Romig-Koch; +Cc: law, cgw, egcs > It looks like GCC is choosing unsigned char for this particular enum > with -fshort-enums, which is ok standard-wise. Given this though, > I don't see why this example shouldn't work the same as it does without > -fshort-enums. It's probably a biv elimination problem. Try -fno-strength-reduce. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 7:25 ` Joern Rennecke @ 1999-02-28 22:53 ` Joern Rennecke 0 siblings, 0 replies; 26+ messages in thread From: Joern Rennecke @ 1999-02-28 22:53 UTC (permalink / raw) To: Gavin Romig-Koch; +Cc: law, cgw, egcs > It looks like GCC is choosing unsigned char for this particular enum > with -fshort-enums, which is ok standard-wise. Given this though, > I don't see why this example shouldn't work the same as it does without > -fshort-enums. It's probably a biv elimination problem. Try -fno-strength-reduce. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 14018.60839.325601.426391@cetus.cygnus.com > 1999-02-11 7:25 ` Joern Rennecke @ 1999-02-11 8:12 ` Charles G Waldman 1999-02-28 22:53 ` Charles G Waldman 1999-02-11 9:33 ` Jeffrey A Law 2 siblings, 1 reply; 26+ messages in thread From: Charles G Waldman @ 1999-02-11 8:12 UTC (permalink / raw) To: Gavin Romig-Koch; +Cc: law, Charles G Waldman, egcs Gavin Romig-Koch writes: > It looks like GCC is choosing unsigned char for this particular enum > with -fshort-enums, which is ok standard-wise. Given this though, > I don't see why this example shouldn't work the same as it does without > -fshort-enums. I tried to explain this in my original posting. Please see the block of code in the function shorten_compare (in c-common.c) starting at line 2258 (in the egcs-1.1.1 release), which for your convenience I've appended below. In gcc-2.7.x the call to signed_or_unsigned_type (see ***) returned the underlying integral type, but the following change: Mon Oct 28 13:08:51 1996 Richard Kenner (kenner@vlsi1.ultra.nyu.edu) (signed_or_unsigned_type): If already right signedness, return. caused signed_or_unsigned_type to return the enumerated type itself rather than the underlying integral type. Thus maxval and minval refer to the maximum and minimum of the declared enum values rather than of the underlying integral type, and the subsequent logic optimizes away the comparison. My proposed patch fixes this. -------------------------------------------- Excerpt from shorten_compare(c_common.c): /* If comparing an integer against a constant more bits wide, maybe we can deduce a value of 1 or 0 independent of the data. Or else truncate the constant now rather than extend the variable at run time. This is only interesting if the constant is the wider arg. Also, it is not safe if the constant is unsigned and the variable arg is signed, since in this case the variable would be sign-extended and then regarded as unsigned. Our technique fails in this case because the lowest/highest possible unsigned results don't follow naturally from the lowest/highest possible values of the variable operand. For just EQ_EXPR and NE_EXPR there is another technique that could be used: see if the constant can be faithfully represented in the other operand's type, by truncating it and reextending it and see if that preserves the constant's value. */ if (!real1 && !real2 && TREE_CODE (primop1) == INTEGER_CST && TYPE_PRECISION (TREE_TYPE (primop0)) < TYPE_PRECISION (*restype_ptr)) { int min_gt, max_gt, min_lt, max_lt; tree maxval, minval; /* 1 if comparison is nominally unsigned. */ int unsignedp = TREE_UNSIGNED (*restype_ptr); tree val; [***] type = signed_or_unsigned_type (unsignedp0, TREE_TYPE (primop0)); maxval = TYPE_MAX_VALUE (type); minval = TYPE_MIN_VALUE (type); if (unsignedp && !unsignedp0) *restype_ptr = signed_type (*restype_ptr); if (TREE_TYPE (primop1) != *restype_ptr) primop1 = convert (*restype_ptr, primop1); if (type != *restype_ptr) { minval = convert (*restype_ptr, minval); maxval = convert (*restype_ptr, maxval); } if (unsignedp && unsignedp0) { min_gt = INT_CST_LT_UNSIGNED (primop1, minval); max_gt = INT_CST_LT_UNSIGNED (primop1, maxval); min_lt = INT_CST_LT_UNSIGNED (minval, primop1); max_lt = INT_CST_LT_UNSIGNED (maxval, primop1); } else { min_gt = INT_CST_LT (primop1, minval); max_gt = INT_CST_LT (primop1, maxval); min_lt = INT_CST_LT (minval, primop1); max_lt = INT_CST_LT (maxval, primop1); } val = 0; /* This used to be a switch, but Genix compiler can't handle that. */ if (code == NE_EXPR) { if (max_lt || min_gt) val = boolean_true_node; } else if (code == EQ_EXPR) { if (max_lt || min_gt) val = boolean_false_node; } else if (code == LT_EXPR) { if (max_lt) val = boolean_true_node; if (!min_lt) val = boolean_false_node; } else if (code == GT_EXPR) { if (min_gt) val = boolean_true_node; if (!max_gt) val = boolean_false_node; } else if (code == LE_EXPR) { if (!max_gt) val = boolean_true_node; if (min_gt) val = boolean_false_node; } else if (code == GE_EXPR) { if (!min_lt) val = boolean_true_node; if (max_lt) val = boolean_false_node; } ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 8:12 ` Charles G Waldman @ 1999-02-28 22:53 ` Charles G Waldman 0 siblings, 0 replies; 26+ messages in thread From: Charles G Waldman @ 1999-02-28 22:53 UTC (permalink / raw) To: Gavin Romig-Koch; +Cc: law, Charles G Waldman, egcs Gavin Romig-Koch writes: > It looks like GCC is choosing unsigned char for this particular enum > with -fshort-enums, which is ok standard-wise. Given this though, > I don't see why this example shouldn't work the same as it does without > -fshort-enums. I tried to explain this in my original posting. Please see the block of code in the function shorten_compare (in c-common.c) starting at line 2258 (in the egcs-1.1.1 release), which for your convenience I've appended below. In gcc-2.7.x the call to signed_or_unsigned_type (see ***) returned the underlying integral type, but the following change: Mon Oct 28 13:08:51 1996 Richard Kenner (kenner@vlsi1.ultra.nyu.edu) (signed_or_unsigned_type): If already right signedness, return. caused signed_or_unsigned_type to return the enumerated type itself rather than the underlying integral type. Thus maxval and minval refer to the maximum and minimum of the declared enum values rather than of the underlying integral type, and the subsequent logic optimizes away the comparison. My proposed patch fixes this. -------------------------------------------- Excerpt from shorten_compare(c_common.c): /* If comparing an integer against a constant more bits wide, maybe we can deduce a value of 1 or 0 independent of the data. Or else truncate the constant now rather than extend the variable at run time. This is only interesting if the constant is the wider arg. Also, it is not safe if the constant is unsigned and the variable arg is signed, since in this case the variable would be sign-extended and then regarded as unsigned. Our technique fails in this case because the lowest/highest possible unsigned results don't follow naturally from the lowest/highest possible values of the variable operand. For just EQ_EXPR and NE_EXPR there is another technique that could be used: see if the constant can be faithfully represented in the other operand's type, by truncating it and reextending it and see if that preserves the constant's value. */ if (!real1 && !real2 && TREE_CODE (primop1) == INTEGER_CST && TYPE_PRECISION (TREE_TYPE (primop0)) < TYPE_PRECISION (*restype_ptr)) { int min_gt, max_gt, min_lt, max_lt; tree maxval, minval; /* 1 if comparison is nominally unsigned. */ int unsignedp = TREE_UNSIGNED (*restype_ptr); tree val; [***] type = signed_or_unsigned_type (unsignedp0, TREE_TYPE (primop0)); maxval = TYPE_MAX_VALUE (type); minval = TYPE_MIN_VALUE (type); if (unsignedp && !unsignedp0) *restype_ptr = signed_type (*restype_ptr); if (TREE_TYPE (primop1) != *restype_ptr) primop1 = convert (*restype_ptr, primop1); if (type != *restype_ptr) { minval = convert (*restype_ptr, minval); maxval = convert (*restype_ptr, maxval); } if (unsignedp && unsignedp0) { min_gt = INT_CST_LT_UNSIGNED (primop1, minval); max_gt = INT_CST_LT_UNSIGNED (primop1, maxval); min_lt = INT_CST_LT_UNSIGNED (minval, primop1); max_lt = INT_CST_LT_UNSIGNED (maxval, primop1); } else { min_gt = INT_CST_LT (primop1, minval); max_gt = INT_CST_LT (primop1, maxval); min_lt = INT_CST_LT (minval, primop1); max_lt = INT_CST_LT (maxval, primop1); } val = 0; /* This used to be a switch, but Genix compiler can't handle that. */ if (code == NE_EXPR) { if (max_lt || min_gt) val = boolean_true_node; } else if (code == EQ_EXPR) { if (max_lt || min_gt) val = boolean_false_node; } else if (code == LT_EXPR) { if (max_lt) val = boolean_true_node; if (!min_lt) val = boolean_false_node; } else if (code == GT_EXPR) { if (min_gt) val = boolean_true_node; if (!max_gt) val = boolean_false_node; } else if (code == LE_EXPR) { if (!max_gt) val = boolean_true_node; if (min_gt) val = boolean_false_node; } else if (code == GE_EXPR) { if (!min_lt) val = boolean_true_node; if (max_lt) val = boolean_false_node; } ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 14018.60839.325601.426391@cetus.cygnus.com > 1999-02-11 7:25 ` Joern Rennecke 1999-02-11 8:12 ` Charles G Waldman @ 1999-02-11 9:33 ` Jeffrey A Law [not found] ` < 5858.918754318@hurl.cygnus.com > 1999-02-28 22:53 ` Jeffrey A Law 2 siblings, 2 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-11 9:33 UTC (permalink / raw) To: Gavin Romig-Koch; +Cc: Charles G Waldman, egcs In message < 14018.60839.325601.426391@cetus.cygnus.com >you write: > c89 and c9x say that enumerations have to act like some other integer type. > Which integer type is implementation defined, and can vary from enumeration > to enumeration, but it has to be capable of representing all the values > in the enumeration. Yes the underlying type must be capable of representing all the values for the enum. But does the enum restrict the actual values which can be used with the expected results? My quick read of the C++ standard made it appear as if assigning a value that was not a member of the enum resulted in undefined results. Is this possibly a case where C++ and C differ? jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 5858.918754318@hurl.cygnus.com >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 5858.918754318@hurl.cygnus.com > @ 1999-02-11 11:40 ` Gavin Romig-Koch 1999-02-28 22:53 ` Gavin Romig-Koch 1999-02-11 11:44 ` Joe Buck 1 sibling, 1 reply; 26+ messages in thread From: Gavin Romig-Koch @ 1999-02-11 11:40 UTC (permalink / raw) To: law; +Cc: Gavin Romig-Koch, Charles G Waldman, egcs Jeffrey A Law writes: > Yes the underlying type must be capable of representing all the values for > the enum. But does the enum restrict the actual values which can be used > with the expected results? The enum can restrict the actual values which can be used to the values of the choosen underlying type. The underlying type must be an existing type (the compiler can't make up a new "tiny" type for the underlying type). If you know the underlying type, you know what values can be used. > My quick read of the C++ standard made it appear as if assigning a value that > was not a member of the enum resulted in undefined results. Is this possibly a > case where C++ and C differ? Probably. There was talk in the C++ committee to restrict the usable values farther than C does, but I don't remember the outcome of that discussion. c9x kept the old C rules. -gavin... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 11:40 ` Gavin Romig-Koch @ 1999-02-28 22:53 ` Gavin Romig-Koch 0 siblings, 0 replies; 26+ messages in thread From: Gavin Romig-Koch @ 1999-02-28 22:53 UTC (permalink / raw) To: law; +Cc: Gavin Romig-Koch, Charles G Waldman, egcs Jeffrey A Law writes: > Yes the underlying type must be capable of representing all the values for > the enum. But does the enum restrict the actual values which can be used > with the expected results? The enum can restrict the actual values which can be used to the values of the choosen underlying type. The underlying type must be an existing type (the compiler can't make up a new "tiny" type for the underlying type). If you know the underlying type, you know what values can be used. > My quick read of the C++ standard made it appear as if assigning a value that > was not a member of the enum resulted in undefined results. Is this possibly a > case where C++ and C differ? Probably. There was talk in the C++ committee to restrict the usable values farther than C does, but I don't remember the outcome of that discussion. c9x kept the old C rules. -gavin... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 5858.918754318@hurl.cygnus.com > 1999-02-11 11:40 ` Gavin Romig-Koch @ 1999-02-11 11:44 ` Joe Buck [not found] ` < 199902111942.LAA16560@atrus.synopsys.com > 1999-02-28 22:53 ` Joe Buck 1 sibling, 2 replies; 26+ messages in thread From: Joe Buck @ 1999-02-11 11:44 UTC (permalink / raw) To: law; +Cc: gavin, cgw, egcs > My quick read of the C++ standard made it appear as if assigning a value that > was not a member of the enum resulted in undefined results. Wrong. The ISO C++ standard defines what values must be assignable to the enum, and it is a larger set of values than just the members. I don't have the language in front of me, but the intent is to require the ability to hold intermediate values and AND and OR of legal values. Thus code like the following is standard-conformant: enum flags { flag_none = 0x0, flag_a = 0x1, flag_b = 0x2, flag_c = 0x4, flag_d = 0x8 }; ... flags foo = (flags)(flag_a | flag+b); // OK flags bar = (flags) 12; // OK flags xyz = (flags) 23; // not OK the last one is not guaranteed to work because the compiler is allowed to use a four-bit unsigned field and 23 will not fit. > case where C++ and C differ? ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 199902111942.LAA16560@atrus.synopsys.com >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 199902111942.LAA16560@atrus.synopsys.com > @ 1999-02-11 11:55 ` Jeffrey A Law [not found] ` < 6271.918762821@hurl.cygnus.com > 1999-02-28 22:53 ` Jeffrey A Law 0 siblings, 2 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-11 11:55 UTC (permalink / raw) To: Joe Buck; +Cc: gavin, cgw, egcs In message < 199902111942.LAA16560@atrus.synopsys.com >you write: > Wrong. The ISO C++ standard defines what values must be assignable to the > enum, and it is a larger set of values than just the members. I don't > have the language in front of me, but the intent is to require the ability > to hold intermediate values and AND and OR of legal values. Thus code > like the following is standard-conformant: > > enum flags { > flag_none = 0x0, > flag_a = 0x1, > flag_b = 0x2, > flag_c = 0x4, > flag_d = 0x8 > }; > > ... > flags foo = (flags)(flag_a | flag+b); // OK > flags bar = (flags) 12; // OK > flags xyz = (flags) 23; // not OK > > the last one is not guaranteed to work because the compiler is allowed to > use a four-bit unsigned field and 23 will not fit. OK. Thanks. So if we go back to the original sample code: typedef enum {A=0, B, C, D} T; main(){ T x; for (x=A; x<=D; ++x) printf("%d ", (int)x); putchar('\n'); } So the compiler could use a 2 bit unsigned field for x since the values for enum T are 0, 1, 2, 3. Other values will not fit and would be considered invalid. Thus removing the test x <= D is technically valid for C++. Right? That (of course) isn't binding for C. jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 6271.918762821@hurl.cygnus.com >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 6271.918762821@hurl.cygnus.com > @ 1999-02-11 12:07 ` Joe Buck 1999-02-28 22:53 ` Joe Buck 1999-02-11 14:16 ` Tim Hollebeek 1 sibling, 1 reply; 26+ messages in thread From: Joe Buck @ 1999-02-11 12:07 UTC (permalink / raw) To: law; +Cc: jbuck, gavin, cgw, egcs > OK. Thanks. So if we go back to the original sample code: > > > typedef enum {A=0, B, C, D} T; > main(){ > T x; > for (x=A; x<=D; ++x) > printf("%d ", (int)x); > putchar('\n'); > } > > So the compiler could use a 2 bit unsigned field for x since the values for > enum T are 0, 1, 2, 3. Other values will not fit and would be considered > invalid. Thus removing the test x <= D is technically valid for C++. Right? Hmm. I'm not sure. It would seem so. It does seem that the code is risky, at least, and might merit a warning (I definitely think that g++ must not remove this test without issuing a warning). Note also that the operators =, <=, and ++ can be overloaded for an enum, which would change things yet again (I could define ++ to wrap around if I wanted to). > That (of course) isn't binding for C. Yes, in C enums are basically ints of some form. However, even in C there might be similar cases, if one of the enum values is the maximum value of the integral type that is used. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 12:07 ` Joe Buck @ 1999-02-28 22:53 ` Joe Buck 0 siblings, 0 replies; 26+ messages in thread From: Joe Buck @ 1999-02-28 22:53 UTC (permalink / raw) To: law; +Cc: jbuck, gavin, cgw, egcs > OK. Thanks. So if we go back to the original sample code: > > > typedef enum {A=0, B, C, D} T; > main(){ > T x; > for (x=A; x<=D; ++x) > printf("%d ", (int)x); > putchar('\n'); > } > > So the compiler could use a 2 bit unsigned field for x since the values for > enum T are 0, 1, 2, 3. Other values will not fit and would be considered > invalid. Thus removing the test x <= D is technically valid for C++. Right? Hmm. I'm not sure. It would seem so. It does seem that the code is risky, at least, and might merit a warning (I definitely think that g++ must not remove this test without issuing a warning). Note also that the operators =, <=, and ++ can be overloaded for an enum, which would change things yet again (I could define ++ to wrap around if I wanted to). > That (of course) isn't binding for C. Yes, in C enums are basically ints of some form. However, even in C there might be similar cases, if one of the enum values is the maximum value of the integral type that is used. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 6271.918762821@hurl.cygnus.com > 1999-02-11 12:07 ` Joe Buck @ 1999-02-11 14:16 ` Tim Hollebeek [not found] ` < 199902112216.RAA27319@wagner.Princeton.EDU > 1999-02-28 22:53 ` Tim Hollebeek 1 sibling, 2 replies; 26+ messages in thread From: Tim Hollebeek @ 1999-02-11 14:16 UTC (permalink / raw) To: law; +Cc: jbuck, gavin, cgw, egcs Jeffrey A Law writes ... > > typedef enum {A=0, B, C, D} T; > main(){ > T x; > for (x=A; x<=D; ++x) > printf("%d ", (int)x); > putchar('\n'); > } > > So the compiler could use a 2 bit unsigned field for x since the values for > enum T are 0, 1, 2, 3. Other values will not fit and would be considered > invalid. Thus removing the test x <= D is technically valid for C++. Right? I seriously doubt it, since arguments to integer operators undergo integral promotion. The rules are different for C++, but I believe the answer is no, since none of the standard integral types is allowed to have only 2 bits. --------------------------------------------------------------------------- Tim Hollebeek | "Everything above is a true email: tim@wfn-shop.princeton.edu | statement, for sufficiently URL: http://wfn-shop.princeton.edu/~tim | false values of true." ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 199902112216.RAA27319@wagner.Princeton.EDU >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 199902112216.RAA27319@wagner.Princeton.EDU > @ 1999-02-11 14:19 ` Jeffrey A Law [not found] ` < 6754.918771465@hurl.cygnus.com > 1999-02-28 22:53 ` Jeffrey A Law 0 siblings, 2 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-11 14:19 UTC (permalink / raw) To: Tim Hollebeek; +Cc: jbuck, gavin, cgw, egcs In message < 199902112216.RAA27319@wagner.Princeton.EDU >you write: > I seriously doubt it, since arguments to integer operators undergo > integral promotion. The rules are different for C++, but I believe > the answer is no, since none of the standard integral types is allowed > to have only 2 bits. I'm not looking for "I doubt/I believe". I need folks to actually check the relavent standard and say "The standard ..." :-) jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: < 6754.918771465@hurl.cygnus.com >]
* Re: [PATCH] Fix for short-enums comparison bug [not found] ` < 6754.918771465@hurl.cygnus.com > @ 1999-02-11 17:03 ` Joe Buck 1999-02-28 22:53 ` Joe Buck 0 siblings, 1 reply; 26+ messages in thread From: Joe Buck @ 1999-02-11 17:03 UTC (permalink / raw) To: law; +Cc: tim, jbuck, gavin, cgw, egcs > In message < 199902112216.RAA27319@wagner.Princeton.EDU >you write: > > I seriously doubt it, since arguments to integer operators undergo > > integral promotion. The rules are different for C++, but I believe > > the answer is no, since none of the standard integral types is allowed > > to have only 2 bits. > I'm not looking for "I doubt/I believe". I need folks to actually check the > relavent standard and say "The standard ..." :-) The relevant part of the C++ standard has the label "dcl.enum": look for .../dcl.html#dcl.enum in your favorite copy of the standard (I assume you have an up-to-date version inside Cygnus). I'm currently looking at the Dec. 96 draft, so I don't know if there were changes made later (you can check your local copy). Two things are described: the legal integral values that can be assigned to the enum, and the "underlying type", which is implementation-dependent but can't be larger than int unless there are enum values that don't fit in int or unsigned int. Stroustrup "The C++ Programming Language, Third Edition"'s section 4.8 may be a clearer explanation of what is intended (though of course it is non-normative). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 17:03 ` Joe Buck @ 1999-02-28 22:53 ` Joe Buck 0 siblings, 0 replies; 26+ messages in thread From: Joe Buck @ 1999-02-28 22:53 UTC (permalink / raw) To: law; +Cc: tim, jbuck, gavin, cgw, egcs > In message < 199902112216.RAA27319@wagner.Princeton.EDU >you write: > > I seriously doubt it, since arguments to integer operators undergo > > integral promotion. The rules are different for C++, but I believe > > the answer is no, since none of the standard integral types is allowed > > to have only 2 bits. > I'm not looking for "I doubt/I believe". I need folks to actually check the > relavent standard and say "The standard ..." :-) The relevant part of the C++ standard has the label "dcl.enum": look for .../dcl.html#dcl.enum in your favorite copy of the standard (I assume you have an up-to-date version inside Cygnus). I'm currently looking at the Dec. 96 draft, so I don't know if there were changes made later (you can check your local copy). Two things are described: the legal integral values that can be assigned to the enum, and the "underlying type", which is implementation-dependent but can't be larger than int unless there are enum values that don't fit in int or unsigned int. Stroustrup "The C++ Programming Language, Third Edition"'s section 4.8 may be a clearer explanation of what is intended (though of course it is non-normative). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 14:19 ` Jeffrey A Law [not found] ` < 6754.918771465@hurl.cygnus.com > @ 1999-02-28 22:53 ` Jeffrey A Law 1 sibling, 0 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-28 22:53 UTC (permalink / raw) To: Tim Hollebeek; +Cc: jbuck, gavin, cgw, egcs In message < 199902112216.RAA27319@wagner.Princeton.EDU >you write: > I seriously doubt it, since arguments to integer operators undergo > integral promotion. The rules are different for C++, but I believe > the answer is no, since none of the standard integral types is allowed > to have only 2 bits. I'm not looking for "I doubt/I believe". I need folks to actually check the relavent standard and say "The standard ..." :-) jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 14:16 ` Tim Hollebeek [not found] ` < 199902112216.RAA27319@wagner.Princeton.EDU > @ 1999-02-28 22:53 ` Tim Hollebeek 1 sibling, 0 replies; 26+ messages in thread From: Tim Hollebeek @ 1999-02-28 22:53 UTC (permalink / raw) To: law; +Cc: jbuck, gavin, cgw, egcs Jeffrey A Law writes ... > > typedef enum {A=0, B, C, D} T; > main(){ > T x; > for (x=A; x<=D; ++x) > printf("%d ", (int)x); > putchar('\n'); > } > > So the compiler could use a 2 bit unsigned field for x since the values for > enum T are 0, 1, 2, 3. Other values will not fit and would be considered > invalid. Thus removing the test x <= D is technically valid for C++. Right? I seriously doubt it, since arguments to integer operators undergo integral promotion. The rules are different for C++, but I believe the answer is no, since none of the standard integral types is allowed to have only 2 bits. --------------------------------------------------------------------------- Tim Hollebeek | "Everything above is a true email: tim@wfn-shop.princeton.edu | statement, for sufficiently URL: http://wfn-shop.princeton.edu/~tim | false values of true." ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 11:55 ` Jeffrey A Law [not found] ` < 6271.918762821@hurl.cygnus.com > @ 1999-02-28 22:53 ` Jeffrey A Law 1 sibling, 0 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-28 22:53 UTC (permalink / raw) To: Joe Buck; +Cc: gavin, cgw, egcs In message < 199902111942.LAA16560@atrus.synopsys.com >you write: > Wrong. The ISO C++ standard defines what values must be assignable to the > enum, and it is a larger set of values than just the members. I don't > have the language in front of me, but the intent is to require the ability > to hold intermediate values and AND and OR of legal values. Thus code > like the following is standard-conformant: > > enum flags { > flag_none = 0x0, > flag_a = 0x1, > flag_b = 0x2, > flag_c = 0x4, > flag_d = 0x8 > }; > > ... > flags foo = (flags)(flag_a | flag+b); // OK > flags bar = (flags) 12; // OK > flags xyz = (flags) 23; // not OK > > the last one is not guaranteed to work because the compiler is allowed to > use a four-bit unsigned field and 23 will not fit. OK. Thanks. So if we go back to the original sample code: typedef enum {A=0, B, C, D} T; main(){ T x; for (x=A; x<=D; ++x) printf("%d ", (int)x); putchar('\n'); } So the compiler could use a 2 bit unsigned field for x since the values for enum T are 0, 1, 2, 3. Other values will not fit and would be considered invalid. Thus removing the test x <= D is technically valid for C++. Right? That (of course) isn't binding for C. jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 11:44 ` Joe Buck [not found] ` < 199902111942.LAA16560@atrus.synopsys.com > @ 1999-02-28 22:53 ` Joe Buck 1 sibling, 0 replies; 26+ messages in thread From: Joe Buck @ 1999-02-28 22:53 UTC (permalink / raw) To: law; +Cc: gavin, cgw, egcs > My quick read of the C++ standard made it appear as if assigning a value that > was not a member of the enum resulted in undefined results. Wrong. The ISO C++ standard defines what values must be assignable to the enum, and it is a larger set of values than just the members. I don't have the language in front of me, but the intent is to require the ability to hold intermediate values and AND and OR of legal values. Thus code like the following is standard-conformant: enum flags { flag_none = 0x0, flag_a = 0x1, flag_b = 0x2, flag_c = 0x4, flag_d = 0x8 }; ... flags foo = (flags)(flag_a | flag+b); // OK flags bar = (flags) 12; // OK flags xyz = (flags) 23; // not OK the last one is not guaranteed to work because the compiler is allowed to use a four-bit unsigned field and 23 will not fit. > case where C++ and C differ? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 9:33 ` Jeffrey A Law [not found] ` < 5858.918754318@hurl.cygnus.com > @ 1999-02-28 22:53 ` Jeffrey A Law 1 sibling, 0 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-28 22:53 UTC (permalink / raw) To: Gavin Romig-Koch; +Cc: Charles G Waldman, egcs In message < 14018.60839.325601.426391@cetus.cygnus.com >you write: > c89 and c9x say that enumerations have to act like some other integer type. > Which integer type is implementation defined, and can vary from enumeration > to enumeration, but it has to be capable of representing all the values > in the enumeration. Yes the underlying type must be capable of representing all the values for the enum. But does the enum restrict the actual values which can be used with the expected results? My quick read of the C++ standard made it appear as if assigning a value that was not a member of the enum resulted in undefined results. Is this possibly a case where C++ and C differ? jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 6:55 ` Gavin Romig-Koch [not found] ` < 14018.60839.325601.426391@cetus.cygnus.com > @ 1999-02-28 22:53 ` Gavin Romig-Koch 1 sibling, 0 replies; 26+ messages in thread From: Gavin Romig-Koch @ 1999-02-28 22:53 UTC (permalink / raw) To: law, Charles G Waldman; +Cc: egcs Jeffrey A Law writes: > > Here's a very simple example which illustrates the bug in question: > > > > typedef enum {A=0, B, C, D} T; > > main(){ > > T x; > > for (x=A; x<=D; ++x) > > printf("%d ", (int)x); > > putchar('\n'); > > } > > > > If you compile this with no compiler flags set and run it will print > > > > 0 1 2 3 > > > > as expected. If you compile with -fshort-enums it will loop > > indefinitely, printing all values from 0-255 over and over. > I don't have an ANSI C standard here, nor do I have a copy of the ISO C9X > standard. Can someone who does dig into them and determine what happens for > an assignment out of the range of an enumerated type and what assumptions > the compiler can/can not make about the values an enumerated variable > can have? c89 and c9x say that enumerations have to act like some other integer type. Which integer type is implementation defined, and can vary from enumeration to enumeration, but it has to be capable of representing all the values in the enumeration. It looks like GCC is choosing unsigned char for this particular enum with -fshort-enums, which is ok standard-wise. Given this though, I don't see why this example shouldn't work the same as it does without -fshort-enums. > > Try running this with and without the -fshort-enums flag. If the > > intent of -fshort-enums is to really restrict the value of an > > enumerated type to lie within the min and max of that type, a warning > > should be produced when we set x equal to 10. But no such warning is > > produced, just bizarre results (the program prints 10<=3 if > > -fshort-enums is used). > Actually, whether or not a warning must be produced is a standards issue. Even > if the standards don't demand it, a warning would be nice. Someone would have > to grovel around the front-end though to produce the warning. Once we've > left the language front end it's too late. IMO, the intent of -fshort-enums is only to have the compiler choose a smaller integral type for the representation of an enum, and not to change the kinds of diagnostics produced. Warnings about creating enum values outside the range of the enum, and/or outside the range of the representation of the enum should be a separate option. One or both of these options may already exist in gcc, but I don't know without looking. -gavin... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-10 23:47 ` [PATCH] Fix for short-enums comparison bug Jeffrey A Law [not found] ` < 4691.918719200@hurl.cygnus.com > @ 1999-02-28 22:53 ` Jeffrey A Law 1 sibling, 0 replies; 26+ messages in thread From: Jeffrey A Law @ 1999-02-28 22:53 UTC (permalink / raw) To: Charles G Waldman; +Cc: egcs In message <x490ee6l2e.fsf@janus.pgt.com>you write: > The following message is a courtesy copy of an article > that has been posted to gnu.gcc,gnu.gcc.bug as well. Thanks. > I provide below a one-line patch which fixes the bug; however I'd like > to provide a little bit of explanation about the nature of the bug and > how I found it. My apologies if this is a bit long. No need to apologize, it is precisely the kind of analysis that makes my job of reviewing patches a whole lot easier :-) > Here's a very simple example which illustrates the bug in question: > > typedef enum {A=0, B, C, D} T; > main(){ > T x; > for (x=A; x<=D; ++x) > printf("%d ", (int)x); > putchar('\n'); > } > > If you compile this with no compiler flags set and run it will print > > 0 1 2 3 > > as expected. If you compile with -fshort-enums it will loop > indefinitely, printing all values from 0-255 over and over. I'm not a language lawyer -- we really need one to determine what the behavior of that code should be. I don't have an ANSI C standard here, nor do I have a copy of the ISO C9X standard. Can someone who does dig into them and determine what happens for an assignment out of the range of an enumerated type and what assumptions the compiler can/can not make about the values an enumerated variable can have? > Try running this with and without the -fshort-enums flag. If the > intent of -fshort-enums is to really restrict the value of an > enumerated type to lie within the min and max of that type, a warning > should be produced when we set x equal to 10. But no such warning is > produced, just bizarre results (the program prints 10<=3 if > -fshort-enums is used). Actually, whether or not a warning must be produced is a standards issue. Even if the standards don't demand it, a warning would be nice. Someone would have to grovel around the front-end though to produce the warning. Once we've left the language front end it's too late. Jeff ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug @ 1999-02-11 12:12 Mike Stump 1999-02-28 22:53 ` Mike Stump 0 siblings, 1 reply; 26+ messages in thread From: Mike Stump @ 1999-02-11 12:12 UTC (permalink / raw) To: egcs, law > To: Charles G Waldman <cgw@alum.mit.edu> > Date: Thu, 11 Feb 1999 00:46:40 -0700 > From: Jeffrey A Law <law@hurl.cygnus.com> > I'm not a language lawyer -- we really need one to determine what > the behavior of that code should be. I disagree. The gcc documentation has enough. It says that a integral number of bytes are used to represent the enum. And only if it were a bit field, would an integral number of bits be used. If a byte is used 4+1 = 5, 5 is > 4, so the loop must stop. If we used bitfields, 4+1=0 (when we use 2 bits for the enum). But the documentation clearly states and the users expectation is we use a byte. Now, if we want to change to document, we _can_ entertain this, and we can allocate just the number of bits required. We are allowed to do this optimization in C++ certainly (if the user can't otherwise tell). If we do this, then the behavior of the program _is_ correct. > Date: Thu, 11 Feb 1999 10:31:58 -0700 > From: Jeffrey A Law <law@hurl.cygnus.com> > Yes the underlying type must be capable of representing all the > values for the enum. But does the enum restrict the actual values > which can be used with the expected results? It does (for C++), but we don't need to make use of that much latitude in the standard. It we can reasonably maye _more_ code work, even though strickly speaking it isn't portable, we should. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Fix for short-enums comparison bug 1999-02-11 12:12 Mike Stump @ 1999-02-28 22:53 ` Mike Stump 0 siblings, 0 replies; 26+ messages in thread From: Mike Stump @ 1999-02-28 22:53 UTC (permalink / raw) To: egcs, law > To: Charles G Waldman <cgw@alum.mit.edu> > Date: Thu, 11 Feb 1999 00:46:40 -0700 > From: Jeffrey A Law <law@hurl.cygnus.com> > I'm not a language lawyer -- we really need one to determine what > the behavior of that code should be. I disagree. The gcc documentation has enough. It says that a integral number of bytes are used to represent the enum. And only if it were a bit field, would an integral number of bits be used. If a byte is used 4+1 = 5, 5 is > 4, so the loop must stop. If we used bitfields, 4+1=0 (when we use 2 bits for the enum). But the documentation clearly states and the users expectation is we use a byte. Now, if we want to change to document, we _can_ entertain this, and we can allocate just the number of bits required. We are allowed to do this optimization in C++ certainly (if the user can't otherwise tell). If we do this, then the behavior of the program _is_ correct. > Date: Thu, 11 Feb 1999 10:31:58 -0700 > From: Jeffrey A Law <law@hurl.cygnus.com> > Yes the underlying type must be capable of representing all the > values for the enum. But does the enum restrict the actual values > which can be used with the expected results? It does (for C++), but we don't need to make use of that much latitude in the standard. It we can reasonably maye _more_ code work, even though strickly speaking it isn't portable, we should. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~1999-02-28 22:53 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <x490ee6l2e.fsf@janus.pgt.com> 1999-02-10 23:47 ` [PATCH] Fix for short-enums comparison bug Jeffrey A Law [not found] ` < 4691.918719200@hurl.cygnus.com > 1999-02-11 6:55 ` Gavin Romig-Koch [not found] ` < 14018.60839.325601.426391@cetus.cygnus.com > 1999-02-11 7:25 ` Joern Rennecke 1999-02-28 22:53 ` Joern Rennecke 1999-02-11 8:12 ` Charles G Waldman 1999-02-28 22:53 ` Charles G Waldman 1999-02-11 9:33 ` Jeffrey A Law [not found] ` < 5858.918754318@hurl.cygnus.com > 1999-02-11 11:40 ` Gavin Romig-Koch 1999-02-28 22:53 ` Gavin Romig-Koch 1999-02-11 11:44 ` Joe Buck [not found] ` < 199902111942.LAA16560@atrus.synopsys.com > 1999-02-11 11:55 ` Jeffrey A Law [not found] ` < 6271.918762821@hurl.cygnus.com > 1999-02-11 12:07 ` Joe Buck 1999-02-28 22:53 ` Joe Buck 1999-02-11 14:16 ` Tim Hollebeek [not found] ` < 199902112216.RAA27319@wagner.Princeton.EDU > 1999-02-11 14:19 ` Jeffrey A Law [not found] ` < 6754.918771465@hurl.cygnus.com > 1999-02-11 17:03 ` Joe Buck 1999-02-28 22:53 ` Joe Buck 1999-02-28 22:53 ` Jeffrey A Law 1999-02-28 22:53 ` Tim Hollebeek 1999-02-28 22:53 ` Jeffrey A Law 1999-02-28 22:53 ` Joe Buck 1999-02-28 22:53 ` Jeffrey A Law 1999-02-28 22:53 ` Gavin Romig-Koch 1999-02-28 22:53 ` Jeffrey A Law 1999-02-11 12:12 Mike Stump 1999-02-28 22:53 ` Mike Stump
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).