[cfe-dev] False positive null pointer analysis

[Ted Kremenek]

Hi Eric,

Please file a bugzilla report.  Please attach to it the HTML file for the bug report and ideally the preprocessed file (preprocessed with clang) for the source the false positive appears.  That will allow us to reproduce the false positive (which really can depend on specific nuances of the analyzed code).

Cheers,
Ted

On Jul 30, 2010, at 8:26 AM, Eric Blake wrote:

> I believe I encountered a false positive null pointer report while
> compiling the libvirt project, using clang-2.7.5-fc13.i686 bundled with
> Fedora 13 (hopefully you can decipher the plain-text rendering of the
> html report):
> 
> enum {
>    VIR_CGROUP_CONTROLLER_CPU,
>    VIR_CGROUP_CONTROLLER_CPUACCT,
>    VIR_CGROUP_CONTROLLER_CPUSET,
>    VIR_CGROUP_CONTROLLER_MEMORY,
>    VIR_CGROUP_CONTROLLER_DEVICES,
>    VIR_CGROUP_CONTROLLER_FREEZER,
> 
>    VIR_CGROUP_CONTROLLER_LAST
> };
> 
> 478	static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,
> 479	int create, bool_Bool memory_hierarchy)
> 480	{
> 481	int i;
> 482	int rc = 0;
> 483	
> 484	VIR_DEBUG("Make group %s", group->path)virLogMessage("file."
> "../../src/util/cgroup.c", VIR_LOG_DEBUG
> , __func__, 484, 0, "Make group %s", group->path);
> 485	for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
> 	
> 1
> 	Loop condition is true. Entering loop body
> 	
> 4
> 	Loop condition is true. Entering loop body
> 	
> 7
> 	Loop condition is true. Entering loop body
> 486	char *path = NULL((void*)0);
> 487	
> 488	/* Skip over controllers that aren't mounted */
> 489	if (!group->controllers[i].mountPoint)
> 	
> 2
> 	Taking true branch
> 	
> 5
> 	Taking true branch
> 	
> 8
> 	Taking false branch
> 490	continue;
> 	
> 3
> 	Execution continues on line 485
> 	
> 6
> 	Execution continues on line 485
> 491	
> 492	rc = virCgroupPathOfController(group, i, "", &path);
> 493	if (rc < 0)
> 	
> 9
> 	Taking false branch
> 494	return rc;
> 495	
> 496	VIR_DEBUG("Make controller %s", path)virLogMessage("file."
> "../../src/util/cgroup.c", VIR_LOG_DEBUG
> , __func__, 496, 0, "Make controller %s", path);
> 497	if (access(path, F_OK0) != 0) {
> 	
> 10
> 	Taking true branch
> 498	if (!create ||
> 	
> 11
> 	Taking false branch
> 499	mkdir(path, 0755) < 0) {
> 500	rc = -errno(*__errno_location ());
> 501	VIR_FREE(path)virFree(&(path));
> 502	break;
> 503	}
> 504	if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint !=
> NULL((void*)0) &&
> 	
> 12
> 	Assuming pointer value is null
> 	
> 13
> 	Taking false branch
> 505	(i == VIR_CGROUP_CONTROLLER_CPUSET ||
> 506	STREQ(group->controllers[i].mountPoint,
> group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint)(strcmp(group->controllers[i].mountPoint,group->controllers
> [VIR_CGROUP_CONTROLLER_CPUSET].mountPoint) == 0))) {
> 507	rc = virCgroupCpuSetInherit(parent, group);
> 508	if (rc != 0) {
> 509	VIR_FREE(path)virFree(&(path));
> 510	break;
> 511	}
> 512	}
> 513	/*
> 514	* Note that virCgroupSetMemoryUseHierarchy should always be
> 515	* called prior to creating subcgroups and attaching tasks.
> 516	*/
> 517	if (memory_hierarchy &&
> 518	group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint !=
> NULL((void*)0) &&
> 519	(i == VIR_CGROUP_CONTROLLER_MEMORY ||
> 520	STREQ(group->controllers[i].mountPoint,
> group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint)(strcmp(group->controllers[i].mountPoint,group->controllers
> [VIR_CGROUP_CONTROLLER_MEMORY].mountPoint) == 0))) {
> 	
> 14
> 	Within the expansion of the macro 'STREQ':
> a
> 	Null pointer passed as an argument to a 'nonnull' parameter
> 521	rc = virCgroupSetMemoryUseHierarchy(group);
> 522	if (rc != 0) {
> 523	VIR_FREE(path)virFree(&(path));
> 524	break;
> 525	}
> 526	}
> 527	}
> 528	
> 529	VIR_FREE(path)virFree(&(path));
> 530	}
> 531	
> 532	return rc;
> 533	}
> 
> 
> But the complaint at point 14 is invalid - it is calling strcmp on
> group->controllers[i].mountPoint (guaranteed non-NULL, due to line
> 489-490 earlier in the function) and on group->controllers
> [VIR_CGROUP_CONTROLLER_MEMORY].mountPoint (guaranteed non-NULL, due to
> line 518 earlier in the conditional).
> 
> I'm thinking that the 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; once you re-introduce a bogus assumption
> at point 12, that would explain the complaint at point 14.
> 
> -- 
> Eric Blake   eblake at redhat.com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev