linux-kernel - Re: [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20141013090153.GB25506@bill-the-cat>
Date:	Mon, 13 Oct 2014 05:01:53 -0400
From:	Tom Rini <trini@...com>
To:	Jeroen Hofstee <jeroen@...pectrum.nl>
Cc:	Simon Glass <sjg@...omium.org>,
	U-Boot Mailing List <u-boot@...ts.denx.de>,
	lk <linux-kernel@...r.kernel.org>,
	"Yann E. MORIN" <yann.morin.1998@...e.fr>,
	linux-kbuild@...r.kernel.org
Subject: Re: [U-Boot] [PATCH resend] kconfig: Fix compiler warning in menu.c

On Mon, Oct 13, 2014 at 08:48:39AM +0200, Jeroen Hofstee wrote:
> Hello Simon,
> 
> On 13-10-14 07:14, Simon Glass wrote:
> >Hi Jeroen,
> >
> >On 12 October 2014 10:13, Jeroen Hofstee <jeroen@...pectrum.nl> wrote:
> >
> >>Hello Hans,
> >>
> >>On 12-10-14 12:25, Hans de Goede wrote:
> >>
> >>>Hi,
> >>>
> >>>This one seems to have fallen through the cracks.
> >>>
> >>>Regards,
> >>>
> >>>Hans
> >>>
> >>>  (for U-boot)
> >>nope, you replace an innocent warning (_might_ be) with
> >>bad code, without any comment it is just because gcc failed
> >>to recognize it is fine. Nor did you respond to the suggestion
> >>if it helps gcc to recognize that if the two booleans are merged
> >>into a single one. [or even split it in an if () if ()]. With this patch
> >>you prevent any serious warning in case the variable is actually
> >>used but not initialized, which is even worse if you ask me.
> >>
> >That is a pretty acerbic tone to take on the U-Boot list at least. Are you
> >two drinking buddies or something?
> 
> no, it is because we have discussed this patch before and resending
> it won't address the issue raised. But you are right, it is likely done with
> less evil intends then I took it for, so let me explain my concern again
> in a politer way. The problem is that gcc 4.9 starts warning in the
> following case:
> 
> int *ptr;
> 
> if (a)
>     ptr = something;
> 
> if (a && b)
>     ptr->bla = value;
> else
>    do_something_else();
> 
> 
> it will warn that ptr _might_ be used uninitialized (but it always is).
> This is fixed in this patch by assigning NULL to ptr, and while that makes
> the warning go away it actually prevents the valid warning, ptr _is_ used
> uninitialized if you start using it in the else case. Hence my request if we
> can't find a better solution for this.
> 
> Does anyone know a better solution for this or should we consider
> disabling the might be unused warning?

Frankly, looking at the code, this is a compiler bug since as you note
the pointer will always be initalized.  Since we share this code as-is
with upstream kernel, we should see if there's any interst there in
trying to re-write the code so that it's (roughly):
if (a)
 ptr = valid;

if (a && b && ptr)
 ptr->foo = bar;

Or if this gets the required "compiler is being stupid, file a bug"
volume required.

-- 
Tom

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)