llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.27k stars 11.68k forks source link

false positive null pointer analysis #8130

Open llvmbot opened 14 years ago

llvmbot commented 14 years ago
Bugzilla Link 7758
Version 2.7
OS Linux
Attachments html report from clang, with false claim of a null dereference, preprocessed file that triggered the false analysis
Reporter LLVM Bugzilla Contributor
CC @tkremenek

Extended Description

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.

tkremenek commented 14 years ago

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.

llvmbot commented 14 years ago

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.

tkremenek commented 14 years ago

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.

tkremenek commented 14 years ago

GraphViz visualization of trimmed exploded graph

llvmbot commented 14 years ago

assigned to @tkremenek