[PATCH] D26342: [analyzer] Add MutexChecker for the Magenta kernel

NoQ added a comment. In https://reviews.llvm.org/D26342#590881, @khazem wrote: > I think that it's sensible to try merging this with PthreadLockChecker. In > fact, I could also try merging the SpinLockChecker [1] that I've posted for > review with PthreadLockChecker also, since they all impleme

[PATCH] D25985: [analyzer] Export coverage information from the analyzer.

NoQ added a comment. Maybe you could instead make a checker that subscribes for `checkEndAnalysis` and scans the provided `ExplodedGraph`'s `nodes_begin()..nodes_end()` for visited statement-based program points (as in `PathDiagnosticLocation::getStmt(N)`)? This should give you per-statement p

[PATCH] D26442: [analyzer] Fix crash on getSVal: handle case of CompoundVal

NoQ added a comment. Wow, this crash must have been hard to notice! I think we shouldn't be default-binding non-lazy compound values. Normally we unpack them into field bindings right away, but it seems that nobody cared to implement this for unions. The current crash goes through `RegionStore

[PATCH] D26442: [analyzer] Fix crash on getSVal: handle case of CompoundVal

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I think this patch is good to land, but if you have time i'd suggest to investigate this a little bit deeper in order to squash even more bugs. My concern is that we've never implemented reading fro

r286628 - [ASTMatchers] Fix a typo in cStyleCastExpr() docs. NFC.

Author: dergachev Date: Fri Nov 11 14:29:59 2016 New Revision: 286628 URL: http://llvm.org/viewvc/llvm-project?rev=286628&view=rev Log: [ASTMatchers] Fix a typo in cStyleCastExpr() docs. NFC. Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Modified: cfe/trunk/include/clang/ASTMat

r286651 - [ASTMatchers] Fix a typo in cStyleCastExpr() HTML docs as well. NFC.

Author: dergachev Date: Fri Nov 11 16:34:53 2016 New Revision: 286651 URL: http://llvm.org/viewvc/llvm-project?rev=286651&view=rev Log: [ASTMatchers] Fix a typo in cStyleCastExpr() HTML docs as well. NFC. Modified: cfe/trunk/docs/LibASTMatchersReference.html Modified: cfe/trunk/docs/LibASTMa

Re: r286628 - [ASTMatchers] Fix a typo in cStyleCastExpr() docs. NFC.

Ouch, thanks! r286651. On 11/11/16 2:31 PM, Malcolm Parsons wrote: On 11 November 2016 at 20:29, Artem Dergachev via cfe-commits wrote: URL: http://llvm.org/viewvc/llvm-project?rev=286628&view=rev Log: [ASTMatchers] Fix a typo in cStyleCastExpr() docs. NFC. Modified: cfe/trunk/inc

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

NoQ added a comment. Welcome to phabricator! I agree that having the location context in this callback is useful, and i'm all for reducing boilerplate in various checkers through better API. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:231 P

[PATCH] D26589: Add static analyzer checker for finding infinite recursion

NoQ added a comment. Thank you for working on this! The overall approach is good. Because alpha checkers are disabled by default, false positives can be addressed later in subsequent commits. Comment at: lib/StaticAnalyzer/Checkers/RecursionChecker.cpp:2 +// InfiniteRecursion

Re: [PATCH] D12725: [analyzer] A fix for substraction of an integer from a pointer.

NoQ updated this revision to Diff 34320. http://reviews.llvm.org/D12725 Files: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/ptr-arith.c Index: test/Analysis/ptr-arith.c === --- test/Analysis/ptr-arith.c +++ test/A

[PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ created this revision. NoQ added reviewers: zaks.anna, krememek. NoQ added subscribers: cfe-commits, dergachev.a. In Clang Static Analyzer, when the symbol is referenced by an index value of an element region, it does not prevent garbage collection (reaping) of that symbol. Hence, if the ele

Re: [PATCH] D12725: [analyzer] A fix for substraction of an integer from a pointer.

NoQ updated this revision to Diff 34423. NoQ added a comment. Make the tests easier to understand. http://reviews.llvm.org/D12725 Files: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/ptr-arith.c Index: test/Analysis/ptr-arith.c

Re: [PATCH] D12725: [analyzer] A fix for substraction of an integer from a pointer.

NoQ added a comment. Thanks, fixed :) http://reviews.llvm.org/D12725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ updated this revision to Diff 35067. NoQ added a comment. Thanks for the quick reply, sorry for the delay! Was afk for a couple of days. Yeah, right, in fact i didn't even fix the issue for store keys at all; only for store values and environment values. It also seems much harder to test st

Re: [PATCH] D12725: [analyzer] A fix for substraction of an integer from a pointer.

NoQ added a comment. I've got no commit access yet, sorry, that's my first patch here actually :) http://reviews.llvm.org/D12725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ updated this revision to Diff 36946. NoQ marked 4 inline comments as done. NoQ added a comment. Thanks for the review! Fixed all comments. I don't notice any significant performance hit with this patch (+/- 2% on the whole Android codebase runs). I guess i won't break this into separate revi

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ updated this revision to Diff 37094. NoQ marked an inline comment as done. NoQ added a comment. > What are the implications in each case? Uhm, sorry, right, I guess i have to admit i don't quite understand what exactly is going on, so it'd take more time for me to conduct a deeper audit of

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ updated this revision to Diff 37263. NoQ added a comment. Hmm, i think i'm ready to explain most of the stuff. - First of all, the piece of code in `EnvironmentManager::removeDeadBindings()` i mentioned above is truly useless; everything would be marked as live anyway on the next line. - T

Re: [PATCH] D5104: [analyzer] [proposal] Fix for PR20653

NoQ added a subscriber: NoQ. NoQ added a comment. While investigating http://reviews.llvm.org/D12726 , i accidentally came up with a way to test this patch; with the extension of `ExprInspectionChecker` in the aforementioned review, which allows testing SymbolReaper directly, the following test

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ updated this revision to Diff 37266. NoQ added a comment. Whoops accidentally left out a dead code line in tests. http://reviews.llvm.org/D12726 Files: docs/analyzer/DebugChecks.rst include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h lib/StaticAnalyzer/Checkers/ExprInspect

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

NoQ updated this revision to Diff 37267. NoQ added a comment. A, another mis-submit. Sorry for the noise. http://reviews.llvm.org/D12726 Files: docs/analyzer/DebugChecks.rst include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h lib/StaticAnalyzer/Checkers/ExprInspectionCheck

[PATCH] D22622: [analyzer] Add more info to exploded graph dumps

NoQ created this revision. NoQ added reviewers: zaks.anna, dcoughlin. NoQ added subscribers: xazax.hun, a.sidorin, cfe-commits. `debug.ViewExplodedGraph`, aka `-analyzer-viz-egraph-graphviz`, is often the only way to understand the otherwise confusing analyzer report. Exploded graphs are often h

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

NoQ added a comment. Whoops forgot the screenshots: F2187578: 2.png F2187579: 1.png Also not sure how to write tests for these, because we can't choose the directory path for the dump, can we? https://reviews.llvm.org/D2

Re: [PATCH] D20795: Added basic capabilities to detect source code clones.

NoQ added a comment. Regarding the cache stack - it feels easier for me to allocate a separate stack for each statement, and put the stack on stack (!) rather than having it global. This way it'd be automatically cleaned for you when VisitStmt() exits, and you'd be able to address child cache b

Re: [PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

NoQ added a comment. The natural way to avoid both heavy artillery with `StmtNodes.inc` and low-performance if-chains is to use a //Visitor//. (`Const`)`StmtVisitor` is implemented as a huge switch, contents of which are auto-generated from `StmtNodes.inc`. In fact, your approach with `StmtNod

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

NoQ updated this revision to Diff 65223. https://reviews.llvm.org/D22622 Files: lib/StaticAnalyzer/Core/ExprEngine.cpp Index: lib/StaticAnalyzer/Core/ExprEngine.cpp === --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnal

Re: [PATCH] D22622: [analyzer] Add more info to exploded graph dumps

NoQ added a comment. > We could add file names but only in cases the file name does not match the > main file. Whoops, right, i guess i'd just fall back to the default source location printing routine for this case (or for macros). F2198462: 3.png > This i

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ updated this revision to Diff 65248. NoQ marked 11 inline comments as done. NoQ added a comment. Renamed the checker as **xazax.hun** suggested, added a lot more comments, done with inline comments :) https://reviews.llvm.org/D20811 Files: include/clang/StaticAnalyzer/Checkers/Checkers.t

r276557 - [analyzer] Pring LocationContext in ExplodedGraph dumps.

Author: dergachev Date: Sun Jul 24 03:15:58 2016 New Revision: 276557 URL: http://llvm.org/viewvc/llvm-project?rev=276557&view=rev Log: [analyzer] Pring LocationContext in ExplodedGraph dumps. Remove some FIXMEs in the surrounding code, which have been addressed long time ago by introducing check

Re: [PATCH] D20795: Added basic capabilities to detect source code clones.

NoQ added a comment. The idea with strings as keys is curious! I've got a few more minor comments :) Comment at: lib/Analysis/CloneDetection.cpp:104 @@ +103,3 @@ +// Storage for the signatures of the direct child statements. This is only +// needed if the current statemn

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ updated this revision to Diff 65369. NoQ marked 4 inline comments as done. https://reviews.llvm.org/D20811 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp test/Analysis/std-l

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:191 @@ +190,3 @@ +// impossible to verify this precisely, but at least +// this check avoids potential crashes. +bool matchesCall(const CallExpr *CE) const; --

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:192 @@ +191,3 @@ + }; + + // The map of all functions supported by the checker. It is initialized Even though there are some doxygen-style comments in the checkers,

Re: [PATCH] D22419: [CFG] Fix crash in thread sanitizer.

NoQ added a comment. Thanks for working on this! While i probably won't be of much help (no expert in temporaries yet), i notice that this code piece (with `getReferenceInitTemporaryType()`) seems to be duplicated at least three times in this file, with slight variations and FIXMEs. So i guess

Re: [PATCH] D20795: Added basic capabilities to detect source code clones.

NoQ added inline comments. Comment at: lib/Analysis/CloneDetection.cpp:148 @@ +147,3 @@ + +// FIXME: This function has quadratic runtime right now. Check if skipping +// this function for too long CompoundStmts is an option. I've a feeling this comment was

Re: [PATCH] D20795: Added basic capabilities to detect source code clones.

NoQ accepted this revision. NoQ added a reviewer: NoQ. NoQ added a comment. Great! Will commit. https://reviews.llvm.org/D20795 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r276782 - [analyzer] Add basic capabilities to detect source code clones.

Author: dergachev Date: Tue Jul 26 13:13:12 2016 New Revision: 276782 URL: http://llvm.org/viewvc/llvm-project?rev=276782&view=rev Log: [analyzer] Add basic capabilities to detect source code clones. This patch adds the CloneDetector class which allows searching source code for clones. For every

r276791 - [analyzer] Hotfix for build failure due to declaration shadowing in r276782.

Author: dergachev Date: Tue Jul 26 14:05:22 2016 New Revision: 276791 URL: http://llvm.org/viewvc/llvm-project?rev=276791&view=rev Log: [analyzer] Hotfix for build failure due to declaration shadowing in r276782. CloneDetector member variable is shadowing the class with the same name, which cause

Re: r276782 - [analyzer] Add basic capabilities to detect source code clones.

Dergachev via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: Author: dergachev Date: Tue Jul 26 13:13:12 2016 New Revision: 276782 URL: http://llvm.org/viewvc/llvm-project?rev=276782&view=rev Log: [analyzer] Add basic capabilities to detect source c

[PATCH] D22856: [analyzer] Change -analyze-function to accept qualified names.

NoQ created this revision. NoQ added a reviewer: zaks.anna. NoQ added subscribers: dcoughlin, xazax.hun, a.sidorin, cfe-commits. The `-analyze-function` option is very useful when debugging test failures, especially when printing or viewing exploded graphs - with this option you no longer need t

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

NoQ added a comment. I see what's going on. `performTrivialCopy()` already calls `evalBind()`, which in turn calls `runCheckersForBind()`, so no effort is needed there. However, the bind event itself is now different - instead of a separate event for every bind, you're having only one event fo

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ added a comment. Answering myself: Hmm, so the only reason why MPI checker class appears in doxygen (http://clang.llvm.org/doxygen/classclang_1_1ento_1_1mpi_1_1MPIChecker.html) is because this class is not in anonymous namespace (as far as i understand, they needed to be multi-file for som

Re: [PATCH] D22856: [analyzer] Change -analyze-function to accept qualified names.

NoQ added a comment. > I think it would be better for fully-qualified Objective-C methods to be > specified with their Objective-C-style names. For example: "-[Test1 > myMethodWithY:withX:]". Uhm, need to know more about those. So i just print "`-[`", then fully-qualified class name, then sel

[PATCH] D22874: [analyzer] Fixes to the checker developer manual.

NoQ created this revision. NoQ added a reviewer: zaks.anna. NoQ added subscribers: dcoughlin, xazax.hun, a.sidorin, cfe-commits. 1. Fix the explanation of how to run tests after migration from autotools to cmake. 2. Expand the "debugging" section with more interesting stuff, as accidentally dis

Re: [PATCH] D22862: [analyzer] Fix for PR15623: eliminate unwanted ProgramState checker data propagation.

NoQ added inline comments. Comment at: test/Analysis/misc-ps-region-store.m:332 @@ -330,3 +331,3 @@ if (p < q) { // If we reach here, 'p' cannot be null. If 'p' is null, then 'n' must // be '0', meaning that this branch is not feasible. ayartsev wrot

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:509 @@ +508,3 @@ + //} + // } + //} dcoughlin wrote: > I disagree about compactness being valuable here. I think it is more > important to intrinsically docu

Re: [PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

NoQ added inline comments. Comment at: lib/Analysis/CloneDetection.cpp:91 @@ +90,3 @@ + ASTContext &Context; + std::vector &CollectedData; + Maybe it's a good idea to introduce a typedef for `unsigned` here, because it'd be nice to be able to change it (eg., so

r277029 - [analyzer] Update the web manual for checker developers.

Author: dergachev Date: Thu Jul 28 15:13:14 2016 New Revision: 277029 URL: http://llvm.org/viewvc/llvm-project?rev=277029&view=rev Log: [analyzer] Update the web manual for checker developers. Fix the explanation of how to run tests after migration from autotools to cmake. Significantly expand t

[PATCH] D22968: [analyzer] A checker for macOS-specific bool-like objects.

NoQ created this revision. NoQ added reviewers: zaks.anna, dcoughlin. NoQ added a subscriber: cfe-commits. If you store a boolean value as an Objective-C object `NSNumber *X`, and want to see if it's true or false, than it's not a good idea to write this check as 'X == YES'. Because X is a point

[PATCH] D22969: [analyzer] Fix execution permissions for scan-build-py.

NoQ created this revision. NoQ added reviewers: zaks.anna, dcoughlin, rizsotto.mailinglist. NoQ added a subscriber: cfe-commits. It seems that the executable scripts are not marked as executable in the repository, so i had to `chmod +x` them before using. Because the old scan-build is `+x`, i gu

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ added a comment. > Is it really a problem if the checker comments are part of the Doxygen > documentation? Of course not :) I've been mostly thinking about the benefits of the anonymous namespace itself (cleaner global scope, no name collisions, but even these benefits are extremely minor

Re: [PATCH] D22514: CloneDetection now respects statement specific data when searching for clones.

NoQ added a comment. Congrats on the fantastic hack that nobody else noticed! I've a feeling we cannot add tests for that, because any test would quickly break if we change the hash, though demonstrating that we fixed the problem would still be cool (eg. take two unsupported expressions of diff

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

NoQ added a comment. Uhm. I've a feeling that now i realize how it should have been written from the start. That's a pretty bad feeling to have. I wish we had a //series of passes//. Before the first pass, all statements are considered equivalent. After the first pass, they're split into smalle

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

NoQ added a comment. Looks good, minor comments :) Comment at: lib/Analysis/CloneDetection.cpp:99 @@ +98,3 @@ + /// Every item in this list is unique. + std::vector Variables; + I've a feeling this is essentially a `map`, because that harmonizes with our acce

r277338 - [analyzer] Fix execution permissions for the scan-build-py scripts.

Author: dergachev Date: Mon Aug 1 05:55:59 2016 New Revision: 277338 URL: http://llvm.org/viewvc/llvm-project?rev=277338&view=rev Log: [analyzer] Fix execution permissions for the scan-build-py scripts. Differential Revision: https://reviews.llvm.org/D22969 Modified: cfe/trunk/tools/scan-bu

Re: [PATCH] D22856: [analyzer] Change -analyze-function to accept qualified names.

NoQ updated this revision to Diff 66360. NoQ added a comment. Copy-paste the code from CGDebugInfo.cpp to support Objective-C instance and class methods. Add more tests. Move the `analyze_display_progress.cpp` because most tests use dashes in names. https://reviews.llvm.org/D22856 Files: li

r277449 - [analyzer] Respect statement-specific data in CloneDetection.

Author: dergachev Date: Tue Aug 2 07:21:09 2016 New Revision: 277449 URL: http://llvm.org/viewvc/llvm-project?rev=277449&view=rev Log: [analyzer] Respect statement-specific data in CloneDetection. So far the CloneDetector only respected the kind of each statement when searching for clones. This

Re: [PATCH] D23014: [analyzer] Model base to derived casts more precisely.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:423 @@ +422,3 @@ + +if (!val.isZeroConstant()) { + val = getStoreManager().evalDynamicCast(val, T, Failed); I guess if `val` is a //non-zero// constant, it wouldn't mak

Re: [PATCH] D23014: [analyzer] Model base to derived casts more precisely.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:423 @@ +422,3 @@ + +if (!val.isZeroConstant()) { + val = getStoreManager().evalDynamicCast(val, T, Failed); xazax.hun wrote: > NoQ wrote: > > I guess if `val` is a //no

Re: r277449 - [analyzer] Respect statement-specific data in CloneDetection.

Wow, i haven't noticed it's mine! Will have a look, sorry. On 8/2/16 5:51 PM, Renato Golin via cfe-commits wrote: On 2 August 2016 at 13:21, Artem Dergachev via cfe-commits wrote: Author: dergachev Date: Tue Aug 2 07:21:09 2016 New Revision: 277449 URL: http://llvm.org/viewvc/ll

r277473 - [analyzer] Hotfix for buildbot failure due to unspecified triple in r277449

Author: dergachev Date: Tue Aug 2 10:16:06 2016 New Revision: 277473 URL: http://llvm.org/viewvc/llvm-project?rev=277473&view=rev Log: [analyzer] Hotfix for buildbot failure due to unspecified triple in r277449 If a target triple is not specified, the default host triple is used, which is not go

Re: [PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.

NoQ added a comment. Nice catch! Now, this needs a test. How about this one: // enable the debug.ExprInspection checker? void clang_analyzer_eval(int); void test_asume_after_access(unsigned long x) { char buf[100]; buf[x] = 1; clang_analyzer_eval(x <= 99); // expected-warnin

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

NoQ accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Analysis/CloneDetection.cpp:99 @@ +98,3 @@ + /// Every item in this list is unique. + std::vector Variables; + NoQ wrote: > I've a feeling this is essentially a `map`, b

Re: [PATCH] D23060: [analyzer] Show enabled checker list

NoQ added a comment. Like! Do you think that if we rename the ~~bike shed~~ option to `-analyzer-list-enabled-checkers` it'd be more natural and easy to remember? (i barely remember how to spell `-analyzer-checker-help`, it'd be much easier if it was called `-analyzer-list-checkers`). ==

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

NoQ added a comment. Hmm. I suggest: 1. Change this test's constructor so that it was no longer almost-trivial. Because it isn't significant for this test if the constructor is almost-trivial or not. The test would pass. 2. Add a FIXME-test for this checker, in which a completely undefined str

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

NoQ requested changes to this revision. NoQ added a comment. This revision now requires changes to proceed. Whoops, because of a merge conflict while applying the patch, i noticed a few problems in the tests, they seem obvious, but could you have a quick look? Comment at: test/

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

NoQ added a comment. In https://reviews.llvm.org/D22374#505672, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D22374#504855, @NoQ wrote: > > > Hmm. I suggest: > > > > 1. Change this test's constructor so that it was no longer almost-trivial. > > Because it isn't significant for this t

r277757 - [analyzer] Make CloneDetector recognize different variable patterns.

Author: dergachev Date: Thu Aug 4 14:37:00 2016 New Revision: 277757 URL: http://llvm.org/viewvc/llvm-project?rev=277757&view=rev Log: [analyzer] Make CloneDetector recognize different variable patterns. CloneDetector should be able to detect clones with renamed variables. However, if variables

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

NoQ added a comment. Something like this, what do you think?: F2247884: TestAlmostNonPOD.patch https://reviews.llvm.org/D22374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

Re: [PATCH] D22862: [analyzer] Fix for PR15623: eliminate unwanted ProgramState checker data propagation.

NoQ added a comment. Hmm. The test in `unwanted-programstate-data-propagation.c` passes on my machine even without the patch, and the return value on the respective path is correctly represented as `(conj_$6{void *}) != 0U`, which comes from the `evalCast()` call in `VisitLogicalExpr()` and is

[PATCH] D23272: [analyzer][Rewrite] Fix boxes and shadows in HTML rewrites in Firefox.

NoQ created this revision. NoQ added reviewers: dcoughlin, zaks.anna. NoQ added subscribers: xazax.hun, a.sidorin, cfe-commits. Use the official CSS3 properties `border-radius` and `box-shadow` (not only `-webkit-`specific properties). Fixes analyzer's diagnostic pieces in HTML diagnostics mode

r278018 - [analyzer] Change -analyze-function to accept qualified names.

Author: dergachev Date: Mon Aug 8 11:01:02 2016 New Revision: 278018 URL: http://llvm.org/viewvc/llvm-project?rev=278018&view=rev Log: [analyzer] Change -analyze-function to accept qualified names. Both -analyze-function and -analyzer-display-progress now share the same convention for naming fun

Re: [PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.

NoQ accepted this revision. This revision is now accepted and ready to land. Comment at: test/Analysis/out-of-bounds.c:153 @@ +152,3 @@ +// The result is unknown for the same reason as above. +void test_asume_after_access(unsigned long x) { + int buf[100]; Yay, t

Re: [PATCH] D23272: [analyzer][Rewrite] Fix boxes and shadows in HTML rewrites in Firefox.

NoQ added inline comments. Comment at: lib/Rewrite/HTMLRewrite.cpp:304 @@ -303,2 +303,3 @@ + " border-radius:5px; box-shadow:1px 1px 7px #000; " " -webkit-border-radius:5px; -webkit-box-shadow:1px 1px 7px #000; " "position: absolute; top: -1em; l

Re: [PATCH] D23300: [analyzer] Add "Assuming..." diagnostic pieces for unsupported condition expressions.

NoQ added a comment. In https://reviews.llvm.org/D23300#509665, @xazax.hun wrote: > In case there is a more complex condition does it only highligh the part that > influenced taking the branch? > > E.g.: > > if (a || b) { // HIghlight only a, if a was true and b was not evaluated > } > No

Re: [PATCH] D23112: [analyzer] Correctly add assumptions based on array bounds.

NoQ added a comment. Whoops, forgot to answer: In https://reviews.llvm.org/D23112#508333, @xazax.hun wrote: > I am not sure that the checker is the appropriate way to fix the remaining > issue with this checker. Yeah, there are anyway more problems that require this functionality in the `Ran

Re: Modify MallocChecker.cpp to recognize kfree( )

Hey, that's a useful patch, i'd like it to be included! There is odd indentation in some places, but in general it looks correct. The usual thing to do nowadays is to put patches on review on Phabricator, would you like to do that? (http://llvm.org/docs/Phabricator.html, http://llvm.org/docs/

r278238 - [analyzer] Fix a crash in CloneDetector when calling functions by pointers.

Author: dergachev Date: Wed Aug 10 11:25:16 2016 New Revision: 278238 URL: http://llvm.org/viewvc/llvm-project?rev=278238&view=rev Log: [analyzer] Fix a crash in CloneDetector when calling functions by pointers. CallExpr may have a null direct callee when the callee function is not known in compi

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

NoQ added a comment. Wow, i've completely forgot about this one. Sorry about that... I think this checker is good to have in the current shape, and probably incrementally developed. It'd probably move out of alpha whenever it's tweaked to somehow make sure it finds enough real bugs among intent

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

NoQ added inline comments. Comment at: lib/Analysis/CloneDetection.cpp:436 @@ +435,3 @@ +if (IsInMacro) { + Signature.Complexity = 0; +} omtcyfz wrote: > Do I understand correctly that a code generated by a macro doesn't affect > "complexity" at all

Re: [PATCH] D15227: [analyzer] Valist checkers.

NoQ added a comment. The checker's in great shape! I see a few minor things, but that's it. The checks are split into two sections ("uninitialized" and "unterminated"), but there seem to be more auxiliary checks provided (eg. "copies into itself") that are on for both checkers, do you think you

Re: [PATCH] D23316: [analyzer] Fixed the false-positives caused by macro generated code.

NoQ added a comment. No comments of my own, the patch looks good :) Comment at: lib/Analysis/CloneDetection.cpp:436 @@ +435,3 @@ +if (IsInMacro) { + Signature.Complexity = 0; +} omtcyfz wrote: > omtcyfz wrote: > > omtcyfz wrote: > > > NoQ wrote: > >

Re: [PATCH] D22862: [analyzer] Fix for PR15623: eliminate unwanted ProgramState checker data propagation.

NoQ added a comment. > @dcoughlin, @NoQ, could you, please, tell, how you get dumps of symbolic > expressions and constraints like "(conj_$6{void *}) != 0U"? Tried different > debug.* checkers and clang_analyzer_explain() but failed. That's `SVal.dump()`, `SymbolRef->dump()`, `MemRegion.dump()

Re: [PATCH] D23314: [analyzer] CloneDetector allows comparing clones for suspicious variable pattern errors.

NoQ added a comment. I couldn't find any real problems - the code's getting mature! Please accept a few very important "please rename this variable" comments :) Comment at: include/clang/Analysis/CloneDetection.h:236 @@ +235,3 @@ +///clone in this pair. +struct

Re: [PATCH] D22515: [analyzer] Added custom hashing to the CloneDetector.

NoQ added a comment. Considering the refactoring planned in https://reviews.llvm.org/D23418, i've no more comments on the part of the code that isn't rewritten in the subsequent review. I like how the new hashing system is put to work here. Comment at: lib/Analysis/CloneDetect

r259345 - [analyzer] Use a wider integer type for an array index.

Author: dergachev Date: Mon Feb 1 03:29:17 2016 New Revision: 259345 URL: http://llvm.org/viewvc/llvm-project?rev=259345&view=rev Log: [analyzer] Use a wider integer type for an array index. Avoids unexpected overflows while performing pointer arithmetics in 64-bit code. Moreover, neither Pointe

[PATCH] D19057: [analyzer] Let TK_PreserveContents span across the whole base region.

NoQ created this revision. NoQ added reviewers: zaks.anna, dcoughlin. NoQ added a subscriber: cfe-commits. Essentially, if `s` is a structure, and `foo(const void *)` is evaluated conservatively, then `foo(&s)` does not invalidate `s`, but `foo(&(s.x))` invalidates the whole `s`, because the sto

r266292 - [ASTImporter] Implement some expression-related AST node import.

Author: dergachev Date: Thu Apr 14 06:51:27 2016 New Revision: 266292 URL: http://llvm.org/viewvc/llvm-project?rev=266292&view=rev Log: [ASTImporter] Implement some expression-related AST node import. Introduce ASTImporter unit test framework. Fix a memory leak introduced in cf8ccff5: an array i

r267413 - [analyzer] Let TK_PreserveContents span across the whole base region.

Author: dergachev Date: Mon Apr 25 09:44:25 2016 New Revision: 267413 URL: http://llvm.org/viewvc/llvm-project?rev=267413&view=rev Log: [analyzer] Let TK_PreserveContents span across the whole base region. If an address of a field is passed through a const pointer, the whole structure's base regi

[PATCH] D20811: [analyzer] Model some library functions

NoQ created this revision. NoQ added reviewers: zaks.anna, dcoughlin. NoQ added a subscriber: cfe-commits. Herald added a subscriber: aemerson. I've put together a simple checker that throws no warnings, but models some library functions, which has already helped us to suppress some false positiv

Re: [PATCH] D20811: [analyzer] Model some library functions

NoQ added a comment. Yeah, good point, the "Std" part should definitely appear in the name, not sure about the "C" thing, as we could probably expand this checker to model some simple C++ functions as well (and then we'd make a separate checker section to move from unix. to cplusplus. or someth

Re: [PATCH] D20863: [analyzer] Fix for the strdup family in unix.malloc checker

NoQ added a comment. This patch is doing a very good thing - modeling contents of the copied string as a lazy compound value of the original string. For that, i guess it's worth adding some tests (ExprInspection-based(?)) that show that, say, first few characters of the copied string are same a

Re: [PATCH] D21506: [analyzer] Block in critical section

NoQ added a comment. Useful stuff! When I see your approach with `mutexCount`, the following test case comes to mind: std::mutex m; void foo() { // Suppose this function is always called // with the mutex 'm' locked. m.unlock(); // Here probably some useful work is done.

Re: [PATCH] D22048: [analyzer] Suppress false positives in std::shared_ptr

NoQ added inline comments. Comment at: test/Analysis/Inputs/system-header-simulator-cxx.h:349 @@ +348,3 @@ +// No warning is expected as we are suppressing warning coming +// out of std::basic_string. +int z = 0; You mean std::shared_ptr here? Also, p

Re: [PATCH] D22048: [analyzer] Suppress false positives in std::shared_ptr

NoQ accepted this revision. NoQ added a comment. LGTM, all clear to me now :) http://reviews.llvm.org/D22048 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21978: [analyzer] Add LocationContext to SymbolMetadata

NoQ added a comment. I also never noticed that the block count gets reset on every stack frame, that's pretty unobvious. Uhm, and it's already there in SymbolConjured. Anyway, fixing the block count itself seems to be pretty complicated. Also, maybe once the ScopeContext thing is ready, we'd be

Re: [PATCH] D20795: Added basic capabilities to detect source code clones.

NoQ added a comment. Your code is well-written and easy to understand, and makes me want to use it on my code! Added some minor comments, and there seems to be a small logic error in the compound statement hasher. Not sure if that was already explained in detail, but i seem to agree that the o

[PATCH] D22242: [analyzer] De-duplicate some code for discovering the origin of the symbol.

NoQ created this revision. NoQ added a reviewer: dcoughlin. NoQ added a subscriber: cfe-commits. Or, more importantly, tell the users that there's a way to discover the region, value of which - at some moment of time - the symbol was introduced to represent. Idea proposed by Devin Coughlin. Rig

Re: [PATCH] D22242: [analyzer] De-duplicate some code for discovering the origin of the symbol.

NoQ updated this revision to Diff 63688. NoQ marked an inline comment as done. NoQ added a comment. Added a doxygen comment, made the function virtual instead of branching. http://reviews.llvm.org/D22242 Files: include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h include/clang/StaticAn

r275290 - [analyzer] Implement a methond to discover origin region of a symbol.

Author: dergachev Date: Wed Jul 13 13:07:26 2016 New Revision: 275290 URL: http://llvm.org/viewvc/llvm-project?rev=275290&view=rev Log: [analyzer] Implement a methond to discover origin region of a symbol. This encourages checkers to make logical decisions depending on value of which region was t

<    4   5   6   7   8   9   10   11   12   13   >