[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]
So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.
Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.
At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.
After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.
But a lot of the warnings look reasonable, and at least the warnings
are nice in how they actually explain why the warning is happening.
Example:
arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In
function ‘max7315_platform_data’:
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:35:
warning: ‘%d’ directive writing between 1 and 11 bytes into a region
of size 9 [-Wformat-overflow=]
sprintf(base_pin_name, "max7315_%d_base", nr);
^~
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26:
note: directive argument in the range [-2147483647, 2147483647]
Yeah, the compiler is technically correct, but we already made sure we
have at most MAX7315_NUM of those adapters, so no, "nr" is really not
going to be a 10-digit number.
So the warning is kind of bogus.
At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.
And some actually look valid, although they might still require odd input:
net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
^~~~~~~
net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
bytes into a destination of size 4
yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.
Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..
Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.
I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning. But let's see how
many of these we can fix, ok?
Linus
At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.
And some actually look valid, although they might still require odd input:
net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
^~~~~~~
net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
bytes into a destination of size 4
yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.
Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..
the Bluetooth specification defines that the Maximum Encryption Key Size shall be in the range 7 to 16 octets. Which is also reflected in these defines:
#define SMP_MIN_ENC_KEY_SIZE 7
#define SMP_MAX_ENC_KEY_SIZE 16
So it is buf[4] since we know it never gets larger than 16. So even in this case the warning is bogus.
I have no problem in increasing it to buf[5] to shut up the compiler, but that is what I would be doing here. I am not fixing an actual bug.
Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.
I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning. But let's see how
many of these we can fix, ok?
I had to use the -Wno-format-trunction in a few projects since gcc was completely lost. And since we were using snprintf, I saw no point in trying to please gcc with a larger buffer.
Regards
Marcel
[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]
So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.
Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.
At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.
After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.
The hwmon warnings are all about supporting no more than 9,999 sensors
(applesmc) to 999,999,999 sensors (scpi) of a given type. Easy "fix" would
be to replace snprintf() with scnprintf(), presumably because gcc doesn't
know about scnprintf(). We could also increase the name buffer size.
But is that really worth it just to silence gcc ?
Guenter
On Tue, Jul 11, 2017 at 8:10 PM, Guenter Roeck [off-list ref] wrote:
The hwmon warnings are all about supporting no more than 9,999 sensors
(applesmc) to 999,999,999 sensors (scpi) of a given type.
Yeah, I think that's enough.
Easy "fix" would be to replace snprintf() with scnprintf(), presumably
because gcc doesn't know about scnprintf().
If that's the case, I'd prefer just turning off the format-truncation
(but not overflow) warning with '-Wno-format-trunction".
But maybe we can at least start it on a subsystem-by-subsystem basis
after people have verified their own subsusystem?
Linus
On Tue, Jul 11, 2017 at 8:17 PM, Linus Torvalds
[off-list ref] wrote:
If that's the case, I'd prefer just turning off the format-truncation
(but not overflow) warning with '-Wno-format-trunction".
Doing
KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
in the main Makefile certainly cuts down on the warnings.
We still have some overflow warnings, including the crazy one where
gcc doesn't see that the number of max7315 boards is very limited.
But those could easily be converted to just snprintf() instead, and
then the truncation warning disabling takes care of it. Maybe that's
the right answer.
We also have about a bazillion
warning: ‘*’ in boolean context, suggest ‘&&’ instead
warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.
Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?
Linus
From: Jakub Kicinski <hidden> Date: 2017-07-12 04:26:23
On Tue, 11 Jul 2017 15:35:15 -0700, Linus Torvalds wrote:
I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning. But let's see how
many of these we can fix, ok?
Somehow related - what's the stand on -Wimplicit-fallthrough? I run
into the jump tables in jhash.h generating lots of warnings. Is it OK
to do this?
--->8------
@@ -85,20 +85,19 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)k+=12;}/* Last block: affect all 32 bits of (c) */-/* All the case statements fall through */switch(length){-case12:c+=(u32)k[11]<<24;-case11:c+=(u32)k[10]<<16;-case10:c+=(u32)k[9]<<8;-case9:c+=k[8];-case8:b+=(u32)k[7]<<24;-case7:b+=(u32)k[6]<<16;-case6:b+=(u32)k[5]<<8;-case5:b+=k[4];-case4:a+=(u32)k[3]<<24;-case3:a+=(u32)k[2]<<16;-case2:a+=(u32)k[1]<<8;-case1:a+=k[0];+case12:c+=(u32)k[11]<<24;/* fall through */+case11:c+=(u32)k[10]<<16;/* fall through */+case10:c+=(u32)k[9]<<8;/* fall through */+case9:c+=k[8];/* fall through */+case8:b+=(u32)k[7]<<24;/* fall through */+case7:b+=(u32)k[6]<<16;/* fall through */+case6:b+=(u32)k[5]<<8;/* fall through */+case5:b+=k[4];/* fall through */+case4:a+=(u32)k[3]<<24;/* fall through */+case3:a+=(u32)k[2]<<16;/* fall through */+case2:a+=(u32)k[1]<<8;/* fall through */+case1:a+=k[0];/* fall through */__jhash_final(a,b,c);case0:/* Nothing left to add */break;
@@ -131,11 +130,11 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)k+=3;}-/* Handle the last 3 u32's: all the case statements fall through */+/* Handle the last 3 u32's */switch(length){-case3:c+=k[2];-case2:b+=k[1];-case1:a+=k[0];+case3:c+=k[2];/* fall through */+case2:b+=k[1];/* fall through */+case1:a+=k[0];/* fall through */__jhash_final(a,b,c);case0:/* Nothing left to add */break;
[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]
Under drivers/media, I fixed a bunch of gcc 7.1 warnings before the
merge window. While most were just noise, some actually pointed to
human errors.
Now, gcc-7.1.1 produces only 6 warnings with W=1 on x86_64 (allyesconfig),
either due to unused-but-set-variable or unused-const-variable. I guess
both warning options are disabled by default. Anyway, I have patches
to fix them already. I'll send you later.
The atomisp staging driver is a completely different beast, with would
produce itself a huge amount of warnings. I ended by adding some
logic on drivers/staging/media/atomisp/ Makefiles to disable them:
ccflags-y += $(call cc-disable-warning, missing-declarations)
ccflags-y += $(call cc-disable-warning, missing-prototypes)
ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
ccflags-y += $(call cc-disable-warning, unused-const-variable)
ccflags-y += $(call cc-disable-warning, suggest-attribute=format)
ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
(there's actually one patch pending related to atomisp, that I'll also
be sending you soon - meant to avoid warnings if compiled with an older
gcc version)
Thanks,
Mauro
On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]
So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.
I asked Arnd about this the other day on IRC as I've hit this as well on
the stable releases, and it's really annoying. He mentioned that he had
lots of these warnings fixed, but didn't push most of the changes out
yet. Arnd, any repo with them in it that we could look at?
Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.
At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.
After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.
Yeah, that's the warnings in the USB core code, we "know" this will not
happen, and we are using snprintf() for that reason as well, I don't
know how to fool gcc into the fact that it's all ok here.
Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.
I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
Fedora turned more warnings on in their compiler release, I'm running
Arch here:
$ gcc --version
gcc (GCC) 7.1.1 20170621
thanks,
greg k-h
On Wed, Jul 12, 2017 at 5:41 AM, Linus Torvalds
[off-list ref] wrote:
We also have about a bazillion
warning: ‘*’ in boolean context, suggest ‘&&’ instead
warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.
Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?
On Wed, Jul 12, 2017 at 3:10 PM, Greg Kroah-Hartman
[off-list ref] wrote:
On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
quoted
[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]
So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.
I asked Arnd about this the other day on IRC as I've hit this as well on
the stable releases, and it's really annoying. He mentioned that he had
lots of these warnings fixed, but didn't push most of the changes out
yet.
To clarify: most of the patches I wrote ended up getting merged, but
there were a couple that I did not submit a second time after they
got dropped, but I gave up on trying to fix the new -Wformat warnings
and simply disabled them, hoping someone else would do it before me,
or that the gcc developers would find a way to reduce the false-positive
ones before the release.
Arnd, any repo with them in it that we could look at?
I have a private tree on my workstation that has lots of random
crap, and I rebase it all the time but normally don't publish it.
I have uploaded today's snapshot to
git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git randconfig-4.13-next
The way I work with this is helpful to catch build regressions as soon
as they happen, but not so good in finding things that I have either
submitted a patch for before and don't remember if it should be
resubmitted, or stuff that I decided I didn't want to deal with at some
point.
I was already planning to start over from scratch one of these days,
and cherry-pick+resubmit the patches that are actually required
for randconfig builds.
quoted
Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.
I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
Fedora turned more warnings on in their compiler release, I'm running
Arch here:
$ gcc --version
gcc (GCC) 7.1.1 20170621
This is what I get in today's linux-next:
$ grep error: 4.13-next-allmod-warning | sed -e 's:^.*\[-W:-W:' | sort
| uniq -c | cut -f 1 -d\] | sort -n
1 -Werror=parentheses
2 -Werror=tautological-compare
2 -Werror=unused-result
34 -Werror=format-overflow 41 -Werror=int-in-bool-context
233 -Werror=format-truncation
I'll resubmit the patches for -Wparenthese, -Wtautological-compar,
-Wunused-result and -Wint-in-bool-context that I had sent earlier,
plus a new patch to move -Wformat-truncation into W=1.
Arnd
Hello,
On Wed, Jul 12, 2017 at 03:31:02PM +0200, Arnd Bergmann wrote:
quoted
We also have about a bazillion
warning: ‘*’ in boolean context, suggest ‘&&’ instead
warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.
Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?
Tejun applied an almost identical patch of mine a while ago, but it seems to
have gotten lost in the meantime in some rebase:
Yeah, I was scratching my head remembering your patch. Sorry about
that. It should have been routed through for-4.12-fixes.