2014-11-21 04:22:35

by Peter Teoh

[permalink] [raw]
Subject: [PATCH] scripts/kconfig/menu.c warning for uninitialized "jump"

This warning was found in v3.18-rc3-68-g20f3963 of Linus git-tree.

SHIPPED scripts/kconfig/zconf.hash.c
HOSTCC scripts/kconfig/zconf.tab.o
In file included from scripts/kconfig/zconf.tab.c:2537:0:
scripts/kconfig/menu.c: In function ‘get_symbol_str’:
scripts/kconfig/menu.c:590:18: warning: ‘jump’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
jump->offset = strlen(r->s);
^
scripts/kconfig/menu.c:551:19: note: ‘jump’ was declared here
struct jump_key *jump;
^
HOSTCC scripts/kconfig/lxdialog/checklist.o
HOSTCC scripts/kconfig/lxdialog/util.o
HOSTCC scripts/kconfig/lxdialog/inputbox.o


The patch is provided below, to put in the extra checks for jump, and
it also added extra brackets to make the logical expression less
cryptic.

Signed-off-by: Peter Teoh <[email protected]>

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index a26cc5d..4d7eb61 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -586,7 +586,7 @@ static void get_prompt_str(struct gstr *r, struct
property *prop,
str_printf(r, _(" Location:\n"));
for (j = 4; --i >= 0; j += 2) {
menu = submenu[i];
- if (head && location && menu == location)
+ if (head && location && (menu == location) && (jump))
jump->offset = strlen(r->s);
str_printf(r, "%*c-> %s", j, ' ',
_(menu_get_prompt(menu)));


2014-11-28 13:33:53

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] scripts/kconfig/menu.c warning for uninitialized "jump"

On 2014-11-21 05:22, Peter Teoh wrote:
> This warning was found in v3.18-rc3-68-g20f3963 of Linus git-tree.
>
> SHIPPED scripts/kconfig/zconf.hash.c
> HOSTCC scripts/kconfig/zconf.tab.o
> In file included from scripts/kconfig/zconf.tab.c:2537:0:
> scripts/kconfig/menu.c: In function ‘get_symbol_str’:
> scripts/kconfig/menu.c:590:18: warning: ‘jump’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> jump->offset = strlen(r->s);
> ^
> scripts/kconfig/menu.c:551:19: note: ‘jump’ was declared here
> struct jump_key *jump;
> ^

First of all, the warning is bogus (the condition under which 'jump' is
used is stronger than that under which 'jump' is initialized). But since
people have been reporting the warning on and off for some time, we have
to shut it up somehow, as the affected gcc versions are not dying out,
apparently.


> - if (head && location && menu == location)
> + if (head && location && (menu == location) && (jump))
> jump->offset = strlen(r->s);

Let's assume, for the sake of argument, that gcc is right and jump may
be uninitialized here. Then the added check for jump being non-null just
tests an uninitialized variable and thus behaves randomly. It prevents
the code from writing to NULL->offset, but does not prevent it from
writing to <random address>->offset.

Michal

2014-11-28 18:50:17

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] scripts/kconfig/menu.c warning for uninitialized "jump"

On Fri, 2014-11-28 at 14:33 +0100, Michal Marek wrote:
> On 2014-11-21 05:22, Peter Teoh wrote:
> > This warning was found in v3.18-rc3-68-g20f3963 of Linus git-tree.
> >
> > SHIPPED scripts/kconfig/zconf.hash.c
> > HOSTCC scripts/kconfig/zconf.tab.o
> > In file included from scripts/kconfig/zconf.tab.c:2537:0:
> > scripts/kconfig/menu.c: In function ‘get_symbol_str’:
> > scripts/kconfig/menu.c:590:18: warning: ‘jump’ may be used
> > uninitialized in this function [-Wmaybe-uninitialized]
> > jump->offset = strlen(r->s);
> > ^
> > scripts/kconfig/menu.c:551:19: note: ‘jump’ was declared here
> > struct jump_key *jump;
> > ^
>
> First of all, the warning is bogus (the condition under which 'jump' is
> used is stronger than that under which 'jump' is initialized). But since
> people have been reporting the warning on and off for some time, we have
> to shut it up somehow, as the affected gcc versions are not dying out,
> apparently.
>
>
> > - if (head && location && menu == location)
> > + if (head && location && (menu == location) && (jump))
> > jump->offset = strlen(r->s);
>
> Let's assume, for the sake of argument, that gcc is right and jump may
> be uninitialized here. Then the added check for jump being non-null just
> tests an uninitialized variable and thus behaves randomly. It prevents
> the code from writing to NULL->offset, but does not prevent it from
> writing to <random address>->offset.

Maybe the 'right' thing to do is to mark the xmalloc
prototype as "__attribute__((returns_nonnull))"

2014-11-28 21:39:37

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] scripts/kconfig/menu.c warning for uninitialized "jump"

Dne 28.11.2014 v 19:50 Joe Perches napsal(a):
> On Fri, 2014-11-28 at 14:33 +0100, Michal Marek wrote:
>> On 2014-11-21 05:22, Peter Teoh wrote:
>>> This warning was found in v3.18-rc3-68-g20f3963 of Linus git-tree.
>>>
>>> SHIPPED scripts/kconfig/zconf.hash.c
>>> HOSTCC scripts/kconfig/zconf.tab.o
>>> In file included from scripts/kconfig/zconf.tab.c:2537:0:
>>> scripts/kconfig/menu.c: In function ‘get_symbol_str’:
>>> scripts/kconfig/menu.c:590:18: warning: ‘jump’ may be used
>>> uninitialized in this function [-Wmaybe-uninitialized]
>>> jump->offset = strlen(r->s);
>>> ^
>>> scripts/kconfig/menu.c:551:19: note: ‘jump’ was declared here
>>> struct jump_key *jump;
>>> ^
>>
>> First of all, the warning is bogus (the condition under which 'jump' is
>> used is stronger than that under which 'jump' is initialized). But since
>> people have been reporting the warning on and off for some time, we have
>> to shut it up somehow, as the affected gcc versions are not dying out,
>> apparently.
>>
>>
>>> - if (head && location && menu == location)
>>> + if (head && location && (menu == location) && (jump))
>>> jump->offset = strlen(r->s);
>>
>> Let's assume, for the sake of argument, that gcc is right and jump may
>> be uninitialized here. Then the added check for jump being non-null just
>> tests an uninitialized variable and thus behaves randomly. It prevents
>> the code from writing to NULL->offset, but does not prevent it from
>> writing to <random address>->offset.
>
> Maybe the 'right' thing to do is to mark the xmalloc
> prototype as "__attribute__((returns_nonnull))"

The warning says that 'jump' can be used uninitialized (random), not
that it can be NULL.

Michal