-Wmaybe-uninitialized bug fixes for linux-next

Started by Arnd Bergmann on Aug 26, 2016 · 5 messages from 3 authors · Last reply Aug 30, 2016

Series

5 patches, 5 files

+29 −23

Subsystems (8)

Show 5 more

Maintainers (16)

Show 12 more
STALE3722d

open the first message on its own page

  1. [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic
    Related patches
Message details and related content
In 6e8d666e9253 ("Disable "maybe-uninitialized" warning globally"),
Linus wrote:

    Looking at the warnings produced, every single one I looked at was a
    false positive, and the warnings are frequent enough (and big enough)
    that they can easily hide real problems that you don't notice in
    the noise generated by -Wmaybe-uninitialized.

Today, I tried reverting the patch on linux-next and built an ARM
allmodconfig kernel on ARM along with some randconfig kernels,
and got a handful of warnings, all of which appear to be reasonable
and point to actual mistakes in the code. The difference to what
Linus saw must be that previously the useful warnings were more
likely to get fixed before making it into the kernel, while now
we have to find them the hard way.

These five patches address all new warnings. In some cases this
may not be the correct fix, so please review carefully before applying,
or suggest a better fix. No need to keep them as a series, I
just group them here for the sake of discussion. Please pick up
whatever looks right to you.

Obviously, this kind of warnings always produces some false positives
(see https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings), but I still
hope to get a better balance with enabling them sometimes where
people want them, as the current approach of always enabling them
for "make W=1" but never by default seems suboptimal: We had previously
identified a number of options (CONFIG_CC_OPTIMIZE_FOR_SIZE,
CONFIG_PROFILE_ALL_BRANCHES, CONFIG_UBSAN_ALIGNMENT, and
CONFIG_GCOV_PROFILE_ALL) that cause tons of false positives,
but without those options (and avoiding gcc-4.8 or lower),
we typically get mostly reports for actual bugs in my experience.

I can continue running the tests and send patches, but it feels
like a waste of time when they should have been found by the
original developers. Any other suggestions?

	Arnd

Arnd Bergmann (5):

33 more lines

[PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic

Message details and related content
The addition of the Nomadik support in this driver introduced
a bug in clcdfb_of_init_display(), which now calls init_panel
with an uninitialized 'endpoint' pointer, as "gcc -Wmaybe-uninitialized"
warns:

drivers/video/fbdev/amba-clcd.c: In function 'clcdfb_of_init_display':
drivers/video/fbdev/amba-clcd.c:785:5: error: 'endpoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This reverts the broken part of the function to what it was before
the patch, which is the best guess I have to what it should be.
I assume this was left over from an attempted rework of the
code that was partially backed out.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 046ad6cdeb3f ("video: ARM CLCD: support Nomadik variant")
Cc: Linus Walleij <redacted>
---
Cc: <redacted>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Tomi Valkeinen <redacted>
 drivers/video/fbdev/amba-clcd.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index c342ff370108..ec2671d98abc 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -782,12 +782,9 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
 	/*
 	 * Fetch the panel endpoint.
 	 */
-	if (!endpoint) {
-		endpoint = of_graph_get_next_endpoint(fb->dev->dev.of_node,
-						      NULL);
-		if (!endpoint)
-			return -ENODEV;
-	}
+	endpoint = of_graph_get_next_endpoint(fb->dev->dev.of_node, NULL);
+	if (!endpoint)
+		return -ENODEV;
 
 	if (fb->vendor->init_panel) {
 		err = fb->vendor->init_panel(fb, endpoint);
-- 
2.9.0

Re: [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic

Message details and related content
On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann [off-list ref] wrote:

[I don't see why Torvalds was CC'ed on this patch? Was there
some specific complaint from his side that I screw things up
or just the wrong Linus?]
The addition of the Nomadik support in this driver introduced
a bug in clcdfb_of_init_display(), which now calls init_panel
with an uninitialized 'endpoint' pointer, as "gcc -Wmaybe-uninitialized"
warns:

drivers/video/fbdev/amba-clcd.c: In function 'clcdfb_of_init_display':
drivers/video/fbdev/amba-clcd.c:785:5: error: 'endpoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This reverts the broken part of the function to what it was before
the patch, which is the best guess I have to what it should be.
I assume this was left over from an attempted rework of the
code that was partially backed out.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 046ad6cdeb3f ("video: ARM CLCD: support Nomadik variant")
Cc: Linus Walleij <redacted>
Reviewed-by: Linus Walleij <redacted>

Tomi: I think this bug was also reported by Ian King, so suggest
adding his
Reported-by: Colin Ian King <redacted>

When applying.

Yours,
Linus Walleij

Re: [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic

Message details and related content
On Monday 29 August 2016, Linus Walleij wrote:
On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann [off-list ref] wrote:

[I don't see why Torvalds was CC'ed on this patch? Was there
some specific complaint from his side that I screw things up
or just the wrong Linus?]
It was intentional: as mentioned in the introductory mail, he did
the patch that prevented you from seeing the five warnings that I got
after reverting 6e8d666e9253.

	Arnd

Re: [PATCH 2/5] video: ARM CLCD: fix endpoint lookup logic

Message details and related content
On 29/08/16 16:18, Linus Walleij wrote:
On Fri, Aug 26, 2016 at 5:25 PM, Arnd Bergmann [off-list ref] wrote:

[I don't see why Torvalds was CC'ed on this patch? Was there
some specific complaint from his side that I screw things up
or just the wrong Linus?]
quoted
The addition of the Nomadik support in this driver introduced
a bug in clcdfb_of_init_display(), which now calls init_panel
with an uninitialized 'endpoint' pointer, as "gcc -Wmaybe-uninitialized"
warns:

drivers/video/fbdev/amba-clcd.c: In function 'clcdfb_of_init_display':
drivers/video/fbdev/amba-clcd.c:785:5: error: 'endpoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This reverts the broken part of the function to what it was before
the patch, which is the best guess I have to what it should be.
I assume this was left over from an attempted rework of the
code that was partially backed out.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 046ad6cdeb3f ("video: ARM CLCD: support Nomadik variant")
Cc: Linus Walleij <redacted>
Reviewed-by: Linus Walleij <redacted>

Tomi: I think this bug was also reported by Ian King, so suggest
adding his
Reported-by: Colin Ian King <redacted>
Thanks, queued for v4.9.

 Tomi
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help