Fun with Linux 2.6.21-rc1

Fun with Linux 2.6.21-rc1

[Pavel Roskin]

Hello!

The current sparse reports many errors with Linux 2.6.21-rc1.  There are
screenfuls of errors for every file.  I have reduced one of them to this
simple file:

struct st {
	char c;
} __attribute__ ((aligned(2)));
struct st s1;
struct st s2;

$ sparse test.c 
test.c:5:18: error: Expected ( after asm
test.c:5:18: error: got s2
test.c:5:20: error: Expected ) after asm
test.c:5:20: error: got ;
builtin:0:0: error: expected declaration

The strange thing is that there is no "asm" here at all.

-- 
Regards,
Pavel Roskin

Re: Fun with Linux 2.6.21-rc1

[Pavel Roskin]

On Fri, 2007-02-23 at 12:25 -0500, Pavel Roskin wrote:
> Hello!
> 
> The current sparse reports many errors with Linux 2.6.21-rc1.  There are
> screenfuls of errors for every file.  I have reduced one of them to this
> simple file:
> 
> struct st {
> 	char c;
> } __attribute__ ((aligned(2)));
> struct st s1;
> struct st s2;

P.S. The breakage has nothing to do to Linux changes.  It was caused by
the patch called "Make the ptrlist using the sparse allocator".  Undoing
the patch fixes the problem.

-- 
Regards,
Pavel Roskin

[PATCH] Re: Fun with Linux 2.6.21-rc1

[Christopher Li]

On Fri, Feb 23, 2007 at 02:59:26PM -0500, Pavel Roskin wrote:
> P.S. The breakage has nothing to do to Linux changes.  It was caused by
> the patch called "Make the ptrlist using the sparse allocator".  Undoing
> the patch fixes the problem.
>

No, that was not it. The bug does not trigger if I remove "-O" or remove
the recent struct attribute patch.

You make me guilt for 5 mins. But the real bugs is match_idents did not
end with NULL.

This patch should fix it:

Chris

Fix a bug that match_idents forget to end with NULL

Pavel Roskin manage to hit this bug with

struct st {
       char c;
} __attribute__ ((aligned(2)));
struct st s1;
struct st s2;

Signed-Off-By: Christopher Li <sparse@chrisli.org>

Index: sparse/parse.c
===================================================================
--- sparse.orig/parse.c	2007-02-23 14:33:28.000000000 -0800
+++ sparse/parse.c	2007-02-23 14:36:16.000000000 -0800
@@ -861,7 +861,7 @@ static struct token *handle_attributes(s
 			apply_ctype(token->pos, &thistype, ctype);
 			continue;
 		}
-		if (match_idents(token, &asm_ident, &__asm_ident, &__asm___ident)) {
+		if (match_idents(token, &asm_ident, &__asm_ident, &__asm___ident, NULL)) {
 			struct expression *expr;
 			token = expect(token->next, '(', "after asm");
 			token = parse_expression(token->next, &expr);

Re: [PATCH] Re: Fun with Linux 2.6.21-rc1

[Pavel Roskin]

On Fri, 2007-02-23 at 14:30 -0800, Christopher Li wrote:
> On Fri, Feb 23, 2007 at 02:59:26PM -0500, Pavel Roskin wrote:
> > P.S. The breakage has nothing to do to Linux changes.  It was caused by
> > the patch called "Make the ptrlist using the sparse allocator".  Undoing
> > the patch fixes the problem.
> >
> 
> No, that was not it. The bug does not trigger if I remove "-O" or remove
> the recent struct attribute patch.
> 
> You make me guilt for 5 mins. But the real bugs is match_idents did not
> end with NULL.

I understand your patch triggered a bug that was sitting there for more
than two years.  That's quite rare.  Sorry for assuming your guilt.

> This patch should fix it:

Yes, it's working.  Thank you!

-- 
Regards,
Pavel Roskin

exposing __attribute__ portability macros in lib.h; any objections?

[Josh Triplett]

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

Christopher Li wrote:
> Fix a bug that match_idents forget to end with NULL
[...]
> Signed-Off-By: Christopher Li <sparse@chrisli.org>
[...]
> -		if (match_idents(token, &asm_ident, &__asm_ident, &__asm___ident)) {
> +		if (match_idents(token, &asm_ident, &__asm_ident, &__asm___ident, NULL)) {

Applied.

I'd also like to apply a patch to use __attribute__((__sentinel__)) on
match_idents, which would have caught this problem.  In order to do so
portably, I plan to add a portability macro SENTINEL_ATTR, like the
FORMAT_ATTR macro currently in lib.h.  However, lib.h currently defines
FORMAT_ATTR, uses it for a few prototypes, and then undefines it.  Any
objections to defining SENTINEL_ATTR and leaving it defined (and probably
doing the same for FORMAT_ATTR)?  I don't think those would pollute the
namespace any more than lib.h already does with position, verbose, info, and
similar.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: exposing __attribute__ portability macros in lib.h; any objections?

[Christopher Li]

On Sun, Feb 25, 2007 at 02:45:29PM -0800, Josh Triplett wrote:
> Applied.

Thanks!

> 
> I'd also like to apply a patch to use __attribute__((__sentinel__)) on
> match_idents, which would have caught this problem.  In order to do so
> portably, I plan to add a portability macro SENTINEL_ATTR, like the
> FORMAT_ATTR macro currently in lib.h.  However, lib.h currently defines
> FORMAT_ATTR, uses it for a few prototypes, and then undefines it.  Any
> objections to defining SENTINEL_ATTR and leaving it defined (and probably
> doing the same for FORMAT_ATTR)?  I don't think those would pollute the
> namespace any more than lib.h already does with position, verbose, info, and
> similar.

No objection for __sentinel__ per se. In the long run, I would like to
get ride of match_idents completely. I want to define more parser
operation function for pre-defined keywords. So it is driven by the
symbol table rather than comparing the keyword one by one. It is a
big change though.

Chris