Re: clang's static checker report.

On Sun, Aug 30, 2009 at 3:26 PM, Peter Eisentraut<peter_e(at)gmx(dot)net> wrote:
> On lör, 2009-08-29 at 17:35 +0100, Greg Stark wrote:
>> We still have things like this showing "division by zero":
>>
>> Assert(activeTapes > 0);
>> 1913          slotsPerTape = (state->memtupsize - state->mergefirstfree) / activeTapes;
>>
>>
>> It looks like if you marked ExceptionalCondition() as never returning
>> then it should hide this.
>
> Well, if you can disable the assertion, then there is still a possible
> bug here, no?

Yeah, the problem is that clang doesn't know our rep invariants and
inter-procedure call signature guarantees. Ie, activeTapes here is
initialized to the count of tapes in the tuplesort state when a merge
begins. Clang doesn't know that we never call beginmerge on a
tuplesort that has no active tapes.

So going on the assumption that our Asserts indicate somebody actually
thought about the case they cover and checked that it's a reasonable
assumption... then we don't need clang to tell us about them.

I think most of the remaining false positives are cases where palloc,
palloc0, repalloc, MemoryContextAlloc, or MemoryContextAllocZero
return values are deferenced. Clang doesn't know that these functions
never return NULL so it's marking every case as a possible NULL
dereference.

--
greg
http://mit.edu/~gsstark/resume.pdf