[PATCH] Add new warning to Clang to detect when all code paths in a function has a call back to the function.

[Arthur O'Dwyer]

Really cool! I don't understand the template example, though...

    // sum<0>() is instantiated, does recursively call itself, but never runs.
    template <int value> int sum() { return value + sum<value/2>(); }
 // <== notice that your test is actually missing the (), so you've
got a bogus syntax error in there
    template<> int sum<0>() { return 1; }

You imply that Clang would get confused and think that sum<0> calls
itself... but how can that be, since sum<0> simply returns 1? Or, if
you mean that Clang would mistakenly try to instantiate the regular
("un-specialized"?) version of sum<i> with i=0... how can *that* be,
since you could just as well make it

    template <int value> int sum() { static_assert( i != 0, "" );
return value + sum<value/2>(); }

? Clang shouldn't be trying to static-analyze instantiations that
aren't real. :P
I'd like to see this patch fixed so that it works properly on
templates, because that strikes me as exactly the case where this sort
of warning would be most useful. We want to be able to distinguish the
cases

    template <int value> int sum() { return value + sum<value/2>(); }
// no warning
    template<> int sum<0>() { return 1; }
    void f() { sum<4>(); }

and

    template <int value> int sum() { return value + sum<value/2>(); }
// yes warning
    void f() { sum<4>(); }


Also, FWIW, I see no reason to special-case infinite loops.

    void f() { while (true) f(); }

is buggy enough to deserve a diagnostic, IMO. (Such a construct
probably never happens in practice, but when it *does* happen, that
one time in a million, and an engineer spends three hours tracking it
down, he'll probably be annoyed that Clang specifically considered
pointing out the problem and then silently swallowed it.)

my $.02,
–Arthur


On Tue, Oct 8, 2013 at 8:13 PM, Richard Trieu <rtrieu at google.com> wrote:
> Implement a warning to detect when a function will call itself recursively on every code path.  If a program ever calls such a function, the function will attempt to call itself until it runs out of stack space.
>
> This warning searches the CFG to determine if every codepath results in a self call.  In addition to the test for this warning, several other tests needed to be fixed, and a pragma to prevent this warning where Clang really wants a stack overflow.
>
> Testing with this warning has already caught several buggy functions.  Common mistakes include: incorrect namespaces, wrapper classes not forwarding calls properly, similarly named member function and data member, and failing to call an overload of the same function.  When run outside of template instantiations, all true positives.  In template instantiations, only 25% true positive.  Therefore, this warning is disabled in template instantiations.  An example of such a false positive is in the test cases.
>
> http://llvm-reviews.chandlerc.com/D1864
>
> Files:
>   test/Analysis/inlining/test-always-inline-size-option.c
>   test/Analysis/misc-ps-region-store.cpp
>   test/Analysis/cxx11-crashes.cpp
>   test/FixIt/typo.m
>   test/FixIt/fixit.c
>   test/Sema/unused-expr-system-header.c
>   test/Sema/warn-unused-function.c
>   test/Sema/attr-deprecated.c
>   test/Parser/cxx-using-declaration.cpp
>   test/Parser/expressions.c
>   test/CodeGen/functions.c
>   test/SemaCXX/statements.cpp
>   test/SemaCXX/warn-bool-conversion.cpp
>   test/SemaCXX/warn-infinite-recursion.cpp
>   test/SemaCXX/MicrosoftCompatibility.cpp
>   test/Lexer/gnu-flags.c
>   include/clang/Basic/DiagnosticSemaKinds.td
>   include/clang/Basic/DiagnosticGroups.td
>   lib/Sema/AnalysisBasedWarnings.cpp
>   lib/Lex/Pragma.cpp
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>