[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] maint: accommodate gcc's -Wstrict-overflow option
From: |
Jim Meyering |
Subject: |
Re: [PATCH 1/2] maint: accommodate gcc's -Wstrict-overflow option |
Date: |
Fri, 27 May 2011 00:36:19 +0200 |
Pádraig Brady wrote:
> On 26/05/11 17:06, Jim Meyering wrote:
>> There's a nasty bug in gcc, mentioned in log and comments below:
>> (It converts a loop that would obviously terminate into one
>> that never does, just because a signed accumulator may overflow.
>> The crazy part is that the offending accumulator is not related
>> to the loop termination condition. )
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33498
>>
>> Once you read that and understand the implications, you'll
>> realize why I (the first to reject -Wstrict-overflow as not
>> worth accommodating) have decided that finally it is better
>> to use that option than to risk being bitten by the gcc bug.
>
>> -Wstrict-overflow does detect vulnerable code like the
>> example in the bug report.
>
>> Accommodating it required changes in 4 programs and tweaks
>> to configure.ac so that --enable-gcc-warnings now enables
>> that option in src/, but not in lib/ or in gnulib-tests/.
>> There would be 3 warnings in lib/ -- for two of them I
>> have patches.
>
> Great, something off my todo list :)
> The other option is to specify -fno-strict-overflow,
> though given the relatively minor changes to accommodate
> -Wstrict-overflow[=2], I think this is the way to go.
>
> The changes look good.
Thanks for the review.
There was one other trivial change, in pr.c:
Here's what I've merged into that change set:
commit 419b6c9d42ba643265f802cd150d0b232e43186a
Author: Jim Meyering <address@hidden>
Date: Wed May 25 12:29:18 2011 +0200
maint: accommodate gcc's -Wstrict-overflow option
* src/factor.c (factor_using_pollard_rho): Change type of "i"
to unsigned to avoid warning from gcc's -Wstrict-overflow.
* src/expr.c: Use an unsigned intermediate.
* src/dircolors.c (main): Reorder operations to avoid the risk of
pointer overflow.
* src/tr.c (squeeze_filter): Change NOT_A_CHAR from an anonymous
"enum" to an "int", to avoid this warning:
tr.c:1624:10: error: assuming signed overflow does not occur when
simplifying conditional to constant [-Werror=strict-overflow]
* src/pr.c (main): Make index "i" unsigned.
diff --git a/src/pr.c b/src/pr.c
index 7e6b13c..3995c37 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -1165,7 +1165,7 @@ main (int argc, char **argv)
print_files (n_files, file_names);
else
{
- int i;
+ unsigned int i;
for (i = 0; i < n_files; i++)
print_files (1, &file_names[i]);
}