false positive null pointer analysis

[Quuxplusone]

Bugzilla Link	PR7758
Status	NEW
Importance	P normal
Reported by	Eric Blake (eblake@redhat.com)
Reported on	2010-07-30 11:45:56 -0700
Last modified on	2013-03-24 09:20:14 -0700
Version	2.7
Hardware	PC Linux
CC	kremenek@apple.com, llvm-bugs@lists.llvm.org, michaelacrawford@me.com
Fixed by commit(s)
Attachments	report-oLS9I0.html (151301 bytes, text/html) cgroup.i (209015 bytes, text/plain) TrimmedGRExprEngine.pdf (182866 bytes, application/pdf)
Blocks
Blocked by
See also

Created attachment 5289
html report from clang, with false claim of a null dereference

Using scan-build from clang-2.7-5.fc13.i686 on Fedora 13 to compile libvirt, I
found a false positive.  The attached html file claims at step 14 that the code
is passing a potentially NULL argument to strcmp.  However, that claim is
invalid.

At point 14, the two arguments to strcmp are group->controllers[i].mountPoint
(guaranteed non-NULL, due to line 489-490 earlier in the function) and group-
>controllers [VIR_CGROUP_CONTROLLER_MEMORY].mountPoint (guaranteed non-NULL,
due to line 518 earlier in the same conditional).

I'm wondering ifthe clang analyzer is getting confused when the iteration hits
i == 3 == VIR_CGROUP_CONTROLLER_CPUSET, and failing to realize that the
assumption of point 12 of the analysis (assuming that group-
>controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint is NULL) was already
disproved at point 8 (group->controllers[i].mountPoint is non-NULL); once you
re-introduce a bogus assumption at point 12, that would explain the complaint
at point 14.

I'm also attaching the preprocessed input that the html report was generated
from.

[Quuxplusone]

Attached report-oLS9I0.html (151301 bytes, text/html): html report from clang, with false claim of a null dereference

[Quuxplusone]

Attached cgroup.i (209015 bytes, text/plain): preprocessed file that triggered the false analysis

[Quuxplusone]

Attached TrimmedGRExprEngine.pdf (182866 bytes, application/pdf): GraphViz visualization of trimmed exploded graph

[Quuxplusone]

The culprit is:

492 rc = virCgroupPathOfController(group, i, "", &path);

Here 'group' is passed to virCgroupPathOfController. Because the analyzer doesn't know the effects of that function, it assumes that all the fields pointed to by 'group' can be modified, and thus invalidates any assumptions. Without inter-procedural analysis or a more relaxed heuristic, there is no way for the analyzer to know that the fields are unmodified.

[Quuxplusone]

Thanks. I suppose marking the argument as a const pointer (if that doesn't have knock-on effects) or adding an assert that the parameter is still null after the function will be enough information to feed the analyzer enough additional information that the function call did not change the groups pointer.

[Quuxplusone]

(In reply to comment #4)
> Thanks.  I suppose marking the argument as a const pointer (if that doesn't
> have knock-on effects) or adding an assert that the parameter is still null
> after the function will be enough information to feed the analyzer enough
> additional information that the function call did not change the groups
> pointer.

Adding 'const' doesn't appear to do the trick.  That's something we should fix.
Adding the assertion should work.