| Message ID | 348844c6-e774-ca4b-dd8b-693eeb05aa50@ramsayjones.plus.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | None | expand |
On 19/11/2018 20:54, Ramsay Jones wrote: >>From 8764999aa78415e217fd3106a8c8518a5b40c20c Mon Sep 17 00:00:00 2001 > From: Ramsay Jones <ramsay@ramsayjones.plus.com> > Date: Sun, 18 Nov 2018 23:52:23 +0000 > Subject: [PATCH 9/9] constant: add -Wconstant-size warning Heh, obviously, the lines above should have been removed before I hit send. sigh ... :-P ATB, Ramsay Jones > > Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> > --- > cgcc | 2 +- > expression.c | 2 +- > lib.c | 2 ++ > lib.h | 1 + > sparse.1 | 10 ++++++++++ > validation/constant-size-32.c | 15 +++++++++++++++ > validation/constant-size-64.c | 15 +++++++++++++++ > 7 files changed, 45 insertions(+), 2 deletions(-) > create mode 100644 validation/constant-size-32.c > create mode 100644 validation/constant-size-64.c > > diff --git a/cgcc b/cgcc > index 7611dc9..8ad766d 100755 > --- a/cgcc > +++ b/cgcc > @@ -101,7 +101,7 @@ exit 0; > > sub check_only_option { > my ($arg) = @_; > - return 1 if $arg =~ /^-W(no-?)?(address-space|bitwise|cast-to-as|cast-truncate|context|decl|default-bitfield-sign|designated-init|do-while|enum-mismatch|init-cstring|memcpy-max-count|non-pointer-null|old-initializer|one-bit-signed-bitfield|override-init-all|paren-string|ptr-subtraction-blows|return-void|sizeof-bool|sparse-all|sparse-error|transparent-union|typesign|undef|unknown-attribute)$/; > + return 1 if $arg =~ /^-W(no-?)?(address-space|bitwise|cast-to-as|cast-truncate|constant-size|context|decl|default-bitfield-sign|designated-init|do-while|enum-mismatch|init-cstring|memcpy-max-count|non-pointer-null|old-initializer|one-bit-signed-bitfield|override-init-all|paren-string|ptr-subtraction-blows|return-void|sizeof-bool|sparse-all|sparse-error|transparent-union|typesign|undef|unknown-attribute)$/; > return 1 if $arg =~ /^-v(no-?)?(entry|dead)$/; > return 1 if $arg =~ /^-f(dump-ir|memcpy-max-count|diagnostic-prefix)(=\S*)?$/; > return 1 if $arg =~ /^-f(mem2reg|optim)(-enable|-disable|=last)?$/; > diff --git a/expression.c b/expression.c > index 6f4300b..d6ad74e 100644 > --- a/expression.c > +++ b/expression.c > @@ -324,7 +324,7 @@ static void get_number_value(struct expression *expr, struct token *token) > show_token(token)); > want_unsigned = 1; > got_it: > - if (do_warn) > + if (do_warn && Wconstant_size) > warning(expr->pos, "constant %s is so big it is%s%s%s", > show_token(token), > want_unsigned ? " unsigned":"", > diff --git a/lib.c b/lib.c > index 08dc299..a8ebb7d 100644 > --- a/lib.c > +++ b/lib.c > @@ -253,6 +253,7 @@ int Wbitwise = 1; > int Wcast_from_as = 0; > int Wcast_to_as = 0; > int Wcast_truncate = 1; > +int Wconstant_size = 0; > int Wconstexpr_not_const = 0; > int Wcontext = 1; > int Wdecl = 1; > @@ -689,6 +690,7 @@ static const struct flag warnings[] = { > { "cast-from-as", &Wcast_from_as }, > { "cast-to-as", &Wcast_to_as }, > { "cast-truncate", &Wcast_truncate }, > + { "constant-size", &Wconstant_size }, > { "constexpr-not-const", &Wconstexpr_not_const}, > { "context", &Wcontext }, > { "decl", &Wdecl }, > diff --git a/lib.h b/lib.h > index ae0e981..507eb69 100644 > --- a/lib.h > +++ b/lib.h > @@ -141,6 +141,7 @@ extern int Wbitwise; > extern int Wcast_from_as; > extern int Wcast_to_as; > extern int Wcast_truncate; > +extern int Wconstant_size; > extern int Wconstexpr_not_const; > extern int Wcontext; > extern int Wdecl; > diff --git a/sparse.1 b/sparse.1 > index 3e13523..26299b3 100644 > --- a/sparse.1 > +++ b/sparse.1 > @@ -101,6 +101,16 @@ Sparse issues these warnings by default. To turn them off, use > \fB\-Wno\-cast\-truncate\fR. > . > .TP > +.B \-Wconstant-size > +Warn if an integer constant is larger than the maximum representable value > +of the type indicated by the type suffix (if any). For example, on a 64-bit > +system, the constant 0xfffff00000000000U is larger than can be represented > +by an int and so requires an unsigned long type suffix. (So, in this case, > +the warning could be suppressed by using the UL type suffix). > + > +Sparse does not issue these warnings by default. > +. > +.TP > .B \-Wconstexpr-not-const > Warn if a non-constant expression is encountered when really expecting a > constant expression instead. > diff --git a/validation/constant-size-32.c b/validation/constant-size-32.c > new file mode 100644 > index 0000000..164734e > --- /dev/null > +++ b/validation/constant-size-32.c > @@ -0,0 +1,15 @@ > +#define BIGU 0xfffff00000000000U > +#define BIGULL 0xfffff00000000000ULL > + > +static unsigned long long a = BIGU; > +static unsigned long long b = BIGULL; > + > +/* > + * check-name: constant-size > + * check-command: sparse -m32 -Wconstant-size $file > + * > + * check-error-start > +constant-size-32.c:4:31: warning: constant 0xfffff00000000000U is so big it is unsigned long long > + * check-error-end > + */ > + > diff --git a/validation/constant-size-64.c b/validation/constant-size-64.c > new file mode 100644 > index 0000000..471f6bc > --- /dev/null > +++ b/validation/constant-size-64.c > @@ -0,0 +1,15 @@ > +#define BIGU 0xfffff00000000000U > +#define BIGUL 0xfffff00000000000UL > + > +static unsigned long a = BIGU; > +static unsigned long b = BIGUL; > + > +/* > + * check-name: constant-size > + * check-command: sparse -m64 -Wconstant-size $file > + * > + * check-error-start > +constant-size-64.c:4:26: warning: constant 0xfffff00000000000U is so big it is unsigned long > + * check-error-end > + */ > + >
On Mon, Nov 19, 2018 at 08:54:38PM +0000, Ramsay Jones wrote: > From 8764999aa78415e217fd3106a8c8518a5b40c20c Mon Sep 17 00:00:00 2001 > From: Ramsay Jones <ramsay@ramsayjones.plus.com> > Date: Sun, 18 Nov 2018 23:52:23 +0000 > Subject: [PATCH 9/9] constant: add -Wconstant-size warning Since the problem is not really a problem of size, nor a problem of type, but a mismatch between the real type of the constant and the type indicated by its suffix (or the lack of a suffix), I propose to call this warning -Wconstant-suffix. > diff --git a/sparse.1 b/sparse.1 > index 3e13523..26299b3 100644 > --- a/sparse.1 > +++ b/sparse.1 > @@ -101,6 +101,16 @@ Sparse issues these warnings by default. To turn them off, use > \fB\-Wno\-cast\-truncate\fR. > . > .TP > +.B \-Wconstant-size > +Warn if an integer constant is larger than the maximum representable value > +of the type indicated by the type suffix (if any). For example, on a 64-bit > +system, the constant 0xfffff00000000000U is larger than can be represented > +by an int and so requires an unsigned long type suffix. (So, in this case, > +the warning could be suppressed by using the UL type suffix). I don't 100% agree with the use of 'requires'. The type of the constant is determinated by its value, no suffix is required (but the lack of an 'L' in the suffix may make believe that the type is 'unsigned int' or may force you to count the zeroes, ...). I propose to reword the example by something like: +Warn if an integer constant is larger than the maximum representable value +of the type indicated by its type suffix (if any). For example, on a +system where ints are 32-bit and longs 64-bit, the constant \fB0x100000000U\fR +is larger than can be represented by an \fBunsigned int\fR but fits in an +\fBunsigned long\fR. So its type is \fBunsigned long\fR but this is not +indicated by its suffix. In this case, the warning could be suppressed by +using the suffix \fBUL\fR: \fB0x100000000UL\fR. Another solution would be to simply remove the example. What do you think about it? Best regards, -- Luc
On 21/11/2018 22:53, Luc Van Oostenryck wrote: > On Mon, Nov 19, 2018 at 08:54:38PM +0000, Ramsay Jones wrote: >> From 8764999aa78415e217fd3106a8c8518a5b40c20c Mon Sep 17 00:00:00 2001 >> From: Ramsay Jones <ramsay@ramsayjones.plus.com> >> Date: Sun, 18 Nov 2018 23:52:23 +0000 >> Subject: [PATCH 9/9] constant: add -Wconstant-size warning > > Since the problem is not really a problem of size, nor a problem of > type, but a mismatch between the real type of the constant and the > type indicated by its suffix (or the lack of a suffix), I propose > to call this warning -Wconstant-suffix. Sounds good to me. (I am bad at naming things). ;-) >> diff --git a/sparse.1 b/sparse.1 >> index 3e13523..26299b3 100644 >> --- a/sparse.1 >> +++ b/sparse.1 >> @@ -101,6 +101,16 @@ Sparse issues these warnings by default. To turn them off, use >> \fB\-Wno\-cast\-truncate\fR. >> . >> .TP >> +.B \-Wconstant-size >> +Warn if an integer constant is larger than the maximum representable value >> +of the type indicated by the type suffix (if any). For example, on a 64-bit >> +system, the constant 0xfffff00000000000U is larger than can be represented >> +by an int and so requires an unsigned long type suffix. (So, in this case, >> +the warning could be suppressed by using the UL type suffix). > > I don't 100% agree with the use of 'requires'. The type of the constant is > determinated by its value, no suffix is required (but the lack of an 'L' > in the suffix may make believe that the type is 'unsigned int' or may > force you to count the zeroes, ...). Hmm, the standard is a bit ambiguous in its wording. For example, (C99) in 6.4.4.1-2, it states "An integer constant begins with a digit, but has no period or exponent part. It may have a prefix that specifies its base and a suffix that _specifies its type_." Whereas, later in 6.4.4.1-5, it states "The type of an integer constant is the first of the corresponding list in which its _value can be represented_." (followed by a table of types). So, this warning is about the discrepancy in the two 'type indicators' (the type suffix and the actual value of the constant). BTW, my use of 'requires' above was more about avoiding the warning ... ;-) > I propose to reword the example by something like: > +Warn if an integer constant is larger than the maximum representable value > +of the type indicated by its type suffix (if any). For example, on a > +system where ints are 32-bit and longs 64-bit, the constant \fB0x100000000U\fR > +is larger than can be represented by an \fBunsigned int\fR but fits in an > +\fBunsigned long\fR. So its type is \fBunsigned long\fR but this is not > +indicated by its suffix. In this case, the warning could be suppressed by > +using the suffix \fBUL\fR: \fB0x100000000UL\fR. OK. > > Another solution would be to simply remove the example. > I could go with that too, but how would you describe the warning without an example! ;-) ATB, Ramsay Jones
On Thu, Nov 22, 2018 at 12:13:27AM +0000, Ramsay Jones wrote: > > > On 21/11/2018 22:53, Luc Van Oostenryck wrote: > > On Mon, Nov 19, 2018 at 08:54:38PM +0000, Ramsay Jones wrote: > >> From 8764999aa78415e217fd3106a8c8518a5b40c20c Mon Sep 17 00:00:00 2001 > >> From: Ramsay Jones <ramsay@ramsayjones.plus.com> > >> Date: Sun, 18 Nov 2018 23:52:23 +0000 > >> Subject: [PATCH 9/9] constant: add -Wconstant-size warning > > > > Since the problem is not really a problem of size, nor a problem of > > type, but a mismatch between the real type of the constant and the > > type indicated by its suffix (or the lack of a suffix), I propose > > to call this warning -Wconstant-suffix. > > Sounds good to me. (I am bad at naming things). ;-) I think it's often surprinsingly hard and in this case far from obvious. > >> diff --git a/sparse.1 b/sparse.1 > >> index 3e13523..26299b3 100644 > >> --- a/sparse.1 > >> +++ b/sparse.1 > >> @@ -101,6 +101,16 @@ Sparse issues these warnings by default. To turn them off, use > >> \fB\-Wno\-cast\-truncate\fR. > >> . > >> .TP > >> +.B \-Wconstant-size > >> +Warn if an integer constant is larger than the maximum representable value > >> +of the type indicated by the type suffix (if any). For example, on a 64-bit > >> +system, the constant 0xfffff00000000000U is larger than can be represented > >> +by an int and so requires an unsigned long type suffix. (So, in this case, > >> +the warning could be suppressed by using the UL type suffix). > > > > I don't 100% agree with the use of 'requires'. The type of the constant is > > determinated by its value, no suffix is required (but the lack of an 'L' > > in the suffix may make believe that the type is 'unsigned int' or may > > force you to count the zeroes, ...). > > Hmm, the standard is a bit ambiguous in its wording. For example, (C99) > in 6.4.4.1-2, it states "An integer constant begins with a digit, but > has no period or exponent part. It may have a prefix that specifies > its base and a suffix that _specifies its type_." Whereas, later in > 6.4.4.1-5, it states "The type of an integer constant is the first of > the corresponding list in which its _value can be represented_." > (followed by a table of types). > > So, this warning is about the discrepancy in the two 'type indicators' > (the type suffix and the actual value of the constant). BTW, my use of > 'requires' above was more about avoiding the warning ... ;-) OK. Yes, it's a bit ambiguous. I rarely find the standard very clear anyway :). > > I propose to reword the example by something like: > > +Warn if an integer constant is larger than the maximum representable value > > +of the type indicated by its type suffix (if any). For example, on a > > +system where ints are 32-bit and longs 64-bit, the constant \fB0x100000000U\fR > > +is larger than can be represented by an \fBunsigned int\fR but fits in an > > +\fBunsigned long\fR. So its type is \fBunsigned long\fR but this is not > > +indicated by its suffix. In this case, the warning could be suppressed by > > +using the suffix \fBUL\fR: \fB0x100000000UL\fR. > > OK. > > > > > Another solution would be to simply remove the example. > > > > I could go with that too, but how would you describe the warning > without an example! ;-) Yes, it's better with the example. Thanks, -- Luc
diff --git a/cgcc b/cgcc index 7611dc9..8ad766d 100755 --- a/cgcc +++ b/cgcc @@ -101,7 +101,7 @@ exit 0; sub check_only_option { my ($arg) = @_; - return 1 if $arg =~ /^-W(no-?)?(address-space|bitwise|cast-to-as|cast-truncate|context|decl|default-bitfield-sign|designated-init|do-while|enum-mismatch|init-cstring|memcpy-max-count|non-pointer-null|old-initializer|one-bit-signed-bitfield|override-init-all|paren-string|ptr-subtraction-blows|return-void|sizeof-bool|sparse-all|sparse-error|transparent-union|typesign|undef|unknown-attribute)$/; + return 1 if $arg =~ /^-W(no-?)?(address-space|bitwise|cast-to-as|cast-truncate|constant-size|context|decl|default-bitfield-sign|designated-init|do-while|enum-mismatch|init-cstring|memcpy-max-count|non-pointer-null|old-initializer|one-bit-signed-bitfield|override-init-all|paren-string|ptr-subtraction-blows|return-void|sizeof-bool|sparse-all|sparse-error|transparent-union|typesign|undef|unknown-attribute)$/; return 1 if $arg =~ /^-v(no-?)?(entry|dead)$/; return 1 if $arg =~ /^-f(dump-ir|memcpy-max-count|diagnostic-prefix)(=\S*)?$/; return 1 if $arg =~ /^-f(mem2reg|optim)(-enable|-disable|=last)?$/; diff --git a/expression.c b/expression.c index 6f4300b..d6ad74e 100644 --- a/expression.c +++ b/expression.c @@ -324,7 +324,7 @@ static void get_number_value(struct expression *expr, struct token *token) show_token(token)); want_unsigned = 1; got_it: - if (do_warn) + if (do_warn && Wconstant_size) warning(expr->pos, "constant %s is so big it is%s%s%s", show_token(token), want_unsigned ? " unsigned":"", diff --git a/lib.c b/lib.c index 08dc299..a8ebb7d 100644 --- a/lib.c +++ b/lib.c @@ -253,6 +253,7 @@ int Wbitwise = 1; int Wcast_from_as = 0; int Wcast_to_as = 0; int Wcast_truncate = 1; +int Wconstant_size = 0; int Wconstexpr_not_const = 0; int Wcontext = 1; int Wdecl = 1; @@ -689,6 +690,7 @@ static const struct flag warnings[] = { { "cast-from-as", &Wcast_from_as }, { "cast-to-as", &Wcast_to_as }, { "cast-truncate", &Wcast_truncate }, + { "constant-size", &Wconstant_size }, { "constexpr-not-const", &Wconstexpr_not_const}, { "context", &Wcontext }, { "decl", &Wdecl }, diff --git a/lib.h b/lib.h index ae0e981..507eb69 100644 --- a/lib.h +++ b/lib.h @@ -141,6 +141,7 @@ extern int Wbitwise; extern int Wcast_from_as; extern int Wcast_to_as; extern int Wcast_truncate; +extern int Wconstant_size; extern int Wconstexpr_not_const; extern int Wcontext; extern int Wdecl; diff --git a/sparse.1 b/sparse.1 index 3e13523..26299b3 100644 --- a/sparse.1 +++ b/sparse.1 @@ -101,6 +101,16 @@ Sparse issues these warnings by default. To turn them off, use \fB\-Wno\-cast\-truncate\fR. . .TP +.B \-Wconstant-size +Warn if an integer constant is larger than the maximum representable value +of the type indicated by the type suffix (if any). For example, on a 64-bit +system, the constant 0xfffff00000000000U is larger than can be represented +by an int and so requires an unsigned long type suffix. (So, in this case, +the warning could be suppressed by using the UL type suffix). + +Sparse does not issue these warnings by default. +. +.TP .B \-Wconstexpr-not-const Warn if a non-constant expression is encountered when really expecting a constant expression instead. diff --git a/validation/constant-size-32.c b/validation/constant-size-32.c new file mode 100644 index 0000000..164734e --- /dev/null +++ b/validation/constant-size-32.c @@ -0,0 +1,15 @@ +#define BIGU 0xfffff00000000000U +#define BIGULL 0xfffff00000000000ULL + +static unsigned long long a = BIGU; +static unsigned long long b = BIGULL; + +/* + * check-name: constant-size + * check-command: sparse -m32 -Wconstant-size $file + * + * check-error-start +constant-size-32.c:4:31: warning: constant 0xfffff00000000000U is so big it is unsigned long long + * check-error-end + */ + diff --git a/validation/constant-size-64.c b/validation/constant-size-64.c new file mode 100644 index 0000000..471f6bc --- /dev/null +++ b/validation/constant-size-64.c @@ -0,0 +1,15 @@ +#define BIGU 0xfffff00000000000U +#define BIGUL 0xfffff00000000000UL + +static unsigned long a = BIGU; +static unsigned long b = BIGUL; + +/* + * check-name: constant-size + * check-command: sparse -m64 -Wconstant-size $file + * + * check-error-start +constant-size-64.c:4:26: warning: constant 0xfffff00000000000U is so big it is unsigned long + * check-error-end + */ +