Feature request - suppress warnings for system libraries

Feature request - suppress warnings for system libraries

[Pavel Roskin]

Hello!

It would be great if sparse could optionally suppress warnings about
system headers included using from implicit locations, such as
"usr/include", "/usr/local/include" and GCC_INTERNAL_INCLUDE (see
pre-process.c).

Perhaps some major warnings could be left if they can indicate that
sparse may have failed to parse the header properly.  But the petty
stuff should be suppressed.

This would greatly help make sparse useful for code other than Linux
kernel.

-- 
Regards,
Pavel Roskin

Re: Feature request - suppress warnings for system libraries

[Christopher Li]

On Fri, Feb 02, 2007 at 12:43:03PM -0500, Pavel Roskin wrote:
> Hello!
> 
> It would be great if sparse could optionally suppress warnings about
> system headers included using from implicit locations, such as
> "usr/include", "/usr/local/include" and GCC_INTERNAL_INCLUDE (see
> pre-process.c).

Can you be more specific about which warning you are complaining?
Point to the line of sparse code or give a small test case will be nice.

> Perhaps some major warnings could be left if they can indicate that
> sparse may have failed to parse the header properly.  But the petty
> stuff should be suppressed.
> 
> This would greatly help make sparse useful for code other than Linux
> kernel.

Kernel checking takes more priority. But again, it does not hurt to have
option to disable it, as long as it is off by default.

Chris

Re: Feature request - suppress warnings for system libraries

[Christopher Li]

On Fri, Feb 02, 2007 at 05:37:45PM -0500, Pavel Roskin wrote:
> 
> /usr/include/sys/socket.h:88:62: error: attribute
> '__transparent_union__': ignoring attribute __transparent_union__
> /usr/include/sys/socket.h:92:68: error: attribute
> '__transparent_union__': ignoring attribute __transparent_union__
> /usr/include/netdb.h:661:60: error: typename in expression
> /usr/include/stdlib.h:72:56: error: attribute '__transparent_union__':
> ignoring attribute __transparent_union__

I see, that is because sparse does not handle attribute
__transparent_union__ yet. We should improve sparse instead of paper
cover the error message.

> /usr/include/regex.h:543:27: error: typename in expression

Similar here, that is because gcc has this key word "restrict"
inside bracket. Sparse know nothing about "restrict".

> I'm not going to fix socket.h.  I'm checking my code, not glibc.

Again, we should fix sparse instead.

Chris

Re: Feature request - suppress warnings for system libraries

[Pavel Roskin]

On Fri, 2007-02-02 at 14:01 -0800, Christopher Li wrote:
> On Fri, Feb 02, 2007 at 12:43:03PM -0500, Pavel Roskin wrote:
> > Hello!
> > 
> > It would be great if sparse could optionally suppress warnings about
> > system headers included using from implicit locations, such as
> > "usr/include", "/usr/local/include" and GCC_INTERNAL_INCLUDE (see
> > pre-process.c).
> 
> Can you be more specific about which warning you are complaining?
> Point to the line of sparse code or give a small test case will be nice.

I don't want to see stuff like this for every non-trivial file I compile
(that's Fedora Core 6, not some ancient distro):

/usr/include/sys/socket.h:88:62: error: attribute
'__transparent_union__': ignoring attribute __transparent_union__
/usr/include/sys/socket.h:92:68: error: attribute
'__transparent_union__': ignoring attribute __transparent_union__
/usr/include/netdb.h:661:60: error: typename in expression
/usr/include/stdlib.h:72:56: error: attribute '__transparent_union__':
ignoring attribute __transparent_union__
/usr/include/regex.h:543:27: error: typename in expression

I'm not going to fix socket.h.  I'm checking my code, not glibc.

> Kernel checking takes more priority. But again, it does not hurt to have
> option to disable it, as long as it is off by default.

Actually, the kernel should not be affected since it's compiled without
system includes.

-- 
Regards,
Pavel Roskin

Re: Feature request - suppress warnings for system libraries

[Christopher Li]

On Fri, Feb 02, 2007 at 11:17:05PM +0000, Al Viro wrote:
> On Fri, Feb 02, 2007 at 02:31:55PM -0800, Christopher Li wrote:
> 
> I'm not sure.  __transparent_union__ is an atrocious kludge and it does
> deserve a warning.  So getting it to STFU on known offenders we have no
> chance to fix is OK, but legitimizing that abortion is not.

I did not mean to legitimizing it. I just don't want to silence
suppress this kind of warnings.

> Unlike __transparent_union__, restrict is at least a valid C...  It's not
> that hard to handle, except for the shortage of bits for modifiers...

I have a patch free up the specials bits in modifiers. It helps a little
bit. But I think we need a attribute pointer in ctype to fit more attributes
eventually.

Chris

Re: Feature request - suppress warnings for system libraries

[Al Viro]

On Fri, Feb 02, 2007 at 02:31:55PM -0800, Christopher Li wrote:
> On Fri, Feb 02, 2007 at 05:37:45PM -0500, Pavel Roskin wrote:
> > 
> > /usr/include/sys/socket.h:88:62: error: attribute
> > '__transparent_union__': ignoring attribute __transparent_union__
> > /usr/include/sys/socket.h:92:68: error: attribute
> > '__transparent_union__': ignoring attribute __transparent_union__
> > /usr/include/netdb.h:661:60: error: typename in expression
> > /usr/include/stdlib.h:72:56: error: attribute '__transparent_union__':
> > ignoring attribute __transparent_union__
> 
> I see, that is because sparse does not handle attribute
> __transparent_union__ yet. We should improve sparse instead of paper
> cover the error message.

I'm not sure.  __transparent_union__ is an atrocious kludge and it does
deserve a warning.  So getting it to STFU on known offenders we have no
chance to fix is OK, but legitimizing that abortion is not.
 
> > /usr/include/regex.h:543:27: error: typename in expression
> 
> Similar here, that is because gcc has this key word "restrict"
> inside bracket. Sparse know nothing about "restrict".
> 
> > I'm not going to fix socket.h.  I'm checking my code, not glibc.
> 
> Again, we should fix sparse instead.

Unlike __transparent_union__, restrict is at least a valid C...  It's not
that hard to handle, except for the shortage of bits for modifiers...

Re: Feature request - suppress warnings for system libraries

[Linus Torvalds]

On Fri, 2 Feb 2007, Al Viro wrote:
> 
> I'm not sure.  __transparent_union__ is an atrocious kludge and it does
> deserve a warning.  So getting it to STFU on known offenders we have no
> chance to fix is OK, but legitimizing that abortion is not.

Yeah, it really is pretty nasty. That said, nobody really uses it except 
for glibc, and almost nobody is interested in that warning. So we could 
have something that just barely parses the thing enough to avoid the 
warning at parse type, and then sets a flag: don't check this type on the 
caller (so that the _sane_ callers of "wait()" don't get warnings about 
the type).

IOW, just a hack to avoid the warning, no real "support" for that horrid 
thing.

> Unlike __transparent_union__, restrict is at least a valid C...  It's not
> that hard to handle, except for the shortage of bits for modifiers...

At worst, we could parse and ignore it.

		Linus

Re: Feature request - suppress warnings for system libraries

[Al Viro]

On Fri, Feb 02, 2007 at 03:25:20PM -0800, Linus Torvalds wrote:
> > Unlike __transparent_union__, restrict is at least a valid C...  It's not
> > that hard to handle, except for the shortage of bits for modifiers...
> 
> At worst, we could parse and ignore it.

*nod*

BTW, speaking of atrocities...

include/sound/asound.h:
typedef int __bitwise snd_pcm_hw_param_t;
#define SNDRV_PCM_HW_PARAM_ACCESS       ((__force snd_pcm_hw_param_t) 0) /* Access type */
#define SNDRV_PCM_HW_PARAM_FORMAT       ((__force snd_pcm_hw_param_t) 1) /* Format */
#define SNDRV_PCM_HW_PARAM_SUBFORMAT    ((__force snd_pcm_hw_param_t) 2) /* Subformat */
#define SNDRV_PCM_HW_PARAM_FIRST_MASK   SNDRV_PCM_HW_PARAM_ACCESS
#define SNDRV_PCM_HW_PARAM_LAST_MASK    SNDRV_PCM_HW_PARAM_SUBFORMAT

....

struct snd_pcm_hw_params {
        unsigned int flags;
        struct snd_mask masks[SNDRV_PCM_HW_PARAM_LAST_MASK -
                               SNDRV_PCM_HW_PARAM_FIRST_MASK + 1];


WTF is going on here?  We obviously do *not* treat that as bitwise.
Not only a blatant subtraction in the above; we do add that sucker to
pointer when accessing array elements.  Moreover, just look at the
other values:
#define SNDRV_PCM_HW_PARAM_SAMPLE_BITS  ((__force snd_pcm_hw_param_t) 8) /* Bits per sample */
#define SNDRV_PCM_HW_PARAM_FRAME_BITS   ((__force snd_pcm_hw_param_t) 9) /* Bits per frame */
#define SNDRV_PCM_HW_PARAM_CHANNELS     ((__force snd_pcm_hw_param_t) 10) /* Channels */
#define SNDRV_PCM_HW_PARAM_RATE         ((__force snd_pcm_hw_param_t) 11) /* Approx rate */  
#define SNDRV_PCM_HW_PARAM_PERIOD_TIME  ((__force snd_pcm_hw_param_t) 12) /* Approx distance between interrupts in us */
etc.

It's certainly not a bitwise type.  I'm not sure what the hell it's really
meant to be...   We have a similar turd in pm.h:
typedef int __bitwise suspend_state_t;

#define PM_SUSPEND_ON           ((__force suspend_state_t) 0)
#define PM_SUSPEND_STANDBY      ((__force suspend_state_t) 1)
#define PM_SUSPEND_MEM          ((__force suspend_state_t) 3)
#define PM_SUSPEND_DISK         ((__force suspend_state_t) 4)
#define PM_SUSPEND_MAX          ((__force suspend_state_t) 5)

with things like
./kernel/power/main.c:274:      for (s = &pm_states[state]; state < PM_SUSPEND_MAX; s++, state++) {

using it.  Again, we have uses of that sucker as array size, for ordered
comparisons, for adding to pointer and for adding integer to it.  And
|, &, ^ and ~ are very definitely _not_ wanted on these values.

There probably is some legitimate set of checks the authors wanted, but
__bitwise__ sure as hell is not it...

Re: Feature request - suppress warnings for system libraries

[Al Viro]

On Sat, Feb 03, 2007 at 12:30:30AM +0000, Al Viro wrote:
> There probably is some legitimate set of checks the authors wanted, but
> __bitwise__ sure as hell is not it...

... and even weirder:

typedef int __bitwise snd_pcm_format_t;
#define SNDRV_PCM_FORMAT_S8     ((__force snd_pcm_format_t) 0)
#define SNDRV_PCM_FORMAT_U8     ((__force snd_pcm_format_t) 1)
#define SNDRV_PCM_FORMAT_S16_LE ((__force snd_pcm_format_t) 2)
#define SNDRV_PCM_FORMAT_S16_BE ((__force snd_pcm_format_t) 3)
#define SNDRV_PCM_FORMAT_U16_LE ((__force snd_pcm_format_t) 4)
#define SNDRV_PCM_FORMAT_U16_BE ((__force snd_pcm_format_t) 5)
....
#define SNDRV_PCM_FMTBIT_S8             (1ULL << SNDRV_PCM_FORMAT_S8)
#define SNDRV_PCM_FMTBIT_U8             (1ULL << SNDRV_PCM_FORMAT_U8)
#define SNDRV_PCM_FMTBIT_S16_LE         (1ULL << SNDRV_PCM_FORMAT_S16_LE)
#define SNDRV_PCM_FMTBIT_S16_BE         (1ULL << SNDRV_PCM_FORMAT_S16_BE)
#define SNDRV_PCM_FMTBIT_U16_LE         (1ULL << SNDRV_PCM_FORMAT_U16_LE)
#define SNDRV_PCM_FMTBIT_U16_BE         (1ULL << SNDRV_PCM_FORMAT_U16_BE)