Bill Allombert on Wed, 16 Sep 2009 12:46:42 +0200


[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

Re: Static analyzer run


On Wed, Sep 16, 2009 at 06:00:49AM +0200, Lorenz Minder wrote:
> Hi,
> 
> > Can you tell the analyzer that pari_err does not return ?
> > This is a large source of false positive with gcc.
> 
> Hmm, isn't it gcc-clean at the moment?   In any case, maybe we should

It is, but only due to the addition of lot of useless return NULL;

> declare pari_err() with __attribute__((noreturn)).  That would
> presumably help both gcc and clang (whose analyzer I used to find these
> problems).
> 
> I've had no time yet to look into how to make such a change in
> a portable manner.

Yes... Karim objection that this would non-gcc compilers to output contractory
warning from gcc compilers, and we could not shut them both down:

An example:

GEN foo()
{
  ...
  pari_err(talker,"...");
  return NULL;
}

With __attribute__((noreturn)):
  Warning statement not reached:
  return NULL;
Without __attribute__((noreturn)) and without "return NULL"
  Warning: function foo does not return.

> Ok, thanks for those assessments.  I hope I'll get around looking at
> some of the ones that probably aren't false at some point. (Most of
> them are in code I don't understand and therefore won't touch, though.)

Unfortunately I am sometimes in the same situation as you see...

> BTW, I accidentally stumbled over a problem in the SEA module today.
> Can you check the attached patch?

Commited in revision 11921. I find very unfortunate that gerepileall 
take an explicit argument number. I was hopping for some preprocessing
trickery to avoid that, but I found none. We should probably change it
to a NULL-terminated list instead, though this would cause other problems
with argument accidentally equal to NULL.

Cheers,
Bill.