linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Triplett <josh@freedesktop.org>
To: Michael Stefaniuc <mstefani@redhat.com>
Cc: linux-sparse@vger.kernel.org
Subject: Re: Ignore the cdecl and stdcall attributes for now.
Date: Tue, 22 May 2007 13:32:51 -0700	[thread overview]
Message-ID: <46535373.3020109@freedesktop.org> (raw)
In-Reply-To: <46530112.2030105@redhat.com>

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

  reply	other threads:[~2007-05-22 20:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-15 22:05 Ignore the cdecl and stdcall attributes for now Michael Stefaniuc
2007-05-16  6:32 ` Josh Triplett
2007-05-16 11:19   ` Michael Stefaniuc
2007-05-16 12:46     ` Sam Ravnborg
2007-05-22 22:39     ` Josh Triplett
     [not found]   ` <46520647.9090802@redhat.com>
     [not found]     ` <4652819D.4040101@freedesktop.org>
2007-05-22 14:41       ` Michael Stefaniuc
2007-05-22 20:32         ` Josh Triplett [this message]
2007-05-22 23:44           ` Running sparse on the Wine code (Was: Re: Ignore the cdecl and stdcall attributes for now.) Michael Stefaniuc
2007-05-23  2:01             ` Josh Triplett

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46535373.3020109@freedesktop.org \
    --to=josh@freedesktop.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=mstefani@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).