# [PATCH 0/1] Using sparse in a CI job

7 messages from 2019-02-01 to 2019-02-03. Participants: Ramsay Jones, Luc Van Oostenryck, SZEDER Gábor.
Thread: https://gitlist.dev/t/50380

## Ramsay Jones, 2019-02-01 21:01

Subject: [PATCH 0/1] Using sparse in a CI job
Message-ID: <78f85f0b-176f-0b54-2ec8-4c13fd386733@ramsayjones.plus.com>
URL: https://gitlist.dev/e/78f85f0b-176f-0b54-2ec8-4c13fd386733%40ramsayjones.plus.com

```

I suspect that the Makefile sparse target is not easy to use in a CI
job, since the 'sparse' program (via cgcc -no-compile) does not exit
with a non-zero value, even when issuing errors and warnings.

The way I use sparse, this is not an issue. I run sparse over the
master branch, check that the output file has the no/expected
errors and warnings, then just diff the output files from the
next and pu branches. something like:

  $ make sparse >sp-out 2>&1 # on master branch
  $ vim sp-out # check for errors/warnings
  ...
  $ make sparse >nsp-out 2>&1 # on next branch
  $ diff sp-out nsp-out # any new errors/warnings?
  ...
  $ make sparse >psp-out 2>&1 # on pu branch
  $ diff nsp-out psp-out # any new errors/warnings?

At the moment, on Linux, the sp-out file is free from any sparse errors
or warnings. So are next and pu:

  $ grep error sp-out
  $ grep warning sp-out
  $ diff sp-out nsp-out
  391a392
  >     SP fuzz-commit-graph.c
  $ diff nsp-out psp-out
  170a171,180
  >     SP trace2.c
  >     SP trace2/tr2_cfg.c
  >     SP trace2/tr2_dst.c
  >     SP trace2/tr2_sid.c
  >     SP trace2/tr2_tbuf.c
  >     SP trace2/tr2_tgt_event.c
  >     SP trace2/tr2_tgt_normal.c
  >     SP trace2/tr2_tgt_perf.c
  >     SP trace2/tr2_tls.c
  >     SP trace2/tr2_verb.c
  298a309
  >     SP builtin/stash.c
  375a387
  >     SP t/helper/test-trace2.c
  376a389
  >     SP t/helper/test-xml-encode.c
  $ 

However, if we go back a few days, then the pu branch was not clean:

  $ diff nsp-out psp-out
  18a20,24
  >     SP change-table.c
  > change-table.h:53:24: error: dubious one-bit signed bitfield
  > change-table.h:54:25: error: dubious one-bit signed bitfield
  > change-table.h:55:25: error: dubious one-bit signed bitfield
  > change-table.h:56:26: error: dubious one-bit signed bitfield
  69a76
  >     GEN command-list.h
  93a101,106
  >     SP metacommit.c
  > change-table.h:53:24: error: dubious one-bit signed bitfield
  > change-table.h:54:25: error: dubious one-bit signed bitfield
  > change-table.h:55:25: error: dubious one-bit signed bitfield
  > change-table.h:56:26: error: dubious one-bit signed bitfield
  >     SP metacommit-parser.c
  170a184,193
  >     SP trace2.c
  >     SP trace2/tr2_cfg.c
  >     SP trace2/tr2_dst.c
  >     SP trace2/tr2_sid.c
  >     SP trace2/tr2_tbuf.c
  >     SP trace2/tr2_tgt_event.c
  >     SP trace2/tr2_tgt_normal.c
  >     SP trace2/tr2_tgt_perf.c
  >     SP trace2/tr2_tls.c
  >     SP trace2/tr2_verb.c
  213a237
  >     SP builtin/change.c
  298a323
  >     SP builtin/stash.c
  375a401
  >     SP t/helper/test-trace2.c
  376a403,405
  >     SP t/helper/test-xml-encode.c
  > t/helper/test-xml-encode.c:29:40: warning: Using plain integer as NULL pointer
  > t/helper/test-xml-encode.c:37:40: warning: Using plain integer as NULL pointer
  $ 
 
Note that 'make' does not exit at the first 'error', since the command exit
code is zero (or not non-zero! :) ):
 
  $ make change-table.sp
      SP change-table.c
  change-table.h:53:24: error: dubious one-bit signed bitfield
  change-table.h:54:25: error: dubious one-bit signed bitfield
  change-table.h:55:25: error: dubious one-bit signed bitfield
  change-table.h:56:26: error: dubious one-bit signed bitfield
  $ echo $?
  0
  $ 

We can change that by passing '-Wsparse-error' to 'sparse':

  $ make SPARSE_FLAGS=-Wsparse-error change-table.sp
      SP change-table.c
  change-table.h:53:24: error: dubious one-bit signed bitfield
  change-table.h:54:25: error: dubious one-bit signed bitfield
  change-table.h:55:25: error: dubious one-bit signed bitfield
  change-table.h:56:26: error: dubious one-bit signed bitfield
  Makefile:2729: recipe for target 'change-table.sp' failed
  make: *** [change-table.sp] Error 1
  $ echo $?
  2
  $ 

Note that '-Wsparse-error' not only returns a non-zero exit code (1), but
it also changes a 'warning' into an 'error' (see above):
  
  $ make SPARSE_FLAGS=-Wsparse-error t/helper/test-xml-encode.sp
      SP t/helper/test-xml-encode.c
  t/helper/test-xml-encode.c:29:40: error: Using plain integer as NULL pointer
  t/helper/test-xml-encode.c:37:40: error: Using plain integer as NULL pointer
  Makefile:2729: recipe for target 't/helper/test-xml-encode.sp' failed
  make: *** [t/helper/test-xml-encode.sp] Error 1
  $ echo $?
  2
  $ 

Unfortunately, using SPARSE_FLAGS from the 'make' command-line is not
ideal, since it was not really intended to be used that way. This will
cause problems for those files which have target specific settings for
SPARSE_FLAGS. For example:

  $ make pack-revindex.sp
      SP pack-revindex.c
  $ make SPARSE_FLAGS=-Wsparse-error pack-revindex.sp
      SP pack-revindex.c
  pack-revindex.c:65:23: error: memset with byte count of 262144
  Makefile:2729: recipe for target 'pack-revindex.sp' failed
  make: *** [pack-revindex.sp] Error 1
  $ echo $?
  2
  $ 

So, passing SPARSE_FLAGS on the command-line, effectively nullifies the
target specific settings (making them useless).

This patch splits the duties of SPARSE_FLAGS, by introducing a separate
SP_EXTRA_FLAGS variable, for use with the target specific settings. In
addition, we use a conditional assignment (?=) of the default (empty)
value of SPARSE_FLAGS, to allow setting the value of this variable from
the environment as well as the command-line. So, after this patch:

  $ make SPARSE_FLAGS=-Wsparse-error pack-revindex.sp
      SP pack-revindex.c
  $ echo $?
  0
  $ 

  $ SPARSE_FLAGS=-Wsparse-error make pack-revindex.sp
      SP pack-revindex.c
  $ echo $?
  0
  $ 

Now, we should be able to run the sparse Makefile target in a CI job, and
still find all sparse errors and warnings (now marked as errors also),
using something like this:

  $ SPARSE_FLAGS=-Wsparse-error make -k sparse >sp-out 2>&1

Note that the '-k' argument to 'make' is now required. ;-)


ATB,
Ramsay Jones 

Ramsay Jones (1):
  Makefile: improve SPARSE_FLAGS customisation

 Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.20.0

```

## Luc Van Oostenryck, 2019-02-01 22:40

Subject: Re: [PATCH 0/1] Using sparse in a CI job
Message-ID: <20190201223959.zmlyg4buwolw2b3p@ltop.local>
URL: https://gitlist.dev/e/20190201223959.zmlyg4buwolw2b3p%40ltop.local
In-Reply-To: <78f85f0b-176f-0b54-2ec8-4c13fd386733@ramsayjones.plus.com>

```
On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
> 
> I suspect that the Makefile sparse target is not easy to use in a CI
> job, since the 'sparse' program (via cgcc -no-compile) does not exit
> with a non-zero value, even when issuing errors and warnings.

...
 
> We can change that by passing '-Wsparse-error' to 'sparse':
> 
>   $ make SPARSE_FLAGS=-Wsparse-error change-table.sp
>       SP change-table.c
>   change-table.h:53:24: error: dubious one-bit signed bitfield
>   change-table.h:54:25: error: dubious one-bit signed bitfield
>   change-table.h:55:25: error: dubious one-bit signed bitfield
>   change-table.h:56:26: error: dubious one-bit signed bitfield
>   Makefile:2729: recipe for target 'change-table.sp' failed
>   make: *** [change-table.sp] Error 1
>   $ echo $?
>   2
>   $ 
> 
> Note that '-Wsparse-error' not only returns a non-zero exit code (1), but
> it also changes a 'warning' into an 'error' (see above):

Yes, I know :(
The fact that, by default, sparse doesn't fail on errors is wanted
(otherwise it would break the kernel compile). But that the only way
to return an error is to use -Wsparse-error (which is supposed to
replace GCC's -Werror) is a real problem.

-- Luc

```

## SZEDER Gábor, 2019-02-02 00:41

Subject: Re: [PATCH 0/1] Using sparse in a CI job
Message-ID: <20190202004145.GE10587@szeder.dev>
URL: https://gitlist.dev/e/20190202004145.GE10587%40szeder.dev
In-Reply-To: <78f85f0b-176f-0b54-2ec8-4c13fd386733@ramsayjones.plus.com>

```
On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
> I suspect that the Makefile sparse target is not easy to use in a CI
> job, since the 'sparse' program (via cgcc -no-compile) does not exit
> with a non-zero value, even when issuing errors and warnings.

We had the same issue with Coccinelle in the static analysis build job
on Travis CI, which was happily succeeding even when Coccinelle
noticed undesired code patterns entering the codebase.  We've dealt
with that by looking at the patch files generated by 'make
coccicheck': if they were empty, then all is well, if they are not,
then dump their contents into the log and 'exit 1'.  See 0860a7641b
(travis-ci: fail if Coccinelle static analysis found something to
transform, 2018-07-23).

I think we could do something like that with sparse as well.

> At the moment, on Linux, the sp-out file is free from any sparse errors
> or warnings. So are next and pu:
> 
>   $ grep error sp-out
>   $ grep warning sp-out

On 'master' I get:

  $ grep error sp-out 
  $ grep warning sp-out 
  connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
  pack-revindex.c:65:23: warning: memset with byte count of 262144
  unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
  unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
  daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
  daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
  imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
  credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
  $ sparse --version
  v0.5.0


```

## Ramsay Jones, 2019-02-03 01:49

Subject: Re: [PATCH 0/1] Using sparse in a CI job
Message-ID: <719468c8-3c62-96d6-57de-b0629fab0be3@ramsayjones.plus.com>
URL: https://gitlist.dev/e/719468c8-3c62-96d6-57de-b0629fab0be3%40ramsayjones.plus.com
In-Reply-To: <20190202004145.GE10587@szeder.dev>

```


On 02/02/2019 00:41, SZEDER Gábor wrote:
> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
[snip]

 
>> At the moment, on Linux, the sp-out file is free from any sparse errors
>> or warnings. So are next and pu:
>>
>>   $ grep error sp-out
>>   $ grep warning sp-out
> 
> On 'master' I get:
> 
>   $ grep error sp-out 
>   $ grep warning sp-out 
>   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
>   pack-revindex.c:65:23: warning: memset with byte count of 262144
>   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
>   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
>   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
>   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
>   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
>   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
>   $ sparse --version
>   v0.5.0

Yeah, that version of sparse is a bit too old.

If memory serves (it may not), all of the 'argument 2 (invalid types)'
errors are caused by the glibc headers using a 'transparent union' to
define the 'struct sockaddr' type. sparse could not handle that until
commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning
was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in
sparse and commit 54360a1956 in git.

So, it seems you need at least v0.5.2 of sparse on your Linux system
(which can't be too recent, or you would need v0.6.0).

ATB,
Ramsay Jones


```

## SZEDER Gábor, 2019-02-03 12:12

Subject: Re: [PATCH 0/1] Using sparse in a CI job
Message-ID: <20190203121207.GF10587@szeder.dev>
URL: https://gitlist.dev/e/20190203121207.GF10587%40szeder.dev
In-Reply-To: <719468c8-3c62-96d6-57de-b0629fab0be3@ramsayjones.plus.com>

```
On Sun, Feb 03, 2019 at 01:49:37AM +0000, Ramsay Jones wrote:
> On 02/02/2019 00:41, SZEDER Gábor wrote:
> > On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
> >> At the moment, on Linux, the sp-out file is free from any sparse errors
> >> or warnings. So are next and pu:
> >>
> >>   $ grep error sp-out
> >>   $ grep warning sp-out
> > 
> > On 'master' I get:
> > 
> >   $ grep error sp-out 
> >   $ grep warning sp-out 
> >   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
> >   pack-revindex.c:65:23: warning: memset with byte count of 262144
> >   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
> >   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
> >   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
> >   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
> >   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
> >   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
> >   $ sparse --version
> >   v0.5.0
> 
> Yeah, that version of sparse is a bit too old.
> 
> If memory serves (it may not), all of the 'argument 2 (invalid types)'
> errors are caused by the glibc headers using a 'transparent union' to
> define the 'struct sockaddr' type. sparse could not handle that until
> commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning
> was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in
> sparse and commit 54360a1956 in git.
> 
> So, it seems you need at least v0.5.2 of sparse on your Linux system
> (which can't be too recent, or you would need v0.6.0).

The latest Linux image available on Travis CI is based on Ubuntu 16.04
LTS, which contains sparse 0.5.0, while their default image is based
on 14.04, with sparse 0.4.5-rc1.  Even the latest Ubuntu LTS is only
at 0.5.1.

  https://travis-ci.org/szeder/git/jobs/488113660#L487
  https://travis-ci.org/szeder/git/jobs/488113661#L208


```

## Ramsay Jones, 2019-02-03 15:19

Subject: Re: [PATCH 0/1] Using sparse in a CI job
Message-ID: <29138259-51cb-dec5-6f99-865868a00366@ramsayjones.plus.com>
URL: https://gitlist.dev/e/29138259-51cb-dec5-6f99-865868a00366%40ramsayjones.plus.com
In-Reply-To: <20190201223959.zmlyg4buwolw2b3p@ltop.local>

```


On 01/02/2019 22:40, Luc Van Oostenryck wrote:
> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
>>
>> I suspect that the Makefile sparse target is not easy to use in a CI
>> job, since the 'sparse' program (via cgcc -no-compile) does not exit
>> with a non-zero value, even when issuing errors and warnings.
> 
> ...
>  
>> We can change that by passing '-Wsparse-error' to 'sparse':
>>
>>   $ make SPARSE_FLAGS=-Wsparse-error change-table.sp
>>       SP change-table.c
>>   change-table.h:53:24: error: dubious one-bit signed bitfield
>>   change-table.h:54:25: error: dubious one-bit signed bitfield
>>   change-table.h:55:25: error: dubious one-bit signed bitfield
>>   change-table.h:56:26: error: dubious one-bit signed bitfield
>>   Makefile:2729: recipe for target 'change-table.sp' failed
>>   make: *** [change-table.sp] Error 1
>>   $ echo $?
>>   2
>>   $ 
>>
>> Note that '-Wsparse-error' not only returns a non-zero exit code (1), but
>> it also changes a 'warning' into an 'error' (see above):
> 
> Yes, I know :(
> The fact that, by default, sparse doesn't fail on errors is wanted
> (otherwise it would break the kernel compile). But that the only way
> to return an error is to use -Wsparse-error (which is supposed to
> replace GCC's -Werror) is a real problem.

Given that I only use sparse as a checker, I actually don't mind
the current behaviour. That would be different if I was using
sparsec/sparsei etc., as a compiler, of course! ;-)

[If I were to suggest any change at all to -Wsparse-error it would
be: do not change the 'warning' to an 'error' (yes, the actual text
label of the message), exit(1) if any errors or warnings, but *if*
only warnings have been issued, then print an "treating warnings
as errors [-Wsparse-error]" message.]

ATB,
Ramsay Jones


```

## Ramsay Jones, 2019-02-03 15:43

Subject: Re: [PATCH 0/1] Using sparse in a CI job
Message-ID: <9892484b-93f4-b99b-a600-fdcc8fbff806@ramsayjones.plus.com>
URL: https://gitlist.dev/e/9892484b-93f4-b99b-a600-fdcc8fbff806%40ramsayjones.plus.com
In-Reply-To: <20190203121207.GF10587@szeder.dev>

```


On 03/02/2019 12:12, SZEDER Gábor wrote:
> On Sun, Feb 03, 2019 at 01:49:37AM +0000, Ramsay Jones wrote:
>> On 02/02/2019 00:41, SZEDER Gábor wrote:
>>> On Fri, Feb 01, 2019 at 09:01:20PM +0000, Ramsay Jones wrote:
>>>> At the moment, on Linux, the sp-out file is free from any sparse errors
>>>> or warnings. So are next and pu:
>>>>
>>>>   $ grep error sp-out
>>>>   $ grep warning sp-out
>>>
>>> On 'master' I get:
>>>
>>>   $ grep error sp-out 
>>>   $ grep warning sp-out 
>>>   connect.c:652:40: warning: incorrect type in argument 2 (invalid types)
>>>   pack-revindex.c:65:23: warning: memset with byte count of 262144
>>>   unix-socket.c:83:26: warning: incorrect type in argument 2 (invalid types)
>>>   unix-socket.c:108:23: warning: incorrect type in argument 2 (invalid types)
>>>   daemon.c:1041:36: warning: incorrect type in argument 2 (invalid types)
>>>   daemon.c:1184:67: warning: incorrect type in argument 2 (invalid types)
>>>   imap-send.c:1022:42: warning: incorrect type in argument 2 (invalid types)
>>>   credential-cache--daemon.c:180:37: warning: incorrect type in argument 2 (invalid types)
>>>   $ sparse --version
>>>   v0.5.0
>>
>> Yeah, that version of sparse is a bit too old.
>>
>> If memory serves (it may not), all of the 'argument 2 (invalid types)'
>> errors are caused by the glibc headers using a 'transparent union' to
>> define the 'struct sockaddr' type. sparse could not handle that until
>> commit 7698bae699 (aka. v0.5.0-5-g7698bae). The only remaining warning
>> was addressed by commit bcfe020ed9 (aka. v0.5.1-rc1-22-gbcfe020) in
>> sparse and commit 54360a1956 in git.
>>
>> So, it seems you need at least v0.5.2 of sparse on your Linux system
>> (which can't be too recent, or you would need v0.6.0).
> 
> The latest Linux image available on Travis CI is based on Ubuntu 16.04
> LTS, which contains sparse 0.5.0, while their default image is based
> on 14.04, with sparse 0.4.5-rc1.  Even the latest Ubuntu LTS is only
> at 0.5.1.
> 
>   https://travis-ci.org/szeder/git/jobs/488113660#L487
>   https://travis-ci.org/szeder/git/jobs/488113661#L208
> 

Indeed.

[This is why I build sparse from source! :-D ]

With a bit of luck, v0.6.1 will be available soon.

ATB,
Ramsay Jones



```