Mailing List Archive: Making the CVS tree pass the -Wall test...

Making the CVS tree pass the -Wall test...

Jul 4, 2000, 10:45 AM

Post #1 of 8 (337 views)

While fiddling around with the _GNU_SOURCE define I found
that there are several warnings left in the current CVS
version which should be removed, I guess.

Here's the list I get from compiling the tree using -Wall
grouped by C file:

bufferobject.c: In function `PyBuffer_New':
bufferobject.c:174: warning: value computed is not used

classobject.c: In function `instance_dealloc':
classobject.c:526: warning: unused variable `_Py_RefTotal'

object.c: In function `_PyTrash_deposit_object':
object.c:1172: warning: unused variable `hold'
object.c:1171: warning: `typecode' might be used uninitialized in this function

ceval.c: In function `eval_code2':
ceval.c:333: warning: `opcode' might be used uninitialized in this function
ceval.c:334: warning: `oparg' might be used uninitialized in this function

pystate.c: In function `PyInterpreterState_New':
pystate.c:45: warning: value computed is not used

./gcmodule.c: In function `debug_cycle':
./gcmodule.c:295: warning: unsigned int format, long unsigned int arg (arg 5)
./gcmodule.c: In function `collect':
./gcmodule.c:374: warning: int format, long int arg (arg 3)
./gcmodule.c:374: warning: int format, long int arg (arg 4)
./gcmodule.c:374: warning: int format, long int arg (arg 5)
./gcmodule.c:438: warning: int format, long int arg (arg 3)
./gcmodule.c:438: warning: int format, long int arg (arg 4)
./gcmodule.c: In function `collect_generations':
./gcmodule.c:458: warning: `n' might be used uninitialized in this function

./pypcre.c: In function `compile_branch':
./pypcre.c:1222: warning: `c' might be used uninitialized in this function
./pypcre.c: In function `pcre_exec':
./pypcre.c:4524: warning: variable `start_bits' might be clobbered by `longjmp' or `vfork'
./pypcre.c:4525: warning: variable `start_match' might be clobbered by `longjmp' or `vfork'

./posixmodule.c: At top level:
./posixmodule.c:496: warning: `posix_strint' defined but not used

_sre.c: In function `pattern_match':
_sre.c:819: warning: `mark1' might be used uninitialized in this function
_sre.c:819: warning: `mark0' might be used uninitialized in this function
./_sre.c:819: warning: `mark1' might be used uninitialized in this function
./_sre.c:819: warning: `mark0' might be used uninitialized in this function
_sre.c: In function `pattern_search':
_sre.c:985: warning: `prefix_len' might be used uninitialized in this function
_sre.c:819: warning: `mark0' might be used uninitialized in this function
[...many more of these... all referencing line 819]

In file included from ./mmapmodule.c:24:
../Include/Python.h:18: warning: `_GNU_SOURCE' redefined
./mmapmodule.c:21: warning: this is the location of the previous definition
[...looks like someone needed _GNU_SOURCE too ;-)...]

./parsermodule.c: In function `validate_arglist':
./parsermodule.c:2089: warning: `ok' might be used uninitialized in this function

--
Marc-Andre Lemburg
______________________________________________________________________
Business: http://www.lemburg.com/
Python Pages: http://www.lemburg.com/python/

Re: Making the CVS tree pass the -Wall test... [ In reply to ]

fdrake at beopen

Jul 4, 2000, 11:50 AM

Post #2 of 8 (324 views)

Permalink

M.-A. Lemburg writes:
> Here's the list I get from compiling the tree using -Wall
> grouped by C file:

Feel free to fix these! (I agree they should be done, but I need to
pull away from email and these little proto-buglets so I can actually
get done some of the things on my list. ;)

> ./parsermodule.c: In function `validate_arglist':
> ./parsermodule.c:2089: warning: `ok' might be used uninitialized in this function

Ok, I've fixed this one, since it's probably my fault. ;)

-Fred

--
Fred L. Drake, Jr. <fdrake at beopen.com>
BeOpen PythonLabs Team Member

Re: Making the CVS tree pass the -Wall test... [ In reply to ]

mal at lemburg

Jul 4, 2000, 12:00 PM

Post #3 of 8 (329 views)

Permalink

"Fred L. Drake, Jr." wrote:
>
> M.-A. Lemburg writes:
> > Here's the list I get from compiling the tree using -Wall
> > grouped by C file:
>
> Feel free to fix these! (I agree they should be done, but I need to
> pull away from email and these little proto-buglets so I can actually
> get done some of the things on my list. ;)

Same here ;-)

--
Marc-Andre Lemburg
______________________________________________________________________
Business: http://www.lemburg.com/
Python Pages: http://www.lemburg.com/python/

RE: Making the CVS tree pass the -Wall test... [ In reply to ]

tim_one at email

Jul 4, 2000, 10:51 PM

Post #4 of 8 (323 views)

Permalink

[M.-A. Lemburg]
> While fiddling around with the _GNU_SOURCE define I found
> that there are several warnings left in the current CVS
> version which should be removed, I guess.
>
> Here's the list I get from compiling the tree using -Wall
> grouped by C file:

So long as any warnings remain, would you please repost the list from time
to time? There's no need to group them by file.

> ./pypcre.c:4524: warning: variable `start_bits' might be
> clobbered by `longjmp' or `vfork'
> ./pypcre.c:4525: warning: variable `start_match' might be
> clobbered by `longjmp' or `vfork'

These are often a PITA to fix: gcc does brain-dead flow analysis, and this
particular msg is almost always bogus as a result. There are several open
bugs filed against gcc on this (I know because one of them is mine <wink>).
The others look straightforward to fix (on a first scan).

Making the CVS tree pass the -Wall test... [ In reply to ]

martin at loewis

Jul 4, 2000, 11:50 PM

Post #5 of 8 (324 views)

Permalink

>> ./pypcre.c:4524: warning: variable `start_bits' might be
>> clobbered by `longjmp' or `vfork'
>> ./pypcre.c:4525: warning: variable `start_match' might be
>> clobbered by `longjmp' or `vfork'

> These are often a PITA to fix: gcc does brain-dead flow analysis, and this
> particular msg is almost always bogus as a result. There are several open
> bugs filed against gcc on this (I know because one of them is mine <wink>).
> The others look straightforward to fix (on a first scan).

At least for start_match, what exactly is bogus about this message?

setjmp is invoked in line 4691 of pypcre, and start_match is modified
in line 4737, at the end of the while loop containing the setjmp
call. If somebody would invoke longjmp on the jmp_buf after
start_match is modified, it would have an indeterminate value,
according to 7.13.2.1p3 of C99.

Likewise for start_bits, as it may also be modified after setjmp() has
been called, since it appears in the same loop. I admit that there
wouldn't be too many calls that could actually invoke longjmp() before
the next invocation of setjmp, except perhaps for pchars(), but gcc is
not supposed to determine potential callers of longjmp:

The compiler sees only the calls to `setjmp'. It cannot know
where `longjmp' will be called; in fact, a signal handler
could call it at any point in the code. As a result, you may
get a warning even when there is in fact no problem because
`longjmp' cannot in fact be called at the place which would
cause a problem.

So while I'd admit that gcc "does brain-dead flow analysis", I think
this particular msg is not bogus in this specific case. It's more
often than not that gcc users fail to understand this message, though.

Regards,
Martin

Re: Making the CVS tree pass the -Wall test... [ In reply to ]

mal at lemburg

Jul 5, 2000, 12:55 AM

Post #6 of 8 (324 views)

Permalink

Tim Peters wrote:
>
> [M.-A. Lemburg]
> > While fiddling around with the _GNU_SOURCE define I found
> > that there are several warnings left in the current CVS
> > version which should be removed, I guess.
> >
> > Here's the list I get from compiling the tree using -Wall
> > grouped by C file:
>
> So long as any warnings remain, would you please repost the list from time
> to time? There's no need to group them by file.

Ok.

Note that the idea behind posting the list was to make
module authors/editors aware of warnings in their code... Fred
already made his changes -- it would probably be a good idea
if the others did the same.

> > ./pypcre.c:4524: warning: variable `start_bits' might be
> > clobbered by `longjmp' or `vfork'
> > ./pypcre.c:4525: warning: variable `start_match' might be
> > clobbered by `longjmp' or `vfork'
>
> These are often a PITA to fix: gcc does brain-dead flow analysis, and this
> particular msg is almost always bogus as a result. There are several open
> bugs filed against gcc on this (I know because one of them is mine <wink>).
> The others look straightforward to fix (on a first scan).

That one has been in Python since pcre was adopted -- I don't
think there's much we can do about it either... but then: sre
is coming on strong so we might as well drop pcre for say 2.1.

--
Marc-Andre Lemburg
______________________________________________________________________
Business: http://www.lemburg.com/
Python Pages: http://www.lemburg.com/python/

RE: Making the CVS tree pass the -Wall test... [ In reply to ]

tim_one at email

Jul 5, 2000, 2:05 AM

Post #7 of 8 (326 views)

Permalink

>> ./pypcre.c:4524: warning: variable `start_bits' might be
>> clobbered by `longjmp' or `vfork'
>> ./pypcre.c:4525: warning: variable `start_match' might be
>> clobbered by `longjmp' or `vfork'

[.Tim sez these are almost always bogus wngs, Martin sez these two are
legit]

The start_bits wng is bogus because start_bit's only definition points
precede the do-loop: start_bits will either be NULL or extra->start_bits by
the time the "do" is entered, and can never change after that (yes,
start_bits "appears" in the loop, but not as an lvalue -- it's only
referenced). So the value setjmp tucks away is necessarily the correct
value for longjmp to restore. This is a purely local flow question;
knowledge of when a longjmp may occur isn't needed.

I agree the start_match wng is legit. In my previous life, I tracked down
about a hundred of these, and found only one that was legit. So pypcre is
as sick as Dragon's half-million lines of speech recognition code <wink>.

"The usual" way to fix this is to slop "volatile" onto the declarations, as
the author already did for three other locals. That's the second reason
this warning sucks: it depends on which locals gcc decides to carry in
registers, and that varies from release to release. And that's the third
reason this warning sucks: gcc is actually complaining about its *own*
decision to carry something in a register <0.1 wink -- and if it *knows*
it's not safe to carry something in a register, it shouldn't!>.

RE: Making the CVS tree pass the -Wall test... [ In reply to ]

tim_one at email

Jul 5, 2000, 2:07 AM

Post #8 of 8 (322 views)

Permalink

[MAL]
> While fiddling around with the _GNU_SOURCE define I found
> that there are several warnings left in the current CVS
> version which should be removed, I guess.

[Tim]
> So long as any warnings remain, would you please repost the
> list from time to time? There's no need to group them by file.

[MAL]
> Ok.
>
> Note that the idea behind posting the list was to make
> module authors/editors aware of warnings in their code... Fred
> already made his changes -- it would probably be a good idea
> if the others did the same.

Of course. That's why I'm asking you to repost this list until no wngs
remain <wink>.

one-time-implicit-nags-don't-work-ly y'rs - tim