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

[Josh Triplett <josh@freedesktop.org>]

[-- 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 --]