threads / patch / 29189

patchFix an "variable might be used uninitialized" gcc warning

Subject: [PATCH] Fix an "variable might be used uninitialized" gcc warning

## tl;dr

7 messages between Dec 16, 2011 and Feb 2, 2012. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Ramsay Jones· Dec 16, 2011, 22:44 UTC · lore
In particular, gcc issues the following warning:
        CC builtin/checkout.o
    builtin/checkout.c: In function `cmd_checkout':
    builtin/checkout.c:160: warning: 'mode' might be used uninitialized \
        in this function

However, the analysis performed by gcc is too conservative, in this case, since the mode variable will not be used uninitialised. Note that, if the mode variable is not set in the loop, then "threeway[1]" will also still be set to the null SHA1. This will then result in control leaving the function, almost directly after the loop, well before the potential use in the call to make_cache_entry().

In order to suppress the warning, we initialise the mode variable to zero in it's declaration.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Just in case you haven't found the time to apply your own patch!

[Note that only 2 out of the 3 versions of gcc I use issues this warning]

ATB, Ramsay Jones

 builtin/checkout.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin/checkout.c +1 −1
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 787d468..f1984d9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -157,7 +157,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	unsigned char sha1[20];
 	mmbuffer_t result_buf;
 	unsigned char threeway[3][20];
-	unsigned mode;
+	unsigned mode = 0;
 
 	memset(threeway, 0, sizeof(threeway));
 	while (pos < active_nr) {

-- 
1.7.8
Jonathan Nieder· Dec 16, 2011, 23:59 UTC · re: Ramsay Jones · lore

Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning

Ramsay Jones wrote:
>         CC builtin/checkout.o
>     builtin/checkout.c: In function `cmd_checkout':
>     builtin/checkout.c:160: warning: 'mode' might be used uninitialized \
>         in this function
[...]
> [Note that only 2 out of the 3 versions of gcc I use issues this
> warning]

Which version of gcc is that? Is gcc getting more sane, so we won't have to worry about this after a while, or is the false positive a new regression that should be reported to them?

Andreas Schwab· Dec 17, 2011, 10:22 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 13 quoted lines
> Ramsay Jones wrote:
>
>>         CC builtin/checkout.o
>>     builtin/checkout.c: In function `cmd_checkout':
>>     builtin/checkout.c:160: warning: 'mode' might be used uninitialized \
>>         in this function
> [...]
>> [Note that only 2 out of the 3 versions of gcc I use issues this
>> warning]
>
> Which version of gcc is that?  Is gcc getting more sane, so we won't
> have to worry about this after a while, or is the false positive a
> new regression that should be reported to them?

The regression is that the function has been changed in a way that makes it impossible to infer the intended flow.

Andreas.
-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Ramsay Jones· Jan 31, 2012, 18:36 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning

Jonathan Nieder wrote:
Show 13 quoted lines
> Ramsay Jones wrote:
> 
>>         CC builtin/checkout.o
>>     builtin/checkout.c: In function `cmd_checkout':
>>     builtin/checkout.c:160: warning: 'mode' might be used uninitialized \
>>         in this function
> [...]
>> [Note that only 2 out of the 3 versions of gcc I use issues this
>> warning]
> 
> Which version of gcc is that?  Is gcc getting more sane, so we won't
> have to worry about this after a while, or is the false positive a
> new regression that should be reported to them?
[Sorry for the late reply, I've been away from email for several weeks...]

The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles the code without complaint. So, gcc *may* be getting more sane, but I wouldn't bet on it! :-P

I've had examples of this kind of warning, which relies heavily on the analysis performed primarily for the optimizer, come-and-go in gcc before; so don't hold your breath (this is the most volatile part of the compiler).

Having said that, unless you are going to decree that the project only supports gcc (and presumably only some particular versions of gcc), then you may well find similar warnings triggered when using other compilers anyway ...

ATB, Ramsay Jones

Jonathan Nieder· Jan 31, 2012, 19:43 UTC · re: Ramsay Jones · lore

Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning

Ramsay Jones wrote:
Show 6 quoted lines
> The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles
> the code without complaint. So, gcc *may* be getting more sane, but I wouldn't
> bet on it! :-P
>
> I've had examples of this kind of warning, which relies heavily on the
> analysis performed primarily for the optimizer, come-and-go in gcc before

Yep, judging from the commit message, Junio found the same warning in 4.6.2.

[...]
> Having said that, unless you are going to decree that the project only
> supports gcc (and presumably only some particular versions of gcc), then you
> may well find similar warnings triggered when using other compilers anyway ...

Sure, when the control flow grows too complicated, that's probably worth fixing anyway, for the sake of humans especially.

Sometimes gcc is the only crazy one, though. ;-)

Thanks for the update. Jonathan

Miles Bader· Feb 1, 2012, 07:16 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 18 quoted lines
>> The versions which complain are 3.4.4 and 4.1.2, whereas 4.4.0 compiles
>> the code without complaint. So, gcc *may* be getting more sane, but I wouldn't
>> bet on it! :-P
>>
>> I've had examples of this kind of warning, which relies heavily on the
>> analysis performed primarily for the optimizer, come-and-go in gcc before
>
> Yep, judging from the commit message, Junio found the same warning
> in 4.6.2.
>
>> Having said that, unless you are going to decree that the project only
>> supports gcc (and presumably only some particular versions of gcc), then you
>> may well find similar warnings triggered when using other compilers anyway ...
>
> Sure, when the control flow grows too complicated, that's probably worth
> fixing anyway, for the sake of humans especially.
>
> Sometimes gcc is the only crazy one, though. ;-)

It's hard to see how any compiler could detect that "mode" always receives a value here .... it would have to realize that "stage" always becomes 2 before the loop is exited, and that seems to depend on non-trivial properties of external data structures...

-miles
-- 
Joy, n. An emotion variously excited, but in its highest degree arising from
the contemplation of grief in another.
Ramsay Jones· Feb 2, 2012, 18:25 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Fix an "variable might be used uninitialized" gcc warning

Jonathan Nieder wrote:
> Sure, when the control flow grows too complicated, that's probably worth
> fixing anyway, for the sake of humans especially.
> 
> Sometimes gcc is the only crazy one, though. ;-)
Indeed. :-D

ATB, Ramsay Jones

← back to recent threads