Ignore the cdecl and stdcall attributes for now.
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.
--- parse.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
parse.c +4 -0
@@ -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)
Signature · 9 lines
1.5.0.6 -- Michael Stefaniuc Tel.: +49-711-96437-199 Sr. Network Engineer Fax.: +49-711-96437-111 Red Hat GmbH Email: [email protected] Hauptstaetterstr. 58 http://www.redhat.de/ D-70178 Stuttgart
Quoted 7 lines · Michael Stefaniuc
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 <[email protected]>
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
Quoted 10 lines · Josh Triplett
Quoted 7 lines · Michael Stefaniuc
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 <[email protected]>Applied, along with a test case. Thanks!
Thanks.
Quoted 2 lines
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 :)
Quoted 1 line
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
Quoted 1 line
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
Signature · 5 lines
Michael Stefaniuc Tel.: +49-711-96437-199 Sr. Network Engineer Fax.: +49-711-96437-111 Red Hat GmbH Email: [email protected] Hauptstaetterstr. 58 http://www.redhat.de/ D-70178 Stuttgart
From a31ddd057217701c239677f820fa4bb5456ab079 Mon Sep 17 00:00:00 2001
From: Michael Stefaniuc <[email protected]>
Date: Wed, 16 May 2007 13:12:42 +0200
Subject: [PATCH] Add test for typedef on pointer to function with stdcall attribute.
--- validation/calling-convention-attributes.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
validation/calling-convention-attributes.c +3 -0
@@ -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 ;
Signature · 1 line
1.5.0.6
Quoted 7 lines
Quoted 3 lines
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
Quoted 7 lines · Michael Stefaniuc
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 14 lines · Josh Triplett
Quoted 5 lines · Michael Stefaniuc
Quoted 4 lines · Josh Triplett
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?Quoted 7 lines
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.
Quoted 2 lines
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.
Quoted 1 line
Some observations:
Thanks for your time looking at this.
Quoted 6 lines
-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.
Quoted 9 lines
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.
Quoted 4 lines
The undefined preprocessor identifiers from
limits.hcome 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.
Quoted 3 lines
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
Signature · 5 lines
Michael Stefaniuc Tel.: +49-711-96437-199 Sr. Network Engineer Fax.: +49-711-96437-111 Red Hat GmbH Email: [email protected] Hauptstaetterstr. 58 http://www.redhat.de/ D-70178 Stuttgart
Quoted 24 lines · Michael Stefaniuc
Quoted 13 lines · Josh Triplett
Quoted 5 lines · Michael Stefaniuc
Quoted 4 lines · Josh Triplett
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?Quoted 7 lines
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.Quoted 2 lines
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.
Quoted 1 line
Some observations:
Thanks for your time looking at this.
Quoted 2 lines
-Wno-transparent-union should help; that would eliminate 318 warnings.
Quoted 7 lines
Quoted 4 lines
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.
Quoted 2 lines
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.
Quoted 5 lines
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.
Quoted 9 lines
Quoted 9 lines
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
Quoted 3 lines
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.
Quoted 5 lines
Quoted 4 lines
The undefined preprocessor identifiers from
limits.hcome 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!
Quoted 5 lines
Quoted 3 lines
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
Running sparse on the Wine code (Was: Re: Ignore the cdecl and stdcall attributes for now.)
Quoted 10 lines · Josh Triplett
Quoted 8 lines · Michael Stefaniuc
Quoted 4 lines · Josh Triplett
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.
Quoted 6 lines
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 11 lines
Linux. I appreciate you trying it on Wine; I think working on this will help both Wine and Sparse.
Quoted 5 lines
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 :)
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
The undefined preprocessor identifiers from
limits.hcome 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
Signature · 5 lines
Michael Stefaniuc Tel.: +49-711-96437-199 Sr. Network Engineer Fax.: +49-711-96437-111 Red Hat GmbH Email: [email protected] 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.)
Quoted 12 lines · Michael Stefaniuc
Quoted 11 lines · Josh Triplett
Quoted 8 lines · Michael Stefaniuc
Quoted 4 lines · Josh Triplett
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.
Quoted 7 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
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.
Quoted 11 lines
Quoted 10 lines
Linux. I appreciate you trying it on Wine; I think working on this will help both Wine and Sparse.
Quoted 5 lines
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 :)
:)
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
The undefined preprocessor identifiers from
limits.hcome 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.
Quoted 8 lines
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:
Quoted 4 lines
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