Odd sparse behaviour

Odd sparse behaviour

[David Given]

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

I've found a couple of places where sparse behaves rather oddly.

Firstly, if you declare something static, and then try to define it
later, sparse appears to get confused:

---snip---
static int i;
int i = 0;
---snip---

$ ./test-parsing test.c
test.c:2:5: warning: symbol 'i' was not declared. Should it be static?

.align 4
int static [signed] [toplevel] i,
.align 4
int [signed] [addressable] [toplevel] i =
	movi.32		v1,$0

Enumeration of the list of defined symbols shows two different symbols
with the name 'i'. This isn't entirely obvious when using the test tools
as, of course, two strings with the same value look the same! This is
causing me issues with forward declarations of static functions:

static void foo();
{ ... foo(); ... }  // calls static foo

foo() {}            // defines extern foo
{ ... foo(); ... }  // calls extern foo

This then leads to link errors as static foo hasn't been found.


The second issue is with the following piece of C99 code:

---snip---
extern void nop(void)
void bar(void)
{ for (int i=0; i<10; i++) nop(); }
---snip---

$ ./test-linearise test.c
test.c:3:6: warning: symbol 'bar' was not declared. Should it be static?
bar:
.L0xb7d7600c:
	<entry-point>
	br          .L0xb7d76038

.L0xb7d76038:
	call        nop
	br          .L0xb7d76038

The for loop seems to silently turn into an infinite loop. Which did
cause one of my benchmarks to, um, produce rather poor results...

(You may be interested to know that my compiler is now producing
working, running code for non-trivial apps. Big chunks of it do need
throwing away and rewriting, but it's actually *working*!)

-- 
┌─── ｄｇ＠ｃｏｗｌａｒｋ．ｃｏｍ ───── http://www.cowlark.com ─────
│ "I have always wished for my computer to be as easy to use as my
│ telephone; my wish has come true because I can no longer figure out
│ how to use my telephone." --- Bjarne Stroustrup


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

Re: Odd sparse behaviour

[Josh Triplett]

On Sun, 2008-07-06 at 22:33 +0100, David Given wrote:
> I've found a couple of places where sparse behaves rather oddly.
> 
> Firstly, if you declare something static, and then try to define it
> later, sparse appears to get confused:
> 
> ---snip---
> static int i;
> int i = 0;
> ---snip---
> 
> $ ./test-parsing test.c
> test.c:2:5: warning: symbol 'i' was not declared. Should it be static?
> 
> .align 4
> int static [signed] [toplevel] i,
> .align 4
> int [signed] [addressable] [toplevel] i =
> 	movi.32		v1,$0
> 
> Enumeration of the list of defined symbols shows two different symbols
> with the name 'i'. This isn't entirely obvious when using the test tools
> as, of course, two strings with the same value look the same! This is
> causing me issues with forward declarations of static functions:
> 
> static void foo();
> { ... foo(); ... }  // calls static foo
> 
> foo() {}            // defines extern foo
> { ... foo(); ... }  // calls extern foo
> 
> This then leads to link errors as static foo hasn't been found.

I've observed this problem before.  Alexey Zaytsev reported this as
well.  I think he may have a fix in the works.  Alexey?

> The second issue is with the following piece of C99 code:
> 
> ---snip---
> extern void nop(void)
> void bar(void)
> { for (int i=0; i<10; i++) nop(); }
> ---snip---
> 
> $ ./test-linearise test.c
> test.c:3:6: warning: symbol 'bar' was not declared. Should it be static?
> bar:
> .L0xb7d7600c:
> 	<entry-point>
> 	br          .L0xb7d76038
> 
> .L0xb7d76038:
> 	call        nop
> 	br          .L0xb7d76038
> 
> The for loop seems to silently turn into an infinite loop. Which did
> cause one of my benchmarks to, um, produce rather poor results...

No kidding.  An interesting bug.  Does this go away if you declare "int
i" at the top of the function?

> (You may be interested to know that my compiler is now producing
> working, running code for non-trivial apps. Big chunks of it do need
> throwing away and rewriting, but it's actually *working*!)

Awesome!

- Josh Triplett

Re: Odd sparse behaviour

[Alexey Zaytsev]

On Mon, Jul 7, 2008 at 9:48 PM, Josh Triplett <josht@linux.vnet.ibm.com> wrote:
> On Sun, 2008-07-06 at 22:33 +0100, David Given wrote:
>> I've found a couple of places where sparse behaves rather oddly.
>>
>> Firstly, if you declare something static, and then try to define it
>> later, sparse appears to get confused:
>>
>> ---snip---
>> static int i;
>> int i = 0;
>> ---snip---
>>
>> $ ./test-parsing test.c
>> test.c:2:5: warning: symbol 'i' was not declared. Should it be static?
>>
>> .align 4
>> int static [signed] [toplevel] i,
>> .align 4
>> int [signed] [addressable] [toplevel] i =
>>       movi.32         v1,$0
>>
>> Enumeration of the list of defined symbols shows two different symbols
>> with the name 'i'. This isn't entirely obvious when using the test tools
>> as, of course, two strings with the same value look the same! This is
>> causing me issues with forward declarations of static functions:
>>
>> static void foo();
>> { ... foo(); ... }  // calls static foo
>>
>> foo() {}            // defines extern foo
>> { ... foo(); ... }  // calls extern foo
>>
>> This then leads to link errors as static foo hasn't been found.
>
> I've observed this problem before.  Alexey Zaytsev reported this as
> well.  I think he may have a fix in the works.  Alexey?

I did some research, but no working patch so far. If you'd like to
look at it yourself, I'd suggest lookign at parse.c:external_declaration().
Right now it reads:

        /* Parse declaration-specifiers, if any */
        token = declaration_specifiers(token, &ctype, 0);
        decl = alloc_symbol(token->pos, SYM_NODE);
        decl->ctype = ctype;
        token = declarator(token, decl, &ident);
        apply_modifiers(token->pos, &decl->ctype);

I think that after calling declaration_specifiers(), we should
check if token_type(token) == TOKEN_IDENT and lookup
the symbol in the relevant namespaces. If it is found, we shoud
check if the types and produce soe warnings. And reuse the
symbol, if the types match.

Josh, does this make any sense?

>
>> The second issue is with the following piece of C99 code:
>>
>> ---snip---
>> extern void nop(void)
>> void bar(void)
>> { for (int i=0; i<10; i++) nop(); }
>> ---snip---
>>
>> $ ./test-linearise test.c
>> test.c:3:6: warning: symbol 'bar' was not declared. Should it be static?
>> bar:
>> .L0xb7d7600c:
>>       <entry-point>
>>       br          .L0xb7d76038
>>
>> .L0xb7d76038:
>>       call        nop
>>       br          .L0xb7d76038
>>
>> The for loop seems to silently turn into an infinite loop. Which did
>> cause one of my benchmarks to, um, produce rather poor results...
>
> No kidding.  An interesting bug.  Does this go away if you declare "int
> i" at the top of the function?
>
>> (You may be interested to know that my compiler is now producing
>> working, running code for non-trivial apps. Big chunks of it do need
>> throwing away and rewriting, but it's actually *working*!)
>
> Awesome!
>
> - Josh Triplett
>
>
>

Re: Odd sparse behaviour

[Christopher Li]

On Mon, Jul 7, 2008 at 11:09 AM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:

>
>        /* Parse declaration-specifiers, if any */
>        token = declaration_specifiers(token, &ctype, 0);
>        decl = alloc_symbol(token->pos, SYM_NODE);
>        decl->ctype = ctype;
>        token = declarator(token, decl, &ident);
>        apply_modifiers(token->pos, &decl->ctype);
>
> I think that after calling declaration_specifiers(), we should
> check if token_type(token) == TOKEN_IDENT and lookup
> the symbol in the relevant namespaces. If it is found, we shoud
> check if the types and produce soe warnings. And reuse the
> symbol, if the types match.

This is a old bug of sparse and unfortunately very nasty to fix.

I have considered some thing similar to your approach before.
Yours is too simple, it can't handle node is a pointer for example.
But even if we change direct_declarator, there is other problem
with this approach. The type  evaluate happens _after_ the
parsing stage. So at the parsing state it is very hard to answer
the question "if the types match". No to mention "reuse the symbol"
does break a lot of the assumption of the parsing code which
assume symbol node is new. It just blindly assign ctype members
rather than merging it.

On the other hand, breaking that assumption might be a good thing
for other reasons. One of the thing I really want to do is make the
ctype have some optional attribute fields. e.g. no return functions.
We are running out of bits in the ctype attribute. Keep on adding
member to ctype does not scale because it is used every where.

Another approach to this problem is let parsing code create a new node.
Let check_duplicates merge the symbol if the type matches.
Then evaluate_symbol needs to return the merged result symbol
as well. The caller need to either place the current one with
the new symbol or delete the original one from the list.
Replacing the original symbol part seems ugly. On the bright side
there is not too many place needs it. It will have a smaller patch
than the first approach.

I like to heard if any one else have better suggestion as well.

Chris