[PATCH] D24010: [ReachableCode] Skip over ExprWithCleanups in isConfigurationValue

krememek added a comment. LGTM. Repository: rL LLVM https://reviews.llvm.org/D24010 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25503: [analyzer] Remove superquadratic behaviour from DataflowWorklist

krememek added a comment. Looks great to me too. Thanks for doing this! Repository: rL LLVM https://reviews.llvm.org/D25503 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r252841 - Change code owner for Clang Static Analyzer to Anna Zaks.

Author: kremenek Date: Wed Nov 11 19:31:27 2015 New Revision: 252841 URL: http://llvm.org/viewvc/llvm-project?rev=252841&view=rev Log: Change code owner for Clang Static Analyzer to Anna Zaks. Modified: cfe/trunk/CODE_OWNERS.TXT Modified: cfe/trunk/CODE_OWNERS.TXT URL: http://llvm.org/viewv

Re: [PATCH] D10021: Refactor: Simplify boolean conditional return statements in lib/StaticAnalyzer/Checkers

krememek added a subscriber: krememek. krememek accepted this revision. krememek added a reviewer: krememek. krememek added a comment. This revision is now accepted and ready to land. These all look good to me. http://reviews.llvm.org/D10021 ___ cfe

Re: [libcxx] r249738 - Split out of .

> On Oct 14, 2015, at 3:49 PM, Duncan P. N. Exon Smith > wrote: > > My understanding is that we don't expect clang to be used with *newer* libc++ > headers, just older or equal. This kind of break is a little unfortunate, > but probably fine. Ted, can you confirm? On Apple platforms, we wi

Re: [PATCH] D12827: [analyzer] fix an error finding clang path

krememek accepted this revision. krememek added a comment. This revision is now accepted and ready to land. Applied in r247510. http://reviews.llvm.org/D12827 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

r247510 - Use -f instead of -d flag for testing existing of clang executable (http://reviews.llvm.org/D12827).

Author: kremenek Date: Sat Sep 12 11:01:34 2015 New Revision: 247510 URL: http://llvm.org/viewvc/llvm-project?rev=247510&view=rev Log: Use -f instead of -d flag for testing existing of clang executable (http://reviews.llvm.org/D12827). Modified: cfe/trunk/tools/scan-build/scan-build Modifie

Re: [PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

krememek added a comment. LGTM as well. Thanks Sean. Repository: rL LLVM http://reviews.llvm.org/D12406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Preventing several replacements on a macro call.

+ Argyrios Hi Angel, I believe Argyrios is the original author of the code in question, as this looks related to the Objective-C “modernizer” migrator he wrote a while back. This code started life on our internal development branch at Apple related to an Xcode feature we were doing, and I did

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

krememek accepted this revision. krememek added a comment. This revision is now accepted and ready to land. Committed in r246978. http://reviews.llvm.org/D12673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

krememek added a comment. I am OK with taking these changes. For those using git, git blame -w should suffice to show the real blame history. http://reviews.llvm.org/D12673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

Re: [PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

krememek added a subscriber: krememek. krememek added a comment. This looks fine, but I'm not a fan of the actual name chosen here. It's not very clear what it means by just looking at the name, and as we grow the number of configuration options that will become more important. A few suggestio

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. In http://reviews.llvm.org/D12358#238303, @seaneveson wrote: > In http://reviews.llvm.org/D12358#237099, @krememek wrote: > > > To get the conservative invalidation that Anna suggests (actually, a little > > less conservative), I think you can just pass in a two MemRegi

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. In http://reviews.llvm.org/D12358#235142, @seaneveson wrote: > If I refactor this patch in its current state into a separate file and put it > behind a flag, would it then be acceptable? I would then like to take some > time to look at the invalidation improvements as

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. In http://reviews.llvm.org/D12358#234975, @krememek wrote: > > > > 1. We need to consult the number of times a loop has been executed to > determine when to apply this widening. We can look at the basic block > counts, which are already used to cull off too ma

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

Hi Sean, I responded with some more feedback. Conceptually the patch is quite simple, but I think Anna’s points are all spot on. I think we’d all be quite amendable for this work to go in under a flag, with further improvements going in on top of that. That way we can all collectively start

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. I think the functionality started here by doing something clever with loops is complex enough to warrant putting this into a separate .cpp file. We can keep this here for now, but this seems like complexity that is going to naturally creep up and make this file even m

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. In http://reviews.llvm.org/D12358#234949, @zaks.anna wrote: > > I accept that my current patch is not a comprehensive solution to the > > problem and that it may introduce > false positives, however I do think it > > is an improvement, where it is preferable to have fa

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

> On Aug 27, 2015, at 8:57 AM, Sean Eveson wrote: > > I accept that my current patch is not a comprehensive solution to the problem > and that it may introduce false positives, however I do think it is an > improvement, where it is preferable to have false positives over doing no > analysi

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. One thing about the tests passing: that's great, but that's obviously insufficient. We probably have fewer tests showing that the analyzer can't handle something than tests that show it does handle something. When a patch like this lands, which expands the analysis sc

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. FWIW, I do think this is a great problem to work on. It is easy to come up with solutions that work for specific examples but fall over on general code. I completely agree that failing to analyzing code after the loop is a major hole and lost opportunity to find bugs

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

> On Aug 26, 2015, at 3:59 AM, Sean Eveson via cfe-commits > wrote: > > We have been looking at the following problem, where any code after the > constant bound loop is not analyzed because of the limit on how many times > the same block is visited, as described in bugzillas #7638 and #23438.

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

krememek added a comment. In http://reviews.llvm.org/D12358#233983, @zaks.anna wrote: > A way this could be improved is by invalidating all the values that the loops > effects, both the values on the stack and on the heap. (We could even start > overly conservative and invalidate all the values

r246003 - Add missing newline.

Author: kremenek Date: Tue Aug 25 22:11:31 2015 New Revision: 246003 URL: http://llvm.org/viewvc/llvm-project?rev=246003&view=rev Log: Add missing newline. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChec

Re: [PATCH] D11700: Added remove taint support to ProgramState.

krememek added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:444-448 @@ +443,7 @@ + + SymbolRef + getSymbolFromStmt(const Stmt *S, const LocationContext *LCtx) const; + + const MemRegion* + getRegionFromStmt(const Stmt *S, const L

Re: [PATCH] D11700: Added remove taint support to ProgramState.

krememek added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:444-448 @@ +443,7 @@ + + SymbolRef + getSymbolFromStmt(const Stmt *S, const LocationContext *LCtx) const; + + const MemRegion* + getRegionFromStmt(const Stmt *S, const L

Re: [PATCH] D12119: Analyzer: Fix a crasher in UbigraphViz

krememek added inline comments. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:749 @@ -748,3 @@ - - assert (Src != Dst && "Self-edges are not allowed."); - Did you look at the test case that causes this assertion to fail? I think it would be good

Re: [PATCH] D12119: Analyzer: Fix a crasher in UbigraphViz

krememek added inline comments. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:778 @@ -780,3 +777,3 @@ -UbigraphViz::UbigraphViz(std::unique_ptr Out, StringRef Filename) -: Out(std::move(Out)), Filename(Filename), Cntr(0) { +UbigraphViz::UbigraphViz(std::unique

Re: [PATCH] D12144: Fix 4 typos in lib/Analysis/

krememek added a comment. Looks fine in UninitializedValues. http://reviews.llvm.org/D12144 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12123: [analyzer] Skip Pre/Post handlers for ObjC calls when receiver is nil.

krememek added a subscriber: krememek. krememek added a comment. I think this is a great refinement overall, with a few minor nits. It also isn't clear what the test does. Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:577 @@ -559,1 +576,3 @@ + const std::ve

Re: [PATCH] D12119: Analyzer: Fix a crasher in UbigraphViz

krememek added inline comments. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:749 @@ -748,3 +748,3 @@ assert (Src != Dst && "Self-edges are not allowed."); ismailp wrote: > Is Ubigraph generator actively maintained? If I run tests with > '-a

Re: [PATCH] D12119: Analyzer: Fix a crasher in UbigraphViz

krememek added inline comments. Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:784 @@ -783,3 +783,3 @@ - *Out << "('vertex_style_attribute', 0, ('shape', 'icosahedron'))\n"; - *Out << "('vertex_style', 1, 0, ('shape', 'sphere'), ('color', '#ffcc66')," + *this->O

r244400 - [Static Analyzer] Add --analyzer-target option to scan-build.

Author: kremenek Date: Sat Aug 8 12:58:47 2015 New Revision: 244400 URL: http://llvm.org/viewvc/llvm-project?rev=244400&view=rev Log: [Static Analyzer] Add --analyzer-target option to scan-build. When interposing on a compiler doing cross-compilation, scan-build does not infer the target triple

Re: [PATCH] D10356: scan-build: Add --analyzer-target option

krememek added a comment. Committed r244400 http://reviews.llvm.org/D10356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D10356: scan-build: Add --analyzer-target option

krememek added a comment. In http://reviews.llvm.org/D10356#217564, @honggyu.kim wrote: > > With --analyzer-target option, we can provide target info to clang static > analyzer as follows: > > $ scan-build -v -v -v --analyzer-target=arm-linux-gnueabi > --use-cc=arm-linux-gnueabi-gcc make