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

Sean Silva silvas at purdue.edu
Fri Nov 8 22:47:28 PST 2013


On Fri, Nov 8, 2013 at 8:52 PM, Chandler Carruth <chandlerc at google.com>wrote:

> Random thoughts...
>
> On Fri, Nov 8, 2013 at 3:20 PM, Sean Silva <silvas at purdue.edu> wrote:
>
>> At a higher level, is this really needed as a compiler warning? I mean,
>> it's nice and all to detect these things statically, but is this really
>> something that needs to be happening on every build?
>
>
> Note that if this is a significant compile time issue, that's relevant.
> Everything else is assuming that it isn't. =]
>
> Could we just do this in the static analyzer? In practice, I can't imagine
>> any of these being hard to debug "while developing", since they will always
>> result in a stack overflow the second they are called (and then you just
>> look at the core file (or the debugger that your IDE attached) to see which
>> function it was)
>
>
> Your description alone is the evidence for why developers should have a
> warning. ;] First off, stack overflows are notoriously annoying to debug.
> Core files are often missing. Running under the debugger is yet another
> step to do. I would much rather the compiler just tell me about it.
>

Absolutely; a static warning is always preferable. All my comments were in
light of a perception that this was maybe not cheap enough to justify
running at every compile (of course, need to wait for Richard to respond
with some real measurements of the compile time impact).

-- Sean Silva


>
> Almost all of our warnings "aren't needed" because they could be tested
> and debugged. That doesn't remove their value.
>
>
>> So essentially it seems like this is finding bugs in code that has no
>> test coverage and has never been executed in practice; that kind of
>> "cleaning out crusty unused parts of the codebase" seems like it would be
>> better left to the static analyzer.
>
>
> No, it's shortening the cycle time for developers that hit this bug.
>
> It also is cleaning out bugs in crufty code, which is always nice. Very
> few people will run the static analyzer over all of their crufty code
> because if they aren't changing it and it is working in production, they
> aren't going to want to sift through the false positives of "maybe that's a
> bug". Static analyzers run much more over code under active development.
>
> And fundamentally, we *routinely* do all of the static analysis that is
> sufficiently inexpensive and has sufficiently rare false positives at
> compile time. So I think we should focus on those two criteria, the latter
> one being the most interesting here.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131109/79c8fed1/attachment.html>


More information about the cfe-commits mailing list