Ignore the cdecl and stdcall attributes for now.

Ignore the cdecl and stdcall attributes for now.

[Michael Stefaniuc]

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

Wine uses the __stdcall__ attribute extensively. The effects of the
patch on a sparse run on the Wine code are:
- Removes 143000 "attribute '__stdcall__': unknown attribute" errors.
- Removes 116 "attribute '__cdecl__': unknown attribute" errors.
- Reduces the amount of "error: too many errors" from 1992 to 1459.

Signed-off-by: Michael Stefaniuc <mstefani@redhat.com>
---
 parse.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/parse.c b/parse.c
index a1410c4..eb86bf6 100644
--- a/parse.c
+++ b/parse.c
@@ -323,6 +323,10 @@ static struct init_keyword {
 	{ "__warn_unused_result__",	NS_KEYWORD,	.op = &ignore_attr_op },
 	{ "model",	NS_KEYWORD,	.op = &ignore_attr_op },
 	{ "__model__",	NS_KEYWORD,	.op = &ignore_attr_op },
+	{ "cdecl",	NS_KEYWORD,	.op = &ignore_attr_op },
+	{ "__cdecl__",	NS_KEYWORD,	.op = &ignore_attr_op },
+	{ "stdcall",	NS_KEYWORD,	.op = &ignore_attr_op },
+	{ "__stdcall__",	NS_KEYWORD,	.op = &ignore_attr_op },
 };
 
 void init_parser(int stream)
-- 
1.5.0.6


-- 
Michael Stefaniuc               Tel.: +49-711-96437-199
Sr. Network Engineer            Fax.: +49-711-96437-111
Red Hat GmbH                    Email: mstefani@redhat.com
Hauptstaetterstr. 58            http://www.redhat.de/
D-70178 Stuttgart

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Ignore the cdecl and stdcall attributes for now.

[Josh Triplett]

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

Michael Stefaniuc wrote:
> Wine uses the __stdcall__ attribute extensively. The effects of the
> patch on a sparse run on the Wine code are:
> - Removes 143000 "attribute '__stdcall__': unknown attribute" errors.
> - Removes 116 "attribute '__cdecl__': unknown attribute" errors.
> - Reduces the amount of "error: too many errors" from 1992 to 1459.
> 
> Signed-off-by: Michael Stefaniuc <mstefani@redhat.com>

Applied, along with a test case.  Thanks!

I'd love to see the results you get with Wine; in particular, I'd love to see
and fix any parse errors.  Would you consider posting a build log somewhere
with latest Sparse from Git?

Did you integrate Sparse into the Wine build system, or did you use CC=cgcc?

- Josh Triplett


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

Re: Ignore the cdecl and stdcall attributes for now.

[Michael Stefaniuc]

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

Josh Triplett wrote:
> Michael Stefaniuc wrote:
>> Wine uses the __stdcall__ attribute extensively. The effects of the
>> patch on a sparse run on the Wine code are:
>> - Removes 143000 "attribute '__stdcall__': unknown attribute" errors.
>> - Removes 116 "attribute '__cdecl__': unknown attribute" errors.
>> - Reduces the amount of "error: too many errors" from 1992 to 1459.
>>
>> Signed-off-by: Michael Stefaniuc <mstefani@redhat.com>
> 
> Applied, along with a test case.  Thanks!
Thanks.

> I'd love to see the results you get with Wine; in particular, I'd love to see
> and fix any parse errors.  Would you consider posting a build log somewhere
I thankfully accept your offer :)

> with latest Sparse from Git?
The output is still big. But i have a test case for the next big problem 
generator in Wine. It's attached as a git patch to 
validate/calling-convention-attributes.c (I can send it separately to 
the linux-sparse mailing list if you want). Sparse gives:
calling-convention-attributes.c:7:12: error: Expected ; at end of 
declaration
calling-convention-attributes.c:7:12: error: got f5ptr

> Did you integrate Sparse into the Wine build system, or did you use CC=cgcc?
Crude hack in my git tree to Make.rules adding a sparse call to the .c 
=> .o build rule. Tried to see how the Kernel build system integrates 
that and my  head started to smoke ... . I do not plan to send it 
upstream for the moment.

bye
	michael
-- 
Michael Stefaniuc               Tel.: +49-711-96437-199
Sr. Network Engineer            Fax.: +49-711-96437-111
Red Hat GmbH                    Email: mstefani@redhat.com
Hauptstaetterstr. 58            http://www.redhat.de/
D-70178 Stuttgart

[-- Attachment #2: 0001-Add-test-for-typedef-on-pointer-to-function-with-std.patch --]
[-- Type: text/x-patch, Size: 937 bytes --]

From a31ddd057217701c239677f820fa4bb5456ab079 Mon Sep 17 00:00:00 2001
From: Michael Stefaniuc <mstefani@redhat.com>
Date: Wed, 16 May 2007 13:12:42 +0200
Subject: [PATCH] Add test for typedef on pointer to function with stdcall attribute.


Signed-off-by: Michael Stefaniuc <mstefani@redhat.com>
---
 validation/calling-convention-attributes.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/validation/calling-convention-attributes.c b/validation/calling-convention-attributes.c
index 0546754..1bbe575 100644
--- a/validation/calling-convention-attributes.c
+++ b/validation/calling-convention-attributes.c
@@ -2,3 +2,6 @@ extern void __attribute__((cdecl)) f1(void);
 extern void __attribute__((__cdecl__)) f2(void);
 extern void __attribute__((stdcall)) f3(void);
 extern void __attribute__((__stdcall__)) f4(void);
+
+typedef void (__attribute__((__stdcall__)) *f5)(void);
+typedef f5 f5ptr;
-- 
1.5.0.6

Re: Ignore the cdecl and stdcall attributes for now.

[Sam Ravnborg]

> 
> >Did you integrate Sparse into the Wine build system, or did you use 
> >CC=cgcc?
> Crude hack in my git tree to Make.rules adding a sparse call to the .c 
> => .o build rule. Tried to see how the Kernel build system integrates 
> that and my  head started to smoke ... . I do not plan to send it 
> upstream for the moment.

If your smoking head has left a few questions about the kernel build 
system feel free to ask.
I have hacked a bit on kbuild so I should be able to help you.

But on the other hand I never looked at the Wine build system.

	Sam

Re: Ignore the cdecl and stdcall attributes for now.

[Michael Stefaniuc]

Josh Triplett wrote:
> Michael Stefaniuc wrote:
>> Josh Triplett wrote:
>>> Michael Stefaniuc wrote:
>>> I'd love to see the results you get with Wine; in particular, I'd love to see
>>> and fix any parse errors.  Would you consider posting a build log somewhere
>>> with latest Sparse from Git?
>> not sure if you are still interested but here is the output of
>> "building" wine with sparse:
>> http://people.redhat.com/mstefani/wine/download/wine+sparse-make.output.bz2
>> It was generated by "make clean; make > make.out 2>&1". Sparse runs
>> before every gcc call in the .c.o: make rule. As the wine build system
>> is verbose you'll see the exact command line used for sparse in the
>> above file.
> 
> Thanks for posting this.  (Any particular reason you didn't post it to
> linux-sparse?  If not, feel free to fullquote and CC the list.)
Didn't want to spam the list with it. Couldn't imagine that somebody 
wants to wade through 13 MB of Wine+sparse build log.

> Some observations:
Thanks for your time looking at this.

> -Wno-transparent-union should help; that would eliminate 318 warnings.
> 
> Don't pass -Wall to sparse unless you really mean it.  cgcc filters it out for
> a reason; just because you have -Wall in CFLAGS for GCC doesn't mean you want
> -Wall for sparse.  Sparse -Wall includes some warnings with high false
> positive rates that you probably don't want.
Ok good to know. I went the easy way just duplicating the gcc command 
line and replacing gcc with sparse and adding -D__i386___ as Wine won't 
build without a processor type defined. I'm still trying to figure out 
if sparse is useful (signal to noise ratio) for Wine. Wine has some 
constraints (having to follow an existing old grown API; compatibility 
with other C processors on non Linux OSes) that aren't a burden for the 
Linux Kernel. E.g. a patch to move to C99 struct initializer was 
recently rejected due to compatibility concerns with other C compilers.

> In particular, -Wall includes -Wundefined-preprocessor.  Avoiding that would
> probably eliminate thousands of warnings about symbols like _MSC_VER (at
> least, I would guess so without seeing the source of
> /wine/include/winnt.h:283).  With -Wundefined-preprocessor,
> #if expression-containing-SYMBOL
> will generate a warning if you haven't defined
> SYMBOL, and I would guess that happens here.  That said, you might want
> something like:
> #if defined(SYMBOL) && SYMBOL > number
I already looked at those and thought to fix them though I'm not sure if 
it is worth. I would have to look at the C standard what that says. The 
"fix" is trivial and should be compatible with any C compiler.

> The undefined preprocessor identifiers from limits.h come from not using cgcc,
> which defines them.  Sparse should ideally define those itself.  You can work
> around the problem by using cgcc or by defining the symbols on the sparse
> command line as cgcc does.
I'll do a run with cgcc tonight instead of sparse and check the difference.

> Apart from that, the main culprit looks like the one error you already
> mentioned and gave the test case for.  I don't know the cause of that one yet.
> As an error, it probably masks any warnings you might otherwise see.
Right. I've tried to run the test case in gdb but i see i need to learn 
the inner workings of sparse before i can make sense of what i see.

bye
	michael
-- 
Michael Stefaniuc               Tel.: +49-711-96437-199
Sr. Network Engineer            Fax.: +49-711-96437-111
Red Hat GmbH                    Email: mstefani@redhat.com
Hauptstaetterstr. 58            http://www.redhat.de/
D-70178 Stuttgart

Re: Ignore the cdecl and stdcall attributes for now.

[Josh Triplett]

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

Michael Stefaniuc wrote:
> Josh Triplett wrote:
>> Michael Stefaniuc wrote:
>>> Josh Triplett wrote:
>>>> Michael Stefaniuc wrote:
>>>> I'd love to see the results you get with Wine; in particular, I'd love to see
>>>> and fix any parse errors.  Would you consider posting a build log somewhere
>>>> with latest Sparse from Git?
>>> not sure if you are still interested but here is the output of
>>> "building" wine with sparse:
>>> http://people.redhat.com/mstefani/wine/download/wine+sparse-make.output.bz2
>>> It was generated by "make clean; make > make.out 2>&1". Sparse runs
>>> before every gcc call in the .c.o: make rule. As the wine build system
>>> is verbose you'll see the exact command line used for sparse in the
>>> above file.
>> Thanks for posting this.  (Any particular reason you didn't post it to
>> linux-sparse?  If not, feel free to fullquote and CC the list.)
> Didn't want to spam the list with it. Couldn't imagine that somebody 
> wants to wade through 13 MB of Wine+sparse build log.
> 
>> Some observations:
> Thanks for your time looking at this.
> 
>> -Wno-transparent-union should help; that would eliminate 318 warnings.
>>
>> Don't pass -Wall to sparse unless you really mean it.  cgcc filters it out for
>> a reason; just because you have -Wall in CFLAGS for GCC doesn't mean you want
>> -Wall for sparse.  Sparse -Wall includes some warnings with high false
>> positive rates that you probably don't want.
> Ok good to know. I went the easy way just duplicating the gcc command 
> line and replacing gcc with sparse and adding -D__i386___ as Wine won't 
> build without a processor type defined.

cgcc will define the processor type too.

I highly recommend trying a build with CC=cgcc.  You can then pass sparse
flags in CFLAGS, and cgcc will filter them out before calling CC; you can
also specify CHECK="sparse -Wfoo -Wno-bar" if you prefer not to change
CFLAGS.

> I'm still trying to figure out 
> if sparse is useful (signal to noise ratio) for Wine.

It will likely take some time and Sparse modifications in order to parse
Wine; however, I want Sparse to handle as much code as it can, not just
Linux.  I appreciate you trying it on Wine; I think working on this will
help both Wine and Sparse.

> Wine has some 
> constraints (having to follow an existing old grown API; compatibility 
> with other C processors on non Linux OSes) that aren't a burden for the 
> Linux Kernel. E.g. a patch to move to C99 struct initializer was 
> recently rejected due to compatibility concerns with other C compilers.

I just committed support for a -Wno-old-initializer flag to turn off the
sparse warning on non-C99 initializers.

>> In particular, -Wall includes -Wundefined-preprocessor.  Avoiding that would
>> probably eliminate thousands of warnings about symbols like _MSC_VER (at
>> least, I would guess so without seeing the source of
>> /wine/include/winnt.h:283).  With -Wundefined-preprocessor,
>> #if expression-containing-SYMBOL
>> will generate a warning if you haven't defined
>> SYMBOL, and I would guess that happens here.  That said, you might want
>> something like:
>> #if defined(SYMBOL) && SYMBOL > number
> I already looked at those and thought to fix them though I'm not sure if 
> it is worth. I would have to look at the C standard what that says. The 
> "fix" is trivial and should be compatible with any C compiler.

The C standard says that any undefined preprocessor symbol in an #if or #elif
becomes 0.  (See http://c0x.coding-guidelines.com/6.10.1.html , item 1859).
Sparse warns when doing so if you use -Wundefined-preprocessor; you might or
might not want that.

I just suggested including defined(SYMBOL) because you might not expect the
behavior that "#if FOO_VER < number" will pass and include the enclosed code
with FOO_VER undefined.

>> The undefined preprocessor identifiers from limits.h come from not using cgcc,
>> which defines them.  Sparse should ideally define those itself.  You can work
>> around the problem by using cgcc or by defining the symbols on the sparse
>> command line as cgcc does.
> I'll do a run with cgcc tonight instead of sparse and check the difference.

Thanks!

>> Apart from that, the main culprit looks like the one error you already
>> mentioned and gave the test case for.  I don't know the cause of that one yet.
>> As an error, it probably masks any warnings you might otherwise see.
> Right. I've tried to run the test case in gdb but i see i need to learn 
> the inner workings of sparse before i can make sense of what i see.

Feel free to ask if you need help or if you think you might have a theory.

- Josh Triplett


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

Re: Ignore the cdecl and stdcall attributes for now.

[Josh Triplett]

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

Michael Stefaniuc wrote:
> The output is still big. But i have a test case for the next big problem 
> generator in Wine. It's attached as a git patch to 
> validate/calling-convention-attributes.c (I can send it separately to 
> the linux-sparse mailing list if you want). Sparse gives:
> calling-convention-attributes.c:7:12: error: Expected ; at end of 
> declaration
> calling-convention-attributes.c:7:12: error: got f5ptr

I just applied a patch from Ramsay Jones which fixes this test case, and I've
applied your test case patch.

- Josh Triplett


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

Running sparse on the Wine code (Was: Re: Ignore the cdecl and stdcall attributes for now.)

[Michael Stefaniuc]

Josh Triplett wrote:
> Michael Stefaniuc wrote:
>> Josh Triplett wrote:
>>> Don't pass -Wall to sparse unless you really mean it.  cgcc filters it out for
>>> a reason; just because you have -Wall in CFLAGS for GCC doesn't mean you want
>>> -Wall for sparse.  Sparse -Wall includes some warnings with high false
>>> positive rates that you probably don't want.
>> Ok good to know. I went the easy way just duplicating the gcc command 
>> line and replacing gcc with sparse and adding -D__i386___ as Wine won't 
>> build without a processor type defined.
> 
> cgcc will define the processor type too.
> 
> I highly recommend trying a build with CC=cgcc.  You can then pass sparse
> flags in CFLAGS, and cgcc will filter them out before calling CC; you can
> also specify CHECK="sparse -Wfoo -Wno-bar" if you prefer not to change
> CFLAGS.
Hmm ... somehow i have confused cgcc with the code generating backend
that Jeff Garzik planned to write for sparse. With cgcc i won't need my
Wine build hack anymore and makes things even easier for me.

>> I'm still trying to figure out 
>> if sparse is useful (signal to noise ratio) for Wine.
> 
> It will likely take some time and Sparse modifications in order to parse
> Wine; however, I want Sparse to handle as much code as it can, not just
With cgcc Wine builds just fine (I didn't try yet the configure and make
depend stage of the build process). The only nitpick is that the ccache
cache from a normal build isn't valid anymore for the CC=cgcc run. I'll
have a look into it the next time i'll have to wait for the build to finish.

> Linux.  I appreciate you trying it on Wine; I think working on this will
> help both Wine and Sparse.
> 
>> Wine has some 
>> constraints (having to follow an existing old grown API; compatibility 
>> with other C processors on non Linux OSes) that aren't a burden for the 
>> Linux Kernel. E.g. a patch to move to C99 struct initializer was 
>> recently rejected due to compatibility concerns with other C compilers.
> 
> I just committed support for a -Wno-old-initializer flag to turn off the
> sparse warning on non-C99 initializers.
That was fast and I didn't even request it :)

>>> The undefined preprocessor identifiers from limits.h come from not using cgcc,
>>> which defines them.  Sparse should ideally define those itself.  You can work
>>> around the problem by using cgcc or by defining the symbols on the sparse
>>> command line as cgcc does.
>> I'll do a run with cgcc tonight instead of sparse and check the difference.
Updated
http://people.redhat.com/mstefani/wine/download/wine+sparse-make.output.bz2
That is the result of:
make clean; CHECK='sparse -Wno-transparent-union -Wno-old-initializer'
make CC=cgcc > make.out 2>&1
The output is with latest git sparse including the fix for the typedefs
to functions with a stdcall attribute.
I didn't look at the output yet as it is past bed time around here.

bye
	michael
-- 
Michael Stefaniuc               Tel.: +49-711-96437-199
Sr. Network Engineer            Fax.: +49-711-96437-111
Red Hat GmbH                    Email: mstefani@redhat.com
Hauptstaetterstr. 58            http://www.redhat.de/
D-70178 Stuttgart

Re: Running sparse on the Wine code (Was: Re: Ignore the cdecl and stdcall attributes for now.)

[Josh Triplett]

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

Michael Stefaniuc wrote:
> Josh Triplett wrote:
>> Michael Stefaniuc wrote:
>>> Josh Triplett wrote:
>>>> Don't pass -Wall to sparse unless you really mean it.  cgcc filters it out for
>>>> a reason; just because you have -Wall in CFLAGS for GCC doesn't mean you want
>>>> -Wall for sparse.  Sparse -Wall includes some warnings with high false
>>>> positive rates that you probably don't want.
>>> Ok good to know. I went the easy way just duplicating the gcc command 
>>> line and replacing gcc with sparse and adding -D__i386___ as Wine won't 
>>> build without a processor type defined.
>> cgcc will define the processor type too.
>>
>> I highly recommend trying a build with CC=cgcc.  You can then pass sparse
>> flags in CFLAGS, and cgcc will filter them out before calling CC; you can
>> also specify CHECK="sparse -Wfoo -Wno-bar" if you prefer not to change
>> CFLAGS.
> Hmm ... somehow i have confused cgcc with the code generating backend
> that Jeff Garzik planned to write for sparse. With cgcc i won't need my
> Wine build hack anymore and makes things even easier for me.

Glad to hear it.

>>> I'm still trying to figure out 
>>> if sparse is useful (signal to noise ratio) for Wine.
>> It will likely take some time and Sparse modifications in order to parse
>> Wine; however, I want Sparse to handle as much code as it can, not just
> With cgcc Wine builds just fine (I didn't try yet the configure and make
> depend stage of the build process). The only nitpick is that the ccache
> cache from a normal build isn't valid anymore for the CC=cgcc run. I'll
> have a look into it the next time i'll have to wait for the build to finish.

This might happen due to differing compilation flags.  ccache records the
compiler command line.

>> Linux.  I appreciate you trying it on Wine; I think working on this will
>> help both Wine and Sparse.
>>
>>> Wine has some 
>>> constraints (having to follow an existing old grown API; compatibility 
>>> with other C processors on non Linux OSes) that aren't a burden for the 
>>> Linux Kernel. E.g. a patch to move to C99 struct initializer was 
>>> recently rejected due to compatibility concerns with other C compilers.
>> I just committed support for a -Wno-old-initializer flag to turn off the
>> sparse warning on non-C99 initializers.
> That was fast and I didn't even request it :)

:)

>>>> The undefined preprocessor identifiers from limits.h come from not using cgcc,
>>>> which defines them.  Sparse should ideally define those itself.  You can work
>>>> around the problem by using cgcc or by defining the symbols on the sparse
>>>> command line as cgcc does.
>>> I'll do a run with cgcc tonight instead of sparse and check the difference.
> Updated
> http://people.redhat.com/mstefani/wine/download/wine+sparse-make.output.bz2
> That is the result of:
> make clean; CHECK='sparse -Wno-transparent-union -Wno-old-initializer'
> make CC=cgcc > make.out 2>&1
> The output is with latest git sparse including the fix for the typedefs
> to functions with a stdcall attribute.
> I didn't look at the output yet as it is past bed time around here.

Looks much better.

I've added a new -Wno-non-pointer-null flag which will turn off the "Using
plain integer as NULL pointer" warning, though unless you have a good reason I
suggest fixing those warnings by using a pointer type like NULL rather than 0.

"Bad character constant" seems to come from lines like this:
#define PROFILE_LINKED   'LINK'
The C standard says:
> The value of an integer character constant containing more than one
> character (e.g., 'ab'), or containing a character or escape sequence that
> does not map to a single-byte execution character, is
> implementation-defined.
(http://c0x.coding-guidelines.com/6.4.4.4.html#879)
GCC seems to treat it as an integer, but GCC warns about it too.  If you want
to supply a patch to parse it the same way GCC does and generate an optional
warning, that seems reasonable.

The yacc-generated source seems to have some non-ANSI function declarations.
I don't know if you can make it generate properly prototyped declarations.

I added __builtin_strcat and __builtin_strncat.  Wine seems to want the
latter, so this should fix several errors.

I added the constructor and destructor attributes, which Wine seems to want.
That should fix a few errors.

You can turn off "do-while statement is not a compound statement" with
-Wno-do-while, but I'd recommend just fixing such loops to use compound
statements.  That said, I don't know why that warning occurs by default;
possibly it shouldn't.

You can fix the "preprocessor token SYMBOL redefined" warnings by using:
#ifndef SYMBOL
#define SYMBOL ...
#endif
You might manage to talk me into a -Wno-redefined-preprocessor, though.

Unknown escape warnings for \? and \E seem odd.  What does Wine expect
those to do?

"warning: crazy programmer" needs a better description, as soon as I
figure out what it means. :)

- Josh Triplett


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