[PATCH] Fix crash in CFGReachabilityAnalysis triggered by IdempotentOperationChecker.

Alexander Kornienko alexfh at google.com
Sat Dec 21 01:54:59 PST 2013


I found the issue, as we used to turn on all the checkers (except debug
ones) in clang-tidy. We have no special interest in this particular
checker, so it's fine if you remove it, since it's not fully functional
anyway.
On 21 Dec 2013 01:21, "Ted Kremenek" <kremenek at apple.com> wrote:

> Alex,
>
> Are you okay with us removing this checker?  It’s never been fully
> productized, and has many issues.  For one, it can produce false positives
> because the analysis path engine is unsound, and can prune paths that are
> actually reachable.  We also had lots of issues with path coverage, which
> made this checker rarely fire on a lot of code.  I think it is worth
> investigating again one day, but right now its pretty much stale code.
>
> On Dec 20, 2013, at 4:04 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
> > IdempotentOperationsChecker has suffered from several known limitations
> for a while, but the addition of interprocedural analysis has certainly
> compounded the issues.  I had thought about it at the time when we added
> interprocedural analysis, but it never became a pressing action item.
> >
> > This checker has been in the alpha state for a couple years now.  Making
> it real is going to take a lot of work beyond just fixing these immediate
> issues.  I think we should just go and remove it entirely.  If we want to
> resuscitate it one day we can.
> >
> > On Dec 19, 2013, at 9:02 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> >
> >>
> >> Aha, of course. Unfortunately, this means that the IdempotentOperations
> checker is even more broken than we thought, because it tries to make
> claims about inlined functions.
> >>
> >> I like the assert, but I don't think perpetuating the brokenness is a
> good idea. With this fix, any blocks that were in an inlined function will
> not be considered reachable from a path through the caller, even though
> they might be. Worse, and independent of this issue, is the fact that
> within an inlined function we only see one path, and the checker might
> mistakenly take that to be the only possible path.
> >>
> >> If we just throw out inlined functions altogether, it might start
> making sense, but then you don't get very good coverage.
> >>
> >> http://llvm-reviews.chandlerc.com/D2427
> >
> > _______________________________________________
> > 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/20131221/d41faff6/attachment.html>


More information about the cfe-commits mailing list