[KJ] gcc4 warnings, variable used uninitialized

[KJ] gcc4 warnings, variable used uninitialized

[Stephan Peijnik]

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

Hello list!

Does anyone know exactly how gcc4 decides whether a variable is
undefined or not and how that should be handled kernel-wise?

I've come across a few snippets where for example a temporary
u32 is declared and then a pointer to that variable is given
to pci_read_config_dword. When that variable is used afterwards gcc4
complains about it being not initialized (Using gcc-4.0.2 and kernel
2.6.16-rc4 as of yesterday).

Should 'bugs' like those be fixed? I've found like a gazillion of
them in the AGP drivers and other parts of the kernel.
I'm thinking about that as it seems to be the best point to get
myself started and I would appreciate comments on that topic.

Additionally, a comment on whether my last submitted patch is okay
(kauditd_thread not returning a value) would be nice aswell. Probably
including directions on where to send that patch to.

Thanks in advance.

Regards,

Stephan Peijnik


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] gcc4 warnings, variable used uninitialized

[Arnd Bergmann]

On Tuesday 21 February 2006 13:17, Stephan Peijnik wrote:
> Does anyone know exactly how gcc4 decides whether a variable is
> undefined or not and how that should be handled kernel-wise?

It has been found that gcc-4.0 gets it wrong. Most of the
places where gcc-4.0 wrongly complains and gcc-3.4 does not
have been fixed in gcc-4.1.

> I've come across a few snippets where for example a temporary
> u32 is declared and then a pointer to that variable is given
> to pci_read_config_dword. When that variable is used afterwards gcc4
> complains about it being not initialized (Using gcc-4.0.2 and kernel
> 2.6.16-rc4 as of yesterday).
> 
> Should 'bugs' like those be fixed? I've found like a gazillion of
> them in the AGP drivers and other parts of the kernel.
> I'm thinking about that as it seems to be the best point to get
> myself started and I would appreciate comments on that topic.

A small number of these bugs are valid and should be fixed, but
it is not trivial to determine in some cases. What you say about
this happening with pci_read_config_dword sounds like it is a gcc
bug and not a bug in the kernel sources, so it should not be fixed
in the kernel.

Can you be more specific with the example and/or reproduce
it with gcc-4.1?

	Arnd <><
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] gcc4 warnings, variable used uninitialized

[Stephan Peijnik]

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

On Tue, 2006-02-21 at 13:42 +0100, Arnd Bergmann wrote:
> It has been found that gcc-4.0 gets it wrong. Most of the
> places where gcc-4.0 wrongly complains and gcc-3.4 does not
> have been fixed in gcc-4.1.

Okay, I was about to go and use good old 3.4 for testing purposes.
I've been wondering about the warnings anyways.

> A small number of these bugs are valid and should be fixed, but
> it is not trivial to determine in some cases. What you say about
> this happening with pci_read_config_dword sounds like it is a gcc
> bug and not a bug in the kernel sources, so it should not be fixed
> in the kernel.

I wasn't that sure whether those warnings would be right and kept
forever or not. Doing those minimalistic modifications probably wouldn't
have done any harm, but you are right about where the problem(s) should
be fixed.

> 
> Can you be more specific with the example and/or reproduce
> it with gcc-4.1?

I'm compiling a fresh copy of 4.1 right now, but a good example would be
drivers/char/agp/generic.c's get_agp_version function. It defines a 'u32
ncapid'. A pointer to that u32 is then given to pci_read_config_dword
and at exactly that point gcc-4.0 starts complaining. See below.

<snip>
void get_agp_version(struct agp_bridge_data *bridge)
{
	u32 ncapid;

	/* Exit early if already set by errata workarounds. */
	if (bridge->major_version != 0)
		return;

	pci_read_config_dword(bridge->dev, bridge->capndx, &ncapid);
[...]
</snip>

However, thanks for the response. On that topic everything is kind of
clear to me now.


Regards,

Stephan Peijnik


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] gcc4 warnings, variable used uninitialized

[Dave Jones]

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

On Tue, Feb 21, 2006 at 09:12:28PM +0100, Stephan Peijnik wrote:

 > I'm compiling a fresh copy of 4.1 right now, but a good example would be
 > drivers/char/agp/generic.c's get_agp_version function. It defines a 'u32
 > ncapid'. A pointer to that u32 is then given to pci_read_config_dword
 > and at exactly that point gcc-4.0 starts complaining. See below.
 > 
 > <snip>
 > void get_agp_version(struct agp_bridge_data *bridge)
 > {
 > 	u32 ncapid;
 > 
 > 	/* Exit early if already set by errata workarounds. */
 > 	if (bridge->major_version != 0)
 > 		return;
 > 
 > 	pci_read_config_dword(bridge->dev, bridge->capndx, &ncapid);
 > [...]
 > </snip>

gcc bug.  It doesn't seem to understand that sometimes ptrs to vars get
passed to functions that instantiate them.

There are dozens and dozens of such instances in the kernel tree.
99% of the time that warning is completely bogus.

		Dave


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] gcc4 warnings, variable used uninitialized

[Håkon Løvdal]

On 2/21/06, Dave Jones <davej@redhat.com> wrote:
>  > <snip>
>  > void get_agp_version(struct agp_bridge_data *bridge)
>  > {
>  >      u32 ncapid;
>  >
>  >      /* Exit early if already set by errata workarounds. */
>  >      if (bridge->major_version != 0)
>  >              return;
>  >
>  >      pci_read_config_dword(bridge->dev, bridge->capndx, &ncapid);
>  > [...]
>  > </snip>
>
> gcc bug.  It doesn't seem to understand that sometimes ptrs to vars get
> passed to functions that instantiate them.
>
> There are dozens and dozens of such instances in the kernel tree.
> 99% of the time that warning is completely bogus.

I would not call it a bug. The problem is that gcc does not know if the
third argument of pci_read_config_dword is used as an in parameter or
out parameter.

If the pointer is used as an in parameter, then the code above is wrong
and pci_read_config_dword will read some random memory (which gcc rightly
should warn about). If the pointer only is used as out parameter then
there is no problem but since gcc cannot tell, it issues a warning in
any case. Because of that the warning will most likely give many
false alarms and be quite unuseful, but it is not wrong I think.

BR Håkon Løvdal

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] gcc4 warnings, variable used uninitialized

[Bernd Petrovitsch]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On Wed, 2006-02-22 at 00:51 +0100, Håkon Løvdal wrote:
> On 2/21/06, Dave Jones <davej@redhat.com> wrote:
> >  > <snip>
> >  > void get_agp_version(struct agp_bridge_data *bridge)
> >  > {
> >  >      u32 ncapid;
> >  >
> >  >      /* Exit early if already set by errata workarounds. */
> >  >      if (bridge->major_version != 0)
> >  >              return;
> >  >
> >  >      pci_read_config_dword(bridge->dev, bridge->capndx, &ncapid);
> >  > [...]
> >  > </snip>
> >
> > gcc bug.  It doesn't seem to understand that sometimes ptrs to vars get
> > passed to functions that instantiate them.
> >
> > There are dozens and dozens of such instances in the kernel tree.
> > 99% of the time that warning is completely bogus.
> 
> I would not call it a bug. The problem is that gcc does not know if the
> third argument of pci_read_config_dword is used as an in parameter or
> out parameter.

But gcc claims that you use it before actually initializing it (without
actually really knowing it).

Analysis: At most it is possible to declare that parameter as "const
<type> *" which implies that it isn't expected to be written to. This
implies that the parameter is basically read-only (in theory).
And *then* the C compiler can (and should) warn.

If the parameter has no "const" attached as above, the C compiler has no
way (except analyzing the source code) to tell if the parameter, is read
before written too or not.
So the compiler should shut up instead of (possibly) claiming falsely
that the variable is used without initialization.

To go one step further one may say that if the parameter is not "const"
it may be written to (otherwise it should be "const" in these days). But
even this doesn't rule out that it is read before written too (where the
warning makes sense).

To be more contructive: Is it OK to spend the (mostly useless)
initialization code and time (ans shut up the warnings) or not? Or do we
simply wait for a (in this aspect) sane gcc and the problem is solved
anyway.
These are the real questions IMHO.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services




[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] gcc4 warnings, variable used uninitialized

[Dave Jones]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

On Wed, Feb 22, 2006 at 01:19:27AM +0100, Bernd Petrovitsch wrote:

 > To be more contructive: Is it OK to spend the (mostly useless)
 > initialization code and time (ans shut up the warnings) or not? Or do we
 > simply wait for a (in this aspect) sane gcc and the problem is solved
 > anyway.

Ignore them, and wait for a fixed compiler.

		Dave

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors