Re: Clang 3.3 Analyzer Results

On Tue, Nov 12, 2013 at 3:25 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-11-12 15:17:18 -0500, Jeffrey Walton wrote:
>> On Tue, Nov 12, 2013 at 9:38 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > ...
>> > One thought for the Clang people is that most of the reports such as "null
>> > pointer dereference" presumably mean "I think I see an execution path
>> > whereby we could get here with a null pointer". If so, it'd be awfully
>> > helpful if the complaint included some description of what that path is.
>> > I think Coverity does that, or at least I've seen output from some tool
>> > that does it.
>> Clang can be trained with asserts.
>
> It might not recognize our Assert() because it expands as:
> #define TrapMacro(condition, errorType) \
> ((bool) ((! assert_enabled) || ! (condition) || \
> (ExceptionalCondition(CppAsString(condition), (errorType), \
> __FILE__, __LINE__), 0)))
>
> #define Assert(condition) \
> Trap(!(condition), "FailedAssertion")
>
> Kevin, perhaps it reports less errors if you remove the assert_enabled
> check from TrapMacro? I guess you already compiled with --enable-cassert?
Also see http://clang-analyzer.llvm.org/annotations.html (ignore the
OS X specific stuff). There's a couple of ways to annotate source code
and custom asserts. In this case, a `noreturn` annotation will
probably do the trick.

You can even guard it under the Clang analyzer (notwithstanding the
opinions of polluting source code with #define):

#ifdef __clang_analyzer__
// Code to be analyzed or modified
#endif

>> Or, you could check it for NULL and fail the function if the param is
>> NULL. If its a spurious test, then the optimizer will remove it.
>
> Only in the case it can prove that it's redundant - and in that case the
> analyzer presumably wouldn't have reported the error in the first place.
:)

Jeff