Re: [PATCH] usage: add NORETURN to BUG() function definitions

2 messages, 2 authors, 2017-05-23 · open the first message on its own page

Re: [PATCH] usage: add NORETURN to BUG() function definitions

From: Junio C Hamano <hidden>
Date: 2017-05-23 03:33:04

Ramsay Jones [off-list ref] writes:
Having said that, I rarely run sparse over just one file (except
when fixing a sparse error/warning). On each branch (master->next->pu)
I do

    $ make sparse >sp-out 2>&1 # nsp-out on 'next', psp-out on 'pu'

... so that I can diff the files from branch to branch. (I check the
master branch file by hand. There is a single warning on Linux that
is actually a sparse problem).

Just FYI, for today's fetch:

    $ diff sp-out nsp-out
    $ diff nsp-out psp-out
    12a13
    >     SP blame.c
    42a44,46
    > diff.c:813:6: warning: symbol 'emit_line' was not declared. Should it be static?
    > diff.c:828:6: warning: symbol 'emit_line_fmt' was not declared. Should it be static?
    > diff.c:1865:6: warning: symbol 'print_stat_summary_0' was not declared. Should it be static?
    54a59
    >     SP fsmonitor.c
    137a143
    >     SP sub-process.c
    170a177
    >     SP compat/fopen.c
    276a284
    > builtin/worktree.c:539:38: warning: Using plain integer as NULL pointer
    296a305
    >     SP t/helper/test-dir-iterator.c
    $ 
Interesting.  One thing that I found somewhat suboptimal is that we
do not get signalled by non-zero exit.  Otherwise it would make a
good addition to the "Static Analysis" task in .travis.yml file.

Re: [PATCH] usage: add NORETURN to BUG() function definitions

From: Ramsay Jones <hidden>
Date: 2017-05-23 20:48:17


On 23/05/17 04:32, Junio C Hamano wrote:
Interesting.  One thing that I found somewhat suboptimal is that we
do not get signalled by non-zero exit.
Warnings don't lead to non-zero exit, but similarly to -Werror, you can
provide a -Wsparse-error to turn warnings into errors:

  $ make builtin/worktree.sp
      SP builtin/worktree.c
  builtin/worktree.c:539:38: warning: Using plain integer as NULL pointer
  $ 
  $ make SPARSE_FLAGS=-Wsparse-error builtin/worktree.sp
      SP builtin/worktree.c
  builtin/worktree.c:539:38: error: Using plain integer as NULL pointer
  Makefile:2370: recipe for target 'builtin/worktree.sp' failed
  make: *** [builtin/worktree.sp] Error 1
  $ 

Unfortunately, that does not help too much because, as I mentioned before,
one warning is actually a sparse problem (and you can't turn it off):

  $ make pack-revindex.sp
      SP pack-revindex.c
  pack-revindex.c:64:23: warning: memset with byte count of 262144
  $ 

This is caused by sparse _unconditionally_ complaining about the byte count
used in calls to memset(), memcpy(), copy_to_user() and copy_from_user().
In addition, the byte count limits are hard-coded (v <= 0 || v > 100000).
About a decade ago, I wrote a patch to enable/set the limit value from the
command line, but didn't get around to sending the patch upstream. :-D
   
[There is actually another problem warning, if you build with NO_REGEX=1].

Since cgcc was intended to be used as proxy for gcc, you might think you
could use CC=cgcc on a regular build, but that has problems of it's own:

  $ make clean >/dev/null 2>&1  # on 'pu' branch, build output in 'pout'
  $ make CC=cgcc >pout1 2>&1
  $ diff pout pout1
  99a100
  > pack-revindex.c:64:23: warning: memset with byte count of 262144
  199a201
  > imap-send.c:1439:9: warning: expression using sizeof on a function
  200a203,207
  > http.c:675:9: warning: expression using sizeof on a function
  > http.c:1676:25: warning: expression using sizeof on a function
  > http.c:1681:25: warning: expression using sizeof on a function
  > http.c:2082:9: warning: expression using sizeof on a function
  > http.c:2249:9: warning: expression using sizeof on a function
  219a227
  > http-walker.c:377:9: warning: expression using sizeof on a function
  222a231,233
  > http-push.c:189:9: warning: expression using sizeof on a function
  > http-push.c:200:9: warning: expression using sizeof on a function
  > http-push.c:202:9: warning: expression using sizeof on a function
  228a240,243
  > remote-curl.c:524:9: warning: expression using sizeof on a function
  > remote-curl.c:605:17: warning: expression using sizeof on a function
  > remote-curl.c:608:17: warning: expression using sizeof on a function
  > remote-curl.c:676:9: warning: expression using sizeof on a function
  374a390
  > builtin/worktree.c:539:38: warning: Using plain integer as NULL pointer
  
  ...

  $ 

See commit 9371322a60 (sparse: suppress some "using sizeof on a function"
warnings, 06-10-2013) for an explanation of the additional warnings.
I chose the SPARSE_FLAGS method to suppress those warnings, precisely
because I don't build git that way. (git grep -n SPARSE_FLAGS).

So, using CC='cgcc -Wsparse-error' as it stands isn't much help:
  
  $ make clean >/dev/null 2>&1
  $ make CC='cgcc -Wsparse-error'
  GIT_VERSION = 2.13.0.530.g896b4ae59
      * new build flags
      CC credential-store.o
      * new link flags
      CC common-main.o
 
  ...
 
      CC pack-objects.o
      CC pack-revindex.o
  pack-revindex.c:64:23: error: memset with byte count of 262144
  Makefile:2036: recipe for target 'pack-revindex.o' failed
  make: *** [pack-revindex.o] Error 1
  $ 
                                        Otherwise it would make a
good addition to the "Static Analysis" task in .travis.yml file.
Unfortunately, some additional work required. :-P

ATB,
Ramsay Jones

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help