Bug 7561 - gcc warnings with -Wpointer-arith when compiling modules
Summary: gcc warnings with -Wpointer-arith when compiling modules
Status: REJECTED INSUFFICIENT_DATA
Alias: None
Product: Other
Classification: Unclassified
Component: Other (show other bugs)
Hardware: i386 Linux
: P2 low
Assignee: other_other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-11-21 06:33 UTC by Joerg Czeranski
Modified: 2008-09-23 08:56 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.16, 2.6.16.32, 2.6.18.3
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
patch fixes warnings described in bug report (4.89 KB, patch)
2006-11-21 06:34 UTC, Joerg Czeranski
Details | Diff

Description Joerg Czeranski 2006-11-21 06:33:05 UTC
Most recent kernel where this bug did *NOT* occur: n/a
Distribution: SuSE 10.1
Hardware Environment: i386
Software Environment: the kernel headers themselves, gcc 4.1.0

Problem Description:

This isn't quite a bug, more a suggested improvement, so I'm not sure I selected
a reasonable category/component.

I'm compiling my own kernel modules with -Wpointer-arith added to the default
CFLAGS.  The reason is that it took me about one day to track a bug that would
have been caught in conforming C mode (with -Wpointer-arith) but was silently
ignored by gcc with the default flags.

(The code looked roughly like this:  p = kmalloc(sizeof *p, GFP_KERNEL);)

Now with -Wpointer-arith added most of the Linux header files compile just fine,
only two of them produce warnings.  Unfortunately they're needed (indirectly) by
just about every source file of my modules: linux/prefetch.h and asm-i386/io.h

I noticed the warnings first with SuSE 2.6.16.13-4, but I verified that they're
present in vanilla 2.6.16, 2.6.16.32 and 2.6.18.3, too.

I'll attach a patch for 2.6.16.32 that fixes these two warnings for all
architectures (tested only on i386).  It applies unmodified to Ingo Molnar's
2.6.16-rt29 and with offsets to 2.6.18.3.

If there is sufficient interest, I might try to compile the kernel itself with
-Wpointer-arith and have a look at the warnings.

N.b.: the code in the header files that triggers the two warnings is correct, it
does what its supposed to do (with gcc), its just not legal C.


Steps to reproduce:

Compile a module with CFLAGS += -Wpointer-arith and make sure you include
<linux/module.h> and <linux/io.h> or <asm/io.h>
Comment 1 Joerg Czeranski 2006-11-21 06:34:33 UTC
Created attachment 9581 [details]
patch fixes warnings described in bug report
Comment 2 Alexey Dobriyan 2006-11-21 09:49:37 UTC
Could you, please, clarify buggy snippet? Such things could be in mainline, so
someone could grep for them, instead of digging through noise which can be time
consuming and not very productive. And 2.6.19 release is near.

     Alexey "really curious" Dobriyan
Comment 3 Joerg Czeranski 2006-11-22 01:00:12 UTC
The actual faulty code was this:

struct sja1000_softc *sc_extra;
...
sc_extra = sc->extra = kmalloc(sizeof *sc->extra, GFP_KERNEL);

with sc->extra declared as void *.

The correct code is:
sc_extra = sc->extra = kmalloc(sizeof *sc_extra, GFP_KERNEL);

Note the different argument to sizeof.  The idiom p = kmalloc(sizeof *p,
GFP_KERNEL); guarantees that the right size of memory is used or an error is
reported.

For the wrong code, the size was sizeof(void), because sc->extra has type void
*, so it should have been rejected by the compiler as "sizeof of incomplete
type" or something similar.  Unfortunately, gcc without -Wpointer-arith treats
void as char in most contexts, so sizeof(void) is 1.  kmalloc() then allocates
the smallest supported slab size, usually 32 bytes.

This worked fine as long as struct sja1000_softc was small enough.  When I added
more fields to it, the module started overwriting random kernel data, whatever
was allocated after that struct.

I don't know whether there's an easy way to find such a bug with grep.  If
-Wpointer-arith is added, gcc 4.1.0 reports:
warning: invalid application of 'sizeof' to a void type

The easiest way might be to enable that warning, compile everything and grep for
the warning in the output.
Comment 4 Roman Zippel 2007-04-25 09:05:26 UTC
I guess you're better off to send a patch to the kernel mailing list for this.
Comment 5 Dave Jones 2007-06-06 08:20:40 UTC
Note that a bunch of new instances of that warning have been introduced since
your last patch, so they'll also need fixing up.

http://people.redhat.com/davej/kernels/Fedora/f8/RPMS.kernel/i586/build.log

(There are build.log's for other archs in the parent dir too, which may have
additional warnings).

But as Roman mentioned, bringing this up on Linux-kernel is probably the best
way forward.
Comment 6 Natalie Protasevich 2008-04-05 20:33:39 UTC
Has the patch ever been posted? If not, Joerg - can you please send it to the mailing list.
Thanks.

Note You need to log in before you can comment on or make changes to this bug.