static analyser reports null dereference when it clearly cannot happen

[Quuxplusone]

Bugzilla Link	PR15679
Status	NEW
Importance	P normal
Reported by	Greg Jaskiewicz (gryzman@gmail.com)
Reported on	2013-04-04 18:05:15 -0700
Last modified on	2013-04-09 12:56:26 -0700
Version	unspecified
Hardware	PC All
CC	gryzman@gmail.com, jrose@belkadan.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments	formatting.c (130514 bytes, application/octet-stream) formatting.all.c.bz2 (59921 bytes, application/x-bzip2) fe-lobj.all.c.bz2 (23669 bytes, application/x-bzip2)
Blocks
Blocked by
See also

Created attachment 10291
formatting.c from postgresql project

checker-272 on mac

fe-lobj.c from postgresql got flagged (attaching the result file and source
file).
But it clearly is visible in the same source file, that the condition could
never work out ! Clearly clang is unable to figure it out, and throws in loads
of false positives.

In short this looks like so:

void some_funct(struct f *foo)
{
  if (foo==null || foo->bar==0)
  {

    if (initialise_f(foo) < 0)
      return;
  }

  // this gets flagged as dereference of null pointer if foo is null
  do_something(foo->bar);
}

int initialise_f(struct f* foo)
{
  if (!foo)
  {
    return -1'
  }

// ....
}

so in the example above (and attached code, look at function lo_tell64 and
lo_initialise) - this condition (dereference of null pointer) cannot happen.
Yet clang insists it does. After dozens of reports like that - and wasted time
on my pard - I decided to bug it here.

[Quuxplusone]

Thanks, Grzegorz. Can you attach a preprocessed file, so that we don't have to worry about configuration differences here?

[Quuxplusone]

Hm. I don't see this with checker-272 or with trunk on the preprocessed file. How did you run scan-build over this project?

[Quuxplusone]

This is what I got. Looking at the code, I found this very much not plausible http://cl.ly/image/0z1p2C153b3n

I simply used scan-build ./configure and then scan-build make -j 9 (8 cores on this mbpr) .

[Quuxplusone]

Ah, you said the bug is in fe-lobj.c, but the source and preprocessed file come from formatting.c.

[Quuxplusone]

Oh dear. That's what happens when you answer two or three emails at the same
time, and trying to fix few bugs at the same time.
Sorry mate :/

[Quuxplusone]

Attached formatting.c (130514 bytes, application/octet-stream): formatting.c from postgresql project

[Quuxplusone]

Attached formatting.all.c.bz2 (59921 bytes, application/x-bzip2): preprocessed formatting.c

[Quuxplusone]

Attached fe-lobj.all.c.bz2 (23669 bytes, application/x-bzip2): preprocessed fe-lobj.c

[Quuxplusone]

It looks like lo_initialize is too big (has too many stacked branches) for the
analyzer to reason about. It's a little unfortunate that the null check and
early return get thrown out with that, but the analyzer has to guess whether
modeling a particular call is worth the extra time spent in analysis, and in
this case it doesn't know that there's a cheap early return in there that
produce better results.

Workaround:

if (foo == null)
  return;
if (foo->bar == null)
  if (initialize(foo) < 0)
    return;

[Quuxplusone]

Can I configure clang to go on and do it ?
This code doesn't look very complicated, and in any case - I want to go on and
analyse everything I throw at it. That's its job. ;)