[cfe-dev] Bug: Unexpected dereference claim on --analyze

[Jan Engelhardt]

On Tuesday 2011-12-06 03:19, Ted Kremenek wrote:
>On Dec 5, 2011, at 4:43 PM, Jeffrey Yasskin wrote:
>
>>Klocwork has a similar heuristic, and it proved obnoxious in
>>practice. ~75%-90% of the time, the real problem in the code was
>>that someone was checking against 0, but the pointer was actually
>>guaranteed not to be 0, so the check was redundant. Unless the
>>analyzer has other evidence that the pointer may actually be 0, it
>>should treat "dereference null" and "redundant check" as similar
>>likelihood and include both in the warning, or omit the warning.
>
>Excellent points. I think the heuristic is still valuable,
>however. Either the pointer could be null, and thus result in a null
>dereference, or the logic is redundant, and thus the invariants of
>the code aren't well documented or well understood. [...] Do you
>think it would be reasonable to still emit a warning here, but
>categorize it differently in the places where we are less certain
>that the issue is a null dereference?

Seeing that this little code piece made for such a discussion, I am
tempted to post a slightly bigger testcase, to put this into light.

In this one, the warning only occurs with -DWITH_HACK, and that seems 
right to me, following Annas statement

|The analyzer tries to flag the issues which most likely are bugs.

and one can most likely argue that the stab-through cast is a danger 
zone.
Or maybe clang can be made to understand this trick?

====
/*
SUSE Linux clang version 3.0 (branches/release_30 142912) (based on LLVM 3.0)

clang --analyze this
*/
#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
enum dir {
	LEFT = 0,
	RIGHT,
};
struct node {
	struct node *child;
	char color;
};
struct tree {
	struct node *root;
};
#define MAXDEP 48
static void add(struct tree *tree)
{
	struct node *node, *path[MAXDEP];
	unsigned int depth = 0;

#ifdef WITH_HACK
	assert(sizeof(struct node *) <= sizeof(struct node **));
	/* known to be hack, but simplifies root replacement.
	 * Requires that .child is the first element of the struct,
	 * i.e. has the same address as the struct itself. */
	path[depth++] = (struct node *)&tree->root;
#endif
	node = tree->root;

	while (node != NULL) {
		if (depth == MAXDEP)
			abort();
		path[depth++] = node;
		node = node->child;
	}

	if ((node = malloc(sizeof(struct node))) == NULL)
		abort();
#ifdef WITH_HACK
	path[depth-1]->child = node;
#else
	if (depth == 0)
		tree->root = node;
	else
		path[depth-1]->child = node;
#endif
	/* There will always be a root now. Either there was one previously,
	 * or path[0]->child just set one. */
	tree->root->color = 0;
}