Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-23 Thread Alexander Shaposhnikov via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL282293: [analyzer] Fix crash in RetainCountChecker::checkEndFunction (authored by alexshap). Changed prior to commit: https://reviews.llvm.org/D24792?vs=72206&id=72354#toc Repository: rL LLVM https:

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-23 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment. Thanks, Chris has recently granted me commit access, i will rebase, rerun all the tests and then commit. Repository: rL LLVM https://reviews.llvm.org/D24792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-23 Thread Anna Zaks via cfe-commits
zaks.anna added a comment. Thanks! @alexshap, Do yon have commit access or should we commit on your behalf? Repository: rL LLVM https://reviews.llvm.org/D24792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-22 Thread Artem Dergachev via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks again! LGTM. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:656 @@ +655,3 @@ + // Clear the AnalysisManager of old AnalysisDeclContexts. + Mgr->ClearContexts(

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-22 Thread Alexander Shaposhnikov via cfe-commits
alexshap updated this revision to Diff 72206. alexshap added a comment. Move the check to AnalysisConsumer::HandleCode. Repository: rL LLVM https://reviews.llvm.org/D24792 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/An

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-22 Thread Artem Dergachev via cfe-commits
NoQ added a comment. Thanks, this makes a lot more sense to me! Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:691 @@ -690,1 +690,3 @@ + // Ignore autosynthesized code. + if (Mgr->getAnalysisDeclContext(D)->isBodyAutosynthesized()) I think this

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Alexander Shaposhnikov via cfe-commits
alexshap updated the summary for this revision. alexshap updated this revision to Diff 72132. alexshap added a comment. Address the comments Repository: rL LLVM https://reviews.llvm.org/D24792 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp lib/StaticAnalyzer/Frontend/AnalysisC

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Anna Zaks via cfe-commits
zaks.anna added a comment. One approach would be to skip analyzing the functions which we model as top level. - a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -688,6 +688,9 @@ void AnalysisConsumer::ActionExprEngine(Decl *D, bool Ob

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment. @zaks.anna: it happens on one of the ports of libdispatch, but probably i also can reproduce this crash with OSAtomicCompareAndSwap. Repository: rL LLVM https://reviews.llvm.org/D24792 ___ cfe-commits mailing list cfe-c

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Anna Zaks via cfe-commits
zaks.anna added a comment. Can you give a bit more context? Do you see the crash on a redefinition of the OSAtomicCompareAndSwapPtr or one of the other standard functions or do you have another similarly named function that should not be modeled? Repository: rL LLVM https://reviews.llvm.org

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment. > is that a problem for your codebase? @zaks.anna - yes it is. > Another possible issue is that we will use the synthesized body if the > function name starts with "OSAtomicCompareAndSwap" since ?>we do not match > the full function name. If the function body is ava

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Anna Zaks via cfe-commits
zaks.anna added a comment. > However, the assert here has a reason: we clearly shouldn't be trying to > analyze synthesized bodies as top-level functions. Yes, seems like we should update r264687 so that we use the available body when analyzing as top level. Another possible issue is that we

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Devin Coughlin via cfe-commits
dcoughlin added a comment. I suspect I introduced this regression in r264687. Repository: rL LLVM https://reviews.llvm.org/D24792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Artem Dergachev via cfe-commits
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:3866 @@ -3865,3 +3865,3 @@ if (LCtx->getAnalysisDeclContext()->isBodyAutosynthesized()) { -assert(LCtx->getParent()); +assert(LCtx->inTopFrame() || LCtx->getParent()); return

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

2016-09-21 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment. Without the changes: F2438373: Screen Shot 2016-09-21 at 12.38.22 AM.png With the patch the tests are green. Repository: rL LLVM https://reviews.llvm.org/D24792 ___ cfe-commits mailin