2003-06-05 21:21:10

by Hollis Blanchard

[permalink] [raw]
Subject: [CHECKER][PATCH] awe_wave.c user pointer dereference

Two ioctl functions in sound/oss/awe_wave.c were directly dereferencing
a user-supplied pointer in a few places. Please apply.

--
Hollis Blanchard
IBM Linux Technology Center


Attachments:
awe-userptr.txt (1.08 kB)

2003-06-05 21:54:15

by Linus Torvalds

[permalink] [raw]
Subject: Re: [CHECKER][PATCH] awe_wave.c user pointer dereference


On Thu, 5 Jun 2003, Hollis Blanchard wrote:
>
> Two ioctl functions in sound/oss/awe_wave.c were directly dereferencing
> a user-supplied pointer in a few places. Please apply.

When you do patches like this, can you please add the "__user" annotations
while you're at it? Also, if your mailer doesn't rape whitespace, I
seriously prefer patches in-line in the email, so that I don't have to
edit the email and can reply to it directly?

Linus


2003-06-06 16:18:07

by Hollis Blanchard

[permalink] [raw]
Subject: Re: [CHECKER][PATCH] awe_wave.c user pointer dereference

On Thursday, Jun 5, 2003, at 17:52 US/Central, Andrew Morton wrote:

> Hollis Blanchard <[email protected]> wrote:
>>
>> + return -EFAULT;
>> return 0;
>> break;
>>
>> @@ -2063,10 +2064,12 @@
>>
>> case SNDCTL_SYNTH_MEMAVL:
>> return memsize - awe_free_mem_ptr() * 2;
>> + break;
>>
>> default:
>> printk(KERN_WARNING "AWE32: unsupported ioctl %d\n", cmd);
>> return -EINVAL;
>> + break;
>
> There's no need for a "break" after a "return"!

I've gotten a couple questions about that. :)

I did it for two reasons: 1) I'm a big fan of consistancy, and 3 of the
5 ioctl cases already had the break. 2) should the returns be replaced
with something like "retval = -EINVAL" and a single "return retval" at
the end of the function, I can easily imagine visually missing the need
for a break.

However, although a break after a return is fine with gcc -Wall, Joe
Perches informs me that it will annoy lint and the Intel compiler. So
here is the updated patch, which removes the (pre-existing! :) breaks
from the other cases.

--
Hollis Blanchard
IBM Linux Technology Center

===== sound/oss/awe_wave.c 1.12 vs edited =====
--- 1.12/sound/oss/awe_wave.c Thu Apr 3 16:35:48 2003
+++ edited/sound/oss/awe_wave.c Fri Jun 6 11:17:19 2003
@@ -2046,20 +2046,18 @@
awe_info.nr_voices = awe_max_voices;
else
awe_info.nr_voices = AWE_MAX_CHANNELS;
- memcpy((char*)arg, &awe_info, sizeof(awe_info));
+ if (copy_to_user((char*)arg, &awe_info, sizeof(awe_info)))
+ return -EFAULT;
return 0;
- break;

case SNDCTL_SEQ_RESETSAMPLES:
awe_reset(dev);
awe_reset_samples();
return 0;
- break;

case SNDCTL_SEQ_PERCMODE:
/* what's this? */
return 0;
- break;

case SNDCTL_SYNTH_MEMAVL:
return memsize - awe_free_mem_ptr() * 2;
@@ -4314,7 +4312,8 @@
if (((cmd >> 8) & 0xff) != 'M')
return -EINVAL;

- level = *(int*)arg;
+ if (get_user(level, (int *)arg))
+ return -EFAULT;
level = ((level & 0xff) + (level >> 8)) / 2;
DEBUG(0,printk("AWEMix: cmd=%x val=%d\n", cmd & 0xff, level));

@@ -4370,7 +4369,9 @@
level = 0;
break;
}
- return *(int*)arg = level;
+ if (put_user(level, (int *)arg))
+ return -EFAULT;
+ return level;
}
#endif /* CONFIG_AWE32_MIXER */


2003-06-06 16:33:29

by Hollis Blanchard

[permalink] [raw]
Subject: Re: __user annotations

On Thursday, Jun 5, 2003, at 17:07 US/Central, Linus Torvalds wrote:
>
> On Thu, 5 Jun 2003, Hollis Blanchard wrote:
>>
>> Two ioctl functions in sound/oss/awe_wave.c were directly
>> dereferencing
>> a user-supplied pointer in a few places. Please apply.
>
> When you do patches like this, can you please add the "__user"
> annotations
> while you're at it?

I was hoping for a little more explanation on that before I used it...
for example, will the following code generate a warning? An error?

void func_b(void *b) { }

void func_a(__user void *a)
{
func_b(a);
}

How about the other way, passing a normal pointer to a function with
__user in its prototype?

I'm just worried that as soon as I use __user once, entire call chains
are going to start spewing warnings/errors.

> Also, if your mailer doesn't rape whitespace, I
> seriously prefer patches in-line in the email, so that I don't have to
> edit the email and can reply to it directly?

I was under the impression that text/plain attachments were ok with
you, but it looks like inline will work too.

--
Hollis Blanchard
IBM Linux Technology Center

2003-06-06 17:15:15

by Linus Torvalds

[permalink] [raw]
Subject: Re: __user annotations


On Fri, 6 Jun 2003, Hollis Blanchard wrote:
>
> I was hoping for a little more explanation on that before I used it...
> for example, will the following code generate a warning? An error?

A warning. But only if you have "check" installed, and do the kernel build
with "C=1". You can build the whole kernel that way, just do a

"make C=1 bzImage >& make-output"

and you'll still get a fully functional kernel as a result, but you'll
also get a whole lot of warnings for drivers etc that haven't been
annotated.

The nice thing about the annotations is that not only do they give proper
warnings for the whole call-chain if there is something strange, they
actually make the source code more readable. You no longer worry about
whether a pointer is a user pointer or a kernel pointer - it's obvious
from the sources.

> void func_b(void *b) { }
>
> void func_a(__user void *a)
> {
> func_b(a);
> }
>
> How about the other way, passing a normal pointer to a function with
> __user in its prototype?

Also a warning. For example, doing a

make C=1 sound/oss/awe_wave.o

results in this output:

CHECK sound/oss/awe_wave.c
warning: sound/oss/awe_wave.c:2049:21: incorrect type in argument 1 (different address spaces)
warning: sound/oss/awe_wave.c:2049:21: expected void [noderef] *to<asn:1>
warning: sound/oss/awe_wave.c:2049:21: got char *
warning: sound/oss/awe_wave.c:2855:50: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:2855:50: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:2855:50: got char const *addr
warning: sound/oss/awe_wave.c:3046:33: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3046:33: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3046:33: got char const *addr
warning: sound/oss/awe_wave.c:3155:32: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3155:32: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3155:32: got char const *addr
warning: sound/oss/awe_wave.c:3291:39: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3291:39: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3291:39: got char const *addr
warning: sound/oss/awe_wave.c:3338:36: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3338:36: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3338:36: got char const *addr
warning: sound/oss/awe_wave.c:3397:35: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3397:35: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3397:35: got char const *addr
warning: sound/oss/awe_wave.c:3454:35: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3454:35: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3454:35: got char const *addr
warning: sound/oss/awe_wave.c:3673:50: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:3673:50: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:3673:50: got char const *addr
warning: sound/oss/awe_wave.c:4964:55: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:4964:55: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:4964:55: got char const *addr
warning: sound/oss/awe_wave.c:5072:55: incorrect type in argument 2 (different address spaces)
warning: sound/oss/awe_wave.c:5072:55: expected void const [noderef] *from<asn:1>
warning: sound/oss/awe_wave.c:5072:55: got char const *addr
CC sound/oss/awe_wave.o

(Right now, "check" does NOT warn about dereferencing a __user pointer
directly, that will require it to walk the expression tree a second time
after evaluating the types, and I've been lazy. But the capability is
there, that's why __user pointers are marked [noderef].).

> I'm just worried that as soon as I use __user once, entire call chains
> are going to start spewing warnings/errors.

Absolutely. They largely already do - I cleaned up the code kernel files,
but I didn't have the energy to clean up drivers.

HOWEVER, usually it's very obvious to fix the whole chain, unless some
type is sometimes used for kernel addresses and sometimes for user
addresses (which networking does with iovec's, for example).

And "check" does give pretty good error messages, pointing out exactly
which argument it doesn't like, and why, and where the original
declaration was if there are declaration conflicts etc. That is, after
all, the whole point of "check".

You can get check from

bk://kernel.bkbits.net/torvalds/sparse

if you want to play with it.

> I was under the impression that text/plain attachments were ok with
> you, but it looks like inline will work too.

text/plain is _workable_ (so I did actually apply the patch), but inline
is preferred.

Linus

2003-06-07 00:21:56

by Paul Mackerras

[permalink] [raw]
Subject: Re: __user annotations

Linus Torvalds writes:

> You can get check from
>
> bk://kernel.bkbits.net/torvalds/sparse

Is that up to date? I cloned that repository and said "make" and got
heaps of compile errors. First there were a heap of warnings like
this:

symbol.h:73: warning: declaration does not declare anything

and then lots of errors like this:

parse.c:44: error: structure has no member named `ctype'

and indeed the structures defined in the headers don't seem to match
up with their uses in the .c files.

Paul.

2003-06-07 00:28:43

by Sam Ravnborg

[permalink] [raw]
Subject: Re: __user annotations

On Sat, Jun 07, 2003 at 10:32:04AM +1000, Paul Mackerras wrote:
> Linus Torvalds writes:
>
> > You can get check from
> >
> > bk://kernel.bkbits.net/torvalds/sparse
>
> Is that up to date? I cloned that repository and said "make" and got
> heaps of compile errors.

Cloned it today - compiled without a single warning.
(RH8.0, gcc 3.2)

> First there were a heap of warnings like
> this:
>
> symbol.h:73: warning: declaration does not declare anything
My version looks like this:
struct preprocessor_sym {
struct token *expansion;
struct token *arglist;
}; <= line 73

struct ctype_sym {

Looks like something wrong at your side...

Sam

2003-06-07 00:30:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: __user annotations


On Sat, 7 Jun 2003, Paul Mackerras wrote:
> Linus Torvalds writes:
>
> > You can get check from
> >
> > bk://kernel.bkbits.net/torvalds/sparse
>
> Is that up to date? I cloned that repository and said "make" and got
> heaps of compile errors. First there were a heap of warnings like
> this:

You need to have a modern compiler. The "heaps of errors" is what you get
if you use a stone-age compiler that doesn't support anonymous structure
and union members or other C99 features.

Gcc has supported them since some pre-3.x version (which is pretty late,
since they've been around in other compilers for much longer). They are a
great way to make readable data structures that have internal structure
_without_ having to have that structure show up unnecessarily in usage.

Linus

2003-06-07 00:42:36

by Paul Mackerras

[permalink] [raw]
Subject: Re: __user annotations

Sam Ravnborg writes:

> Cloned it today - compiled without a single warning.
> (RH8.0, gcc 3.2)

Hmmm, I am using gcc 3.3 (debian sid).

> My version looks like this:
> struct preprocessor_sym {
> struct token *expansion;
> struct token *arglist;
> }; <= line 73

So does mine.

> struct ctype_sym {
>
> Looks like something wrong at your side...

Well, does symbol.h have a `ctype' member in your copy? It doesn't in
mine. And line 44 of parse.c says:

sym->ctype.base_type = ctype->base_type;

which fails because sym is a struct symbol * and struct symbol doesn't
have a ctype member. And FWIW the compilation fails with gcc-3.2 too.

I have included the version of symbol.h that I have below. Could you
compare that against yours and/or do a pull and see if it still works
for you?

Paul.

#ifndef SYMBOL_H
#define SYMBOL_H
/*
* Basic symbol and namespace definitions.
*
* Copyright (C) 2003 Transmeta Corp.
*
* Licensed under the Open Software License version 1.1
*/

#include "token.h"

/*
* An identifier with semantic meaning is a "symbol".
*
* There's a 1:n relationship: each symbol is always
* associated with one identifier, while each identifier
* can have one or more semantic meanings due to C scope
* rules.
*
* The progression is symbol -> token -> identifier. The
* token contains the information on where the symbol was
* declared.
*/
enum namespace {
NS_NONE,
NS_PREPROCESSOR,
NS_TYPEDEF,
NS_STRUCT,
NS_ENUM,
NS_LABEL,
NS_SYMBOL,
NS_ITERATOR,
};

enum type {
SYM_BASETYPE,
SYM_NODE,
SYM_PTR,
SYM_FN,
SYM_ARRAY,
SYM_STRUCT,
SYM_UNION,
SYM_ENUM,
SYM_TYPEDEF,
SYM_TYPEOF,
SYM_MEMBER,
SYM_BITFIELD,
SYM_LABEL,
};

struct ctype {
unsigned long modifiers;
unsigned long alignment;
unsigned int contextmask, context, as;
struct symbol *base_type;
};

struct symbol {
enum namespace namespace:8;
enum type type:8;
struct position pos; /* Where this symbol was declared */
struct ident *ident; /* What identifier this symbol is associated with */
struct symbol *next_id; /* Next semantic symbol that shares this identifier */
struct symbol **id_list; /* Backpointer to symbol list head */
struct scope *scope;
struct symbol *same_symbol;
int (*evaluate)(struct expression *);

struct preprocessor_sym {
struct token *expansion;
struct token *arglist;
};

struct ctype_sym {
unsigned long offset;
unsigned int bit_size;
unsigned int bit_offset:8,
fieldwidth:8,
arg_count:10,
variadic:1,
used:1,
initialized:1;
int array_size;
struct ctype ctype;
struct symbol_list *arguments;
struct statement *stmt;
struct symbol_list *symbol_list;
struct expression *initializer;
long long value; /* Initial value */
};
void *aux; /* Auxiliary info, eg. backend information */
};

/* Modifiers */
#define MOD_AUTO 0x0001
#define MOD_REGISTER 0x0002
#define MOD_STATIC 0x0004
#define MOD_EXTERN 0x0008

#define MOD_STORAGE (MOD_AUTO | MOD_REGISTER | MOD_STATIC | MOD_EXTERN | MOD_INLINE | MOD_TOPLEVEL)

#define MOD_CONST 0x0010
#define MOD_VOLATILE 0x0020
#define MOD_SIGNED 0x0040
#define MOD_UNSIGNED 0x0080

#define MOD_CHAR 0x0100
#define MOD_SHORT 0x0200
#define MOD_LONG 0x0400
#define MOD_LONGLONG 0x0800

#define MOD_TYPEDEF 0x1000
#define MOD_STRUCTOF 0x2000
#define MOD_UNIONOF 0x4000
#define MOD_ENUMOF 0x8000

#define MOD_TYPEOF 0x10000
#define MOD_ATTRIBUTE 0x20000
#define MOD_INLINE 0x40000
#define MOD_ADDRESSABLE 0x80000

#define MOD_NOCAST 0x100000
#define MOD_NODEREF 0x200000
#define MOD_ACCESSED 0x400000
#define MOD_TOPLEVEL 0x800000 // scoping..

#define MOD_LABEL 0x1000000

/* Basic types */
extern struct symbol void_type,
int_type,
label_type,
fp_type,
vector_type,
bad_type;

/* C types */
extern struct symbol bool_ctype, void_ctype,
char_ctype, uchar_ctype,
short_ctype, ushort_ctype,
int_ctype, uint_ctype,
long_ctype, ulong_ctype,
llong_ctype, ullong_ctype,
float_ctype, double_ctype, ldouble_ctype,
string_ctype, ptr_ctype, label_type;


/* Basic identifiers */
extern struct ident sizeof_ident,
alignof_ident,
__alignof_ident,
__alignof___ident,
if_ident,
else_ident,
switch_ident,
case_ident,
default_ident,
break_ident,
continue_ident,
for_ident,
while_ident,
do_ident,
goto_ident,
return_ident;

extern struct ident __asm___ident,
__asm_ident,
asm_ident,
__volatile___ident,
__volatile_ident,
volatile_ident,
__attribute___ident,
__attribute_ident,
pragma_ident;

#define symbol_is_typename(sym) ((sym)->type == SYM_TYPE)

extern struct symbol_list *used_list;

extern void access_symbol(struct symbol *);


extern struct symbol *lookup_symbol(struct ident *, enum namespace);
extern void init_symbols(void);
extern struct symbol *alloc_symbol(struct position, int type);
extern void show_type(struct symbol *);
extern const char *modifier_string(unsigned long mod);
extern void show_symbol(struct symbol *);
extern void show_type_list(struct symbol *);
extern void show_symbol_list(struct symbol_list *, const char *);
extern void add_symbol(struct symbol_list **, struct symbol *);
extern void bind_symbol(struct symbol *, struct ident *, enum namespace);

extern struct symbol *examine_symbol_type(struct symbol *);
extern void examine_simple_symbol_type(struct symbol *);
extern const char *show_typename(struct symbol *sym);

extern void debug_symbol(struct symbol *);
extern void merge_type(struct symbol *sym, struct symbol *base_type);
extern void check_declaration(struct symbol *sym);

#endif /* SEMANTIC_H */

2003-06-07 00:53:41

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: __user annotations

Em Fri, Jun 06, 2003 at 05:43:58PM -0700, Linus Torvalds escreveu:
>
> On Sat, 7 Jun 2003, Paul Mackerras wrote:
> > Linus Torvalds writes:
> >
> > > You can get check from
> > >
> > > bk://kernel.bkbits.net/torvalds/sparse
> >
> > Is that up to date? I cloned that repository and said "make" and got
> > heaps of compile errors. First there were a heap of warnings like
> > this:
>
> You need to have a modern compiler. The "heaps of errors" is what you get
> if you use a stone-age compiler that doesn't support anonymous structure
> and union members or other C99 features.
>
> Gcc has supported them since some pre-3.x version (which is pretty late,
> since they've been around in other compilers for much longer). They are a
> great way to make readable data structures that have internal structure
> _without_ having to have that structure show up unnecessarily in usage.

In 3.3 this style is not accepted (haven't read the c99 draft to see if it is
OK)

struct foo {
struct bar {
int a, b;
};
}

But this one is:

struct foo {
struct /* bar */ {
int a, b;
}
}

Does anybody knows if gcc 3.3 behaviour is correct and the fact that gcc 3.2
accepts the former style was just a temporary bug?

- Arnaldo

2003-06-07 00:58:59

by Paul Mackerras

[permalink] [raw]
Subject: Re: __user annotations

Arnaldo Carvalho de Melo writes:

> In 3.3 this style is not accepted (haven't read the c99 draft to see if it is
> OK)

Ah, thank you, that explains it, since gcc-3.3 is now the default in
debian sid. In fact it does compile OK for me with gcc-3.2. (I
previously said it didn't but that was because I said `CC=gcc-3.2
make' not 'make CC=gcc-3.2'.)

Paul.

2003-06-07 12:32:49

by Ingo Oeser

[permalink] [raw]
Subject: Re: __user annotations

Hi Linus,

On Fri, Jun 06, 2003 at 10:28:22AM -0700, Linus Torvalds wrote:
> HOWEVER, usually it's very obvious to fix the whole chain, unless some
> type is sometimes used for kernel addresses and sometimes for user
> addresses (which networking does with iovec's, for example).

Then it's not very useful for me. I usally define the ABI between
user space and kernel space trough IOCTL like that:

/* These structures are usally bigger and nested deeper */
struct in_foo_ioctl_name {
int bla;
}

struct out_foo_ioctl_name {
char blubb;
}

union foo_ioctl_name {
struct in_foo_ioctl_name in;
struct out_foo_ioctl_name out;
}

#define SUBSYS_IOCTL 0xee

#define SUBSYS_FOO _IOWR(SUBSYS_IOCTL, 0x1, union foo_ioctl_name)

Now I do in principle

union foo_ioctl_name k, *u = (union foo_ioctl_name *)arg;

if (copy_from_user(&k.in, &u, sizeof(k.in)) return -EFAULT;
if (handle_foo(&k)) return -EINVAL;
if (copy_to_user(&u, &k.out, sizeof(k.out)) return -EFAULT;

which I consider very clean (our project provides both: The
only ABI provider and the only ABI user) and works from 2.0
trough 2.5 so far.

This will NOT work anymore with __user annotations, right?

That's a big pity. How do I workaround this? I would like to
help resolving this issues, if you are interested.

Regards

Ingo Oeser

2003-06-07 16:12:22

by Linus Torvalds

[permalink] [raw]
Subject: Re: __user annotations


On Sat, 7 Jun 2003, Ingo Oeser wrote:
>
> That's a big pity. How do I workaround this? I would like to
> help resolving this issues, if you are interested.

The solution to these things is to _always_ have a separate type for the
user thing than for the kernel thing.

In practice, a lot of code has ended up doing that _anyway_, since the
kernel usually wants to have a few extra fields for its internal use. The
classic unix example of this, of course, is "struct stat" vs "struct
inode".

But if your structures are 100% the same, then you can just share them.
There's nothing wrong with having

struct ioctl_arg {
int value;
int another_value;
..
};

and then in your ioctl routines you have

int my_ioctl_routine(struct ioctl_arg __user *ptr)
{
struct ioctl_arg arg;

if (copy_from_user(&arg, ptr, sizeof(*ptr))
return -EFAULT;
...
}

and that's fine.

You can even have user pointers _inside_ the structure: because "sparse"
really understands C types at a very fundamental level (like a compiler
would, not like some simpler source scanner), you can have

struct ioctl_arg {
int value;
void __user *buf;
};

and do

int my_ioctl_routine(struct ioctl_arg __user *ptr)
{
struct ioctl_arg arg;
char buffer[10];

if (copy_from_user(&arg, ptr, sizeof(*ptr))
return -EFAULT;

and sparse will be aware of the fact that "arg.buf" is a user pointer, and
it will properly warn if you pass it to a function that expects a kernel
pointer (or assign it to a normal non-user pointer thing).

Linus

2003-06-07 16:29:57

by Sam Ravnborg

[permalink] [raw]
Subject: Re: __user annotations

On Sat, Jun 07, 2003 at 09:25:43AM -0700, Linus Torvalds wrote:
>
> You can even have user pointers _inside_ the structure: because "sparse"

Since we start to know your checker by the name sparse, why not
call the default executable sparse?

Sam

===== Makefile 1.21 vs edited =====
--- 1.21/Makefile Tue Jun 3 03:15:27 2003
+++ edited/Makefile Sat Jun 7 18:41:15 2003
@@ -2,7 +2,7 @@
CFLAGS=-g -Wall
AR=ar

-PROGRAMS=test-lexing test-parsing obfuscate check
+PROGRAMS=test-lexing test-parsing obfuscate sparse

LIB_H= token.h parse.h lib.h symbol.h scope.h expression.h target.h

@@ -22,7 +22,7 @@
obfuscate: obfuscate.o $(LIB_FILE)
gcc -o $@ $< $(LIB_FILE)

-check: check.o $(LIB_FILE)
+sparse: check.o $(LIB_FILE)
gcc -o $@ $< $(LIB_FILE)

$(LIB_FILE): $(LIB_OBJS)

2003-06-07 16:34:52

by Sam Ravnborg

[permalink] [raw]
Subject: Re: __user annotations

On Sat, Jun 07, 2003 at 06:43:30PM +0200, Sam Ravnborg wrote:
> Since we start to know your checker by the name sparse, why not
> call the default executable sparse?

And then we can update the top-level Makefile in the kernel.

Sam

Rename Linus' checker tool to sparse, and avoid the hardcoded path.

===== Makefile 1.410 vs edited =====
--- 1.410/Makefile Tue Jun 3 23:27:14 2003
+++ edited/Makefile Sat Jun 7 18:44:22 2003
@@ -204,7 +204,7 @@
DEPMOD = /sbin/depmod
KALLSYMS = scripts/kallsyms
PERL = perl
-CHECK = /home/torvalds/parser/check
+CHECK = sparse
MODFLAGS = -DMODULE
CFLAGS_MODULE = $(MODFLAGS)
AFLAGS_MODULE = $(MODFLAGS)

2003-06-07 16:36:16

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: __user annotations

On Fri, Jun 06, 2003 at 05:43:58PM -0700, Linus Torvalds wrote:
>
> On Sat, 7 Jun 2003, Paul Mackerras wrote:
> > Linus Torvalds writes:
> >
> > > You can get check from
> > >
> > > bk://kernel.bkbits.net/torvalds/sparse
> >
> > Is that up to date? I cloned that repository and said "make" and got
> > heaps of compile errors. First there were a heap of warnings like
> > this:
>
> You need to have a modern compiler. The "heaps of errors" is what you get
> if you use a stone-age compiler that doesn't support anonymous structure
> and union members or other C99 features.
>
> Gcc has supported them since some pre-3.x version (which is pretty late,
> since they've been around in other compilers for much longer). They are a
> great way to make readable data structures that have internal structure
> _without_ having to have that structure show up unnecessarily in usage.

Actually, I believe they are an extension, which GCC honors. Unnamed
structures in standard C99 are actually declaring an unnamed type, not
an unnamed member. Try it:

struct {
int a;
union {
int b;
float c;
};
int d;
} foo;

int bar()
{
return foo.a + foo.d + foo.b;
}


With -std=c99, the reference to foo.b is an error; with -std=gnu99 or
-std=gnu89, it is accepted.


I don't know why they were getting rejected for Paul, though. Did you
have GNU set to -ansi mode?

--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer

2003-06-08 02:07:08

by Paul Mackerras

[permalink] [raw]
Subject: Re: __user annotations

Daniel Jacobowitz writes:

> I don't know why they were getting rejected for Paul, though. Did you
> have GNU set to -ansi mode?

I was using gcc-3.3. I tried both -std=c99 and -std=gnu99, and gcc-3.3
didn't like the code either way, nor did it like it with no -std
option. Gcc-3.2 compiles it perfectly happily though, so that is what
I am doing for now.

(I did have to hack some of the pre-definitions in check.c to make it
useful on PPC, e.g., I changed it to pre-define __ppc__ instead of
__i386__.)

Paul.