| Message ID | 20090206053655.GS28946@ZenIV.linux.org.uk (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On Thu, Feb 5, 2009 at 9:36 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Thu, Feb 05, 2009 at 09:19:21PM +0000, Al Viro wrote: >> IOW, direct_declarator() (which doubles for direct-abstract-declarator) should >> have more than one-bit indication of which case we've got. Right now it's >> done by "have we passed a non-NULL ident ** to store the identifier being >> declared"; that's not enough. What we need is explicit 'is that a part of >> parameter declaration' flag; then the rule turns into >> if (p && *p) >> fn = 1; /* we'd seen identifier already, can't be nested */ >> else if match_op(next, ')') >> fn = 1; /* empty list can't be direct-declarator or >> * direct-abstract-declarator */ >> else >> fn = (in_parameter && lookup_type(next)); > > Umm... It's a bit more subtle (p goes NULL after the nested one is > handled), so we need to keep track of "don't allow nested declarator from > that point on" explicitly. Patch follows: > > Subject: [PATCH] Handle nested declarators vs. parameter lists correctly > > Seeing a typedef name after ( means that we have a parameter-type-list > only if we are parsing a parameter declaration or a typename; otherwise > it might very well be a redeclaration (e.g. int (T); when T had been a > typedef in outer scope). The patch looks great. Applied. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 08, 2009 at 11:52:23PM -0800, Christopher Li wrote: > > Subject: [PATCH] Handle nested declarators vs. parameter lists correctly > > > > Seeing a typedef name after ( means that we have a parameter-type-list > > only if we are parsing a parameter declaration or a typename; otherwise > > it might very well be a redeclaration (e.g. int (T); when T had been a > > typedef in outer scope). > > The patch looks great. Applied. Still leaves a couple of uglies, BTW. One (easily fixed): int [2]*p[3]; is actually (still) accepted. Nevermind, we just need to set dont_nest in a few more places (and no, it's not a regression; moreover, similar crap works with functions instead of arrays). Nastier one: #define A __attribute__((address_space(1))) void (*f)(A int *x, A int *y) = (void *)0; void g(int A *x) { f(x, x); } now try test-parsing on it. You'll see that f gets type void (A *)(int *, int A *); instead of expected void (*)(int A *, int A *); Reason: cretinous gcc grammar is fscked in head. There's a very, _very_ good reason why C doesn't allow e.g. int (const x); Consider seeing void f(int (const What would that "(const" be? Beginning of parameter list in the first parameter of f or a beginning of nested declarator in the same? In C it's the former and we can tell that immediately. I really don't want to get into the reasons why gcc decided to allow that with s/const/__attribute__((....))/ - it's too long a flame, but they did and so we have to do lookahead from hell in parser. Namely, eat all attributes first and only then decide whether this ( starts a nested declarator or a parameter list. ... except that we do it wrong and these attributes land on what will by SYM_FN and not its first parameter. BTW, the longer I'm looking at that, the less I like the original decision to use __attribute__ for our extensions. Or continued use of __attribute__ by gcc folks, while we are at it. One lovely example: T __attribute__((whatever)) *p; is NOT pointer-to-whatever-T; it's whatever-pointer-to-T. Which is clearly not what we want for __user et.al. The only way to get it applied *before* derivation is, BTW, T (__attribute__((whatever)) *p); (and yes, that's where this crap in syntax has come from). Mind you, T * __attribute__((whatever)) *p; *is* pointer-to-whatever-pointer-to-T, which makes the situation even more inconsistent. While we are at it, our handling of __attribute__((mode(...))) is wrong - not only __attribute__((mode(HI)) long x; is fine, it's actually s16. Moreover, int __attribute__((mode(HI)) *x; is only going to work on 16bit boxen, since it's "16bit pointer to int". 64/32bit analogs of that construct *are* used in the wild on some weird targets. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/parse.c b/parse.c index 7e3191a..73e7b65 100644 --- a/parse.c +++ b/parse.c @@ -1227,7 +1227,7 @@ static struct token *abstract_array_declarator(struct token *token, struct symbo } static struct token *parameter_type_list(struct token *, struct symbol *, struct ident **p); -static struct token *declarator(struct token *token, struct symbol *sym, struct ident **p); +static struct token *declarator(struct token *token, struct symbol *sym, struct ident **p, int); static struct token *handle_attributes(struct token *token, struct ctype *ctype, unsigned int keywords) { @@ -1247,13 +1247,15 @@ static struct token *handle_attributes(struct token *token, struct ctype *ctype, return token; } -static struct token *direct_declarator(struct token *token, struct symbol *decl, struct ident **p) +static struct token *direct_declarator(struct token *token, struct symbol *decl, struct ident **p, int prefer_abstract) { struct ctype *ctype = &decl->ctype; + int dont_nest = 0; if (p && token_type(token) == TOKEN_IDENT) { *p = token->ident; token = token->next; + dont_nest = 1; } for (;;) { @@ -1275,14 +1277,16 @@ static struct token *direct_declarator(struct token *token, struct symbol *decl, int fn; next = handle_attributes(next, ctype, KW_ATTRIBUTE); - fn = (p && *p) || match_op(next, ')') || lookup_type(next); + fn = dont_nest || match_op(next, ')') || + (prefer_abstract && lookup_type(next)); if (!fn) { struct symbol *base_type = ctype->base_type; - token = declarator(next, decl, p); + token = declarator(next, decl, p, prefer_abstract); token = expect(token, ')', "in nested declarator"); while (ctype->base_type != base_type) ctype = &ctype->base_type->ctype; + dont_nest = 1; p = NULL; continue; } @@ -1336,10 +1340,10 @@ static struct token *pointer(struct token *token, struct ctype *ctype) return token; } -static struct token *declarator(struct token *token, struct symbol *sym, struct ident **p) +static struct token *declarator(struct token *token, struct symbol *sym, struct ident **p, int prefer_abstract) { token = pointer(token, &sym->ctype); - return direct_declarator(token, sym, p); + return direct_declarator(token, sym, p, prefer_abstract); } static struct token *handle_bitfield(struct token *token, struct symbol *decl) @@ -1399,7 +1403,7 @@ static struct token *declaration_list(struct token *token, struct symbol_list ** struct ident *ident = NULL; struct symbol *decl = alloc_symbol(token->pos, SYM_NODE); decl->ctype = ctype; - token = declarator(token, decl, &ident); + token = declarator(token, decl, &ident, 0); decl->ident = ident; if (match_op(token, ':')) { token = handle_bitfield(token, decl); @@ -1439,7 +1443,7 @@ static struct token *parameter_declaration(struct token *token, struct symbol ** sym = alloc_symbol(token->pos, SYM_NODE); sym->ctype = ctype; *tree = sym; - token = declarator(token, sym, &ident); + token = declarator(token, sym, &ident, 1); sym->ident = ident; apply_modifiers(token->pos, &sym->ctype); sym->endpos = token->pos; @@ -1451,7 +1455,7 @@ struct token *typename(struct token *token, struct symbol **p, int mod) struct symbol *sym = alloc_symbol(token->pos, SYM_NODE); *p = sym; token = declaration_specifiers(token, &sym->ctype, 0); - token = declarator(token, sym, NULL); + token = declarator(token, sym, NULL, 1); apply_modifiers(token->pos, &sym->ctype); if (sym->ctype.modifiers & MOD_STORAGE & ~mod) warning(sym->pos, "storage class in typename (%s)", @@ -2261,7 +2265,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis token = declaration_specifiers(token, &ctype, 0); decl = alloc_symbol(token->pos, SYM_NODE); decl->ctype = ctype; - token = declarator(token, decl, &ident); + token = declarator(token, decl, &ident, 0); apply_modifiers(token->pos, &decl->ctype); decl->endpos = token->pos; @@ -2333,7 +2337,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis decl = alloc_symbol(token->pos, SYM_NODE); decl->ctype = ctype; token = declaration_specifiers(token, &decl->ctype, 1); - token = declarator(token, decl, &ident); + token = declarator(token, decl, &ident, 0); apply_modifiers(token->pos, &decl->ctype); decl->endpos = token->pos; if (!ident) { diff --git a/validation/nested-declarator.c b/validation/nested-declarator.c new file mode 100644 index 0000000..24ed833 --- /dev/null +++ b/validation/nested-declarator.c @@ -0,0 +1,11 @@ +typedef int T; +extern void f(int); +static void g(int x) +{ + int (T); + T = x; + f(T); +} +/* + * check-name: nested declarator vs. parameters + */