Problem report #8

Problem report #8

[Dan Nicolaescu]

I signed up to see the Coverity problem reports. 
RMS asked me to post a few here.
There are 76 problems, I analyzed 10 of them, none of them were bugs
in emacs.

Hopefully more people can help with this.

If you analyze the problem, please add to the subject the resolution
which can be one of the following:

Status:
UNINSPECTED
BUG
FALSE
RESOLVED
IGNORE
PENDING

I will then put this on the website. Or better mark it there directly
if you have access.

ERROR
CID: 8
Checker: DEADCODE (help)
File: emacs/src/xfns.c
Function: Fx_window_property
Description: After this line, the value of "actual_format" is equal to 32

4327 	      if (rc == Success && tmp_data)
4328 	        {
4329 	          /* The man page for XGetWindowProperty says:
4330 	             "If the returned format is 32, the returned data is represented
4331 	             as a long array and should be cast to that type to obtain the
4332 	             elements."
4333 	             This applies even if long is more than 32 bits, the X library
4334 	             converts from 32 bit elements received from the X server to long
4335 	             and passes the long array to us.  Thus, for that case bcopy can not
4336 	             be used.  We convert to a 32 bit type here, because so much code
4337 	             assume on that.
4338 	
4339 	             The bytes and offsets passed to XGetWindowProperty refers to the
4340 	             property and those are indeed in 32 bit quantities if format is
4341 	             32.  */
4342 	

Event dead_error_condition: On this path, the condition "actual_format < 32" could not be true
Event new_values: Conditional "actual_format == 32"
Event const: After this line, the value of "actual_format" is equal to 32
Also see events: [dead_error_begin][new_values][const]

4343 	          if (actual_format == 32 && actual_format < BITS_PER_LONG)
4344 	            {
4345 	              unsigned long i;

Event dead_error_begin: Cannot reach dead code beginning here
Also see events: [dead_error_condition][new_values][const]

Re: Problem report #8

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> I signed up to see the Coverity problem reports. 
> RMS asked me to post a few here.
> There are 76 problems, I analyzed 10 of them, none of them were bugs
> in emacs.

Just to be clear: are the bug reports you posted to this list the ones
that you found to be not bugs, or the ones that you have not analyzed?

Re: Problem report #8

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > > I signed up to see the Coverity problem reports. 
  > > RMS asked me to post a few here.
  > > There are 76 problems, I analyzed 10 of them, none of them were bugs
  > > in emacs.
  > 
  > Just to be clear: are the bug reports you posted to this list the ones
  > that you found to be not bugs, or the ones that you have not analyzed?

The are the ones that I did not analyze. 

Re: Problem report #8

[Richard Stallman]

      > > There are 76 problems, I analyzed 10 of them, none of them were bugs
      > > in emacs.

      > Just to be clear: are the bug reports you posted to this list the ones
      > that you found to be not bugs, or the ones that you have not analyzed?

    The are the ones that I did not analyze. 

It sounds like there are 66 you did not analyze.
I suggested you post just SOME of the ones not analyzed,
and it looks like that's what you did.  So there would be
many others not yet analyzed which you have not posted.

Could you please confirm that?

Re: Problem report #8

[Dan Nicolaescu]

Richard Stallman <rms@gnu.org> writes:

  >       > > There are 76 problems, I analyzed 10 of them, none of them were bugs
  >       > > in emacs.
  > 
  >       > Just to be clear: are the bug reports you posted to this list the ones
  >       > that you found to be not bugs, or the ones that you have not analyzed?
  > 
  >     The are the ones that I did not analyze. 
  > 
  > It sounds like there are 66 you did not analyze.

Correct. 

  > I suggested you post just SOME of the ones not analyzed,
  > and it looks like that's what you did.  So there would be
  > many others not yet analyzed which you have not posted.
  > 
  > Could you please confirm that?

Right.

Re: Problem report #8 FALSE

[Jan D.]

> ERROR
> CID: 8
> Checker: DEADCODE (help)
> File: emacs/src/xfns.c
> Function: Fx_window_property
> Description: After this line, the value of "actual_format" is equal to 32
> 
> 4327 	      if (rc == Success && tmp_data)
> 4328 	        {
> 4329 	          /* The man page for XGetWindowProperty says:
> 4330 	             "If the returned format is 32, the returned data is represented
> 4331 	             as a long array and should be cast to that type to obtain the
> 4332 	             elements."
> 4333 	             This applies even if long is more than 32 bits, the X library
> 4334 	             converts from 32 bit elements received from the X server to long
> 4335 	             and passes the long array to us.  Thus, for that case bcopy can not
> 4336 	             be used.  We convert to a 32 bit type here, because so much code
> 4337 	             assume on that.
> 4338 	
> 4339 	             The bytes and offsets passed to XGetWindowProperty refers to the
> 4340 	             property and those are indeed in 32 bit quantities if format is
> 4341 	             32.  */
> 4342 	
> 
> Event dead_error_condition: On this path, the condition "actual_format < 32" could not be true
> Event new_values: Conditional "actual_format == 32"
> Event const: After this line, the value of "actual_format" is equal to 32
> Also see events: [dead_error_begin][new_values][const]
> 
> 4343 	          if (actual_format == 32 && actual_format < BITS_PER_LONG)
> 4344 	            {

It can not be < 32 if BITS_PER_LONG is 32, that is true.  But for a 64-bit CPU
BITS_PER_LONG is 64, so then it becomes relevant.  It is only intended
to be relevant for the BITS_PER_LONG == 64 case.

I think gcc optimizes the code away if it finds that
actual_format < BITS_PER_LONG can never be true.

	Jan D.

Re: Problem report #8 FALSE

[Miles Bader]

"Jan D." <jan.h.d@swipnet.se> writes:
>> 4343 	          if (actual_format == 32 && actual_format < BITS_PER_LONG)
>> 4344 	            {
>
> It can not be < 32 if BITS_PER_LONG is 32, that is true.  But for a 64-bit CPU
> BITS_PER_LONG is 64, so then it becomes relevant.  It is only intended
> to be relevant for the BITS_PER_LONG == 64 case.
>
> I think gcc optimizes the code away if it finds that
> actual_format < BITS_PER_LONG can never be true.

Then perhaps rewriting the test as:

   if (actual_format == 32 && BITS_PER_LONG > 32)

would get rid of the warning (the checker should treat comparisons of
constants with constants as being uninteresting, as they're almost
always due to cpp macros like this).

It would probably also virtually ensure that gcc optimizes away the
code in the 32-bit BITS_PER_LONG case.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: Problem report #8

[Kenichi Handa]

In article <200604090659.k396xI5h005539@scanner2.ics.uci.edu>, Dan Nicolaescu <dann@ics.uci.edu> writes:

> I signed up to see the Coverity problem reports. 
> RMS asked me to post a few here.
> There are 76 problems, I analyzed 10 of them, none of them were bugs
> in emacs.

> Hopefully more people can help with this.

> If you analyze the problem, please add to the subject the resolution
> which can be one of the following:

> Status:
> UNINSPECTED
> BUG
> FALSE
> RESOLVED
> IGNORE
> PENDING

Could you please explain which of the above to select in
which situation.  For instance, I think FALSE and RESOLVED
are:

FALSE -- the report was false alarm.
RESOLVED -- the report correctly found a bug and it is fixed.

But, I'm not sure about the others.

---
Kenichi Handa
handa@m17n.org

Re: Problem report #8

[Dan Nicolaescu]

Kenichi Handa <handa@m17n.org> writes:

  > In article <200604090659.k396xI5h005539@scanner2.ics.uci.edu>, Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > > I signed up to see the Coverity problem reports. 
  > > RMS asked me to post a few here.
  > > There are 76 problems, I analyzed 10 of them, none of them were bugs
  > > in emacs.
  > 
  > > Hopefully more people can help with this.
  > 
  > > If you analyze the problem, please add to the subject the resolution
  > > which can be one of the following:
  > 
  > > Status:
  > > UNINSPECTED
  > > BUG
  > > FALSE
  > > RESOLVED
  > > IGNORE
  > > PENDING
  > 
  > Could you please explain which of the above to select in
  > which situation.  For instance, I think FALSE and RESOLVED
  > are:
  > 
  > FALSE -- the report was false alarm.
  > RESOLVED -- the report correctly found a bug and it is fixed.
  > 
  > But, I'm not sure about the others.

I am not so sure either, there's no documentation for them, but here's
what I assume. 

  > > UNINSPECTED
Nobody looked at this.

  > > BUG
This is a problem, and it has not been fixed yet.

  > > IGNORE
Not worth looking at. 

  > > PENDING

Here's what this seems to be used for: 
(from the page at http://scan.coverity.com/)

 The fixed defects column is calculated by adding defects marked by
 developers as RESOLVED with defects marked as BUG or PENDING which
 are not present in the most recent analysis run. As such, this figure
 may under report fixes (for instance, when defects are fixed but not
 annotated as a defect by developers within the Coverity GUI), or over
 report fixes (for instance, when defects are annotated by developers
 in the Coverity GUI in regions of the source code which are
 subsequently removed from the project's code base).