diff options
| author | Al Viro <viro@ZenIV.linux.org.uk> | 2009-02-02 07:30:19 +0000 |
|---|---|---|
| committer | Christopher Li <sparse@chrisli.org> | 2009-07-17 23:06:22 +0000 |
| commit | bbba9f8a25b5b8b72735fa41c4509965b1ccb126 (patch) | |
| tree | 61aaa8b9f552eb85d3ea2bdee9943d8489e62fc5 | |
| parent | ac66df10a8db613c4952813d9e4006387e2c1a99 (diff) | |
| download | sparse-dev-bbba9f8a25b5b8b72735fa41c4509965b1ccb126.tar.gz | |
fun with declarations and definitions
There are several interesting problems caused by the fact that
we create a separate symbol for each declaration of given function.
1)
static inline int f(void);
static int g(void)
{
return f();
}
static inline int f(void)
{
return 0;
}
gives an error, since the instance of f in g is not associated with anything
useful. Needless to say, this is a perfectly valid C. Moreover,
static inline int f(void)
{
return 0;
}
static inline int f(void);
static int g(void)
{
return f();
}
will step on the same thing. Currently we get the former case all over the
place in the kernel, thanks to the way DEFINE_SYSCALLx() is done.
I have a kinda-sorta fix for that (basically, add a reference to external
definition to struct symbol and update it correctly - it's not hard).
However, that doesn't cover *another* weirdness in the same area -
gccisms around extern inline. There we can have inline and external
definitions in the same translation unit (and they can be different,
to make the things even more interesting). Anyway, that's a separate
story - as it is, we don't even have a way to tell 'extern inline ...'
from 'inline ...'
2) More fun in the same area: checks for SYM_FN in external_declaration()
do not take into account the possibility of
void f(int);
typeof(f) g;
Ergo, we get linkage-less function declarations. Fun, innit? No patch.
3) Better yet, sparse does _NOT_ reject
typeof(f) g
{
...
}
which is obviously a Bloody Bad Idea(tm) (just think what that does to
argument list). Similar crap is triggerable with typedef. IMO, we really
ought to reject _that_ - not only 6.9.1(2) explicitly requires that, but
there's no even remotely sane way to deal with arguments.
4)
static void f(void);
...
void f(void);
triggers "warning: symbol 'f' was not declared. Should it be static?"
which is at least very confusing - it *is* declared and it *is* static.
IOW, we do not collect the linkage information sanely. (2) will make
fixing that one very interesting.
Anyway, proposed patch for (1) follows:
Subject: [PATCH] Handle mix of declarations and definitions
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
| -rw-r--r-- | evaluate.c | 7 | ||||
| -rw-r--r-- | parse.c | 15 | ||||
| -rw-r--r-- | symbol.h | 1 | ||||
| -rw-r--r-- | validation/definitions.c | 12 |
4 files changed, 35 insertions, 0 deletions
@@ -2771,6 +2771,10 @@ static int evaluate_symbol_call(struct expression *expr) if (ctype->ctype.modifiers & MOD_INLINE) { int ret; struct symbol *curr = current_fn; + + if (ctype->definition) + ctype = ctype->definition; + current_fn = ctype->ctype.base_type; ret = inline_function(expr, ctype); @@ -3076,6 +3080,9 @@ static struct symbol *evaluate_symbol(struct symbol *sym) if (base_type->type == SYM_FN) { struct symbol *curr = current_fn; + if (sym->definition && sym->definition != sym) + return evaluate_symbol(sym->definition); + current_fn = base_type; examine_fn_arguments(base_type); @@ -1991,6 +1991,7 @@ static struct token *parse_function_body(struct token *token, struct symbol *dec struct symbol_list **old_symbol_list; struct symbol *base_type = decl->ctype.base_type; struct statement *stmt, **p; + struct symbol *prev; struct symbol *arg; old_symbol_list = function_symbol_list; @@ -2024,6 +2025,18 @@ static struct token *parse_function_body(struct token *token, struct symbol *dec if (!(decl->ctype.modifiers & MOD_INLINE)) add_symbol(list, decl); check_declaration(decl); + decl->definition = decl; + prev = decl->same_symbol; + if (prev && prev->definition) { + warning(decl->pos, "multiple definitions for function '%s'", + show_ident(decl->ident)); + info(prev->definition->pos, " the previous one is here"); + } else { + while (prev) { + prev->definition = decl; + prev = prev->same_symbol; + } + } function_symbol_list = old_symbol_list; if (function_computed_goto_list) { if (!function_computed_target_list) @@ -2195,6 +2208,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis } } check_declaration(decl); + if (decl->same_symbol) + decl->definition = decl->same_symbol->definition; if (!match_op(token, ',')) break; @@ -154,6 +154,7 @@ struct symbol { struct expression *initializer; struct entrypoint *ep; long long value; /* Initial value */ + struct symbol *definition; }; }; union /* backend */ { diff --git a/validation/definitions.c b/validation/definitions.c new file mode 100644 index 00000000..fce7393e --- /dev/null +++ b/validation/definitions.c @@ -0,0 +1,12 @@ +static inline int f(void); +static int g(void) +{ + return f(); +} +static inline int f(void) +{ + return 0; +} +/* + * check-name: finding definitions + */ |