[PATCH] D52853: [Analyzer] Try fixing ConstraintManager::assumeDual

2018-10-03 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. *reads the comments* no, NDEBUG won't work. Ok, we'll take care of this, thanks! Repository: rC Clang https://reviews.llvm.org/D52853 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D51388: [analyzer] NFC: Legalize state manager factory injection.

2018-10-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h:33 /// introduce a type named \c NameTy. - /// The macro should not be used inside namespaces, or for traits that must - /// be accessible from more than one translati

[PATCH] D52905: CSA: fix accessing GDM data from shared libraries

2018-10-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added subscribers: george.karpenkov, dcoughlin, NoQ. NoQ added a comment. Hmmm, interesting. A checker doesn't usually need to access these specific static locals, at least not directly. These are usually accessed through functions in .cpp files that are supposed to be compiled with a pointe

[PATCH] D52957: [analyzer] Teach CallEvent about C++17 aligned new.

2018-10-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, baloghadamsoftware. In C++17, when class `C` has large alignment value, a special case of overload resolution r

[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter

2018-10-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Just add tests, i guess! Also i'll have a look at whether the checker is in a good shape to be enabled by default. I suspect that mismatched iterators might be very safe to enable. With all these s

[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-10-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: www/analyzer/checker_dev_manual.html:708 +Checker Reviewer Checklist + I think we actually need two separate checklists: * for common bugs (careful use of non-fatal error nodes, etc. - stuff we check for on every review)

[PATCH] D52993: [analyzer][www] Add more useful links

2018-10-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thx! Comment at: www/analyzer/checker_dev_manual.html:703-704 Useful Links +http://lcs.ios.ac.cn/~xuzb/canalyze/memmodel.pdf";>Xu, Zhongxing & Maybe merge wit

[PATCH] D52969: [analyzer][www] Update alpha_checks.html

2018-10-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks! Comment at: www/analyzer/alpha_checks.html:450 - -Variable Argument Alpha Checkers - I think these are missing on the available checkers list, so we should

[PATCH] D52983: [analyzer] Support Reinitializes attribute in MisusedMovedObject check

2018-10-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yay, these look useful. Is there also an attribute for methods that should never be called on a 'moved-from' object? Repository: rC Clang https://reviews.llvm.org/D52983 _

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-01-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. P.S. I also support the idea to have a separate page per checker. It encourages people to write as much documentation as necessary and continously expand it. Otherwise there's a perception that all docs need to be roughly of the same size and shape in order to make the page

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

2019-01-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:166-173 + // The declaration of the value may rely on a pointer so take its l-value. + if (const auto *DRE = dyn_cast_or_null(CondVarExpr)) { +if (const auto *VD = dyn_cast_or_null(DRE->g

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

2019-02-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Fxd the global variable problem in D57619 , thanks! I think you should change back to `getInit()` once D57619 lands. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46421/new/

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-02-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Dunno but made a random guess in the inline comment. Comment at: docs/analyzer/checkers.rst:1946 +Checkers used for debugging the analyzer. +:doc:`DebugChecks` page contains a detailed description. + The error is on this line. It might be t

[PATCH] D57230: [analyzer] Toning down invalidation a bit

2019-02-05 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Herald added a project: LLVM. Hmm. `writes_to_superobject`? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D57230: [analyzer] Toning down invalidation a bit

2019-02-06 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. There seem to be a few regressions - weird memory leaks of inner objects in C++ destructors. Trying to investigate/reproduce. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230

[PATCH] D57230: [analyzer] Toning down invalidation a bit

2019-02-07 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. That's the one: typedef __typeof(sizeof(int)) size_t; void *malloc(size_t); void escape(int **); struct S { int *ptr; }; void foo() { struct S s1; s1.ptr = malloc(sizeof(int)); escape(&s1.ptr); } After the patch the allocated symbol no

[PATCH] D57981: [analyzer] strlcat() syntax check: Fix an off-by-one error.

2019-02-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: dcoughlin, george.karpenkov, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, devnexen. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet. Herald added a project: clang. That's a fix on top o

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

2019-02-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks cool, thanks!! Comment at: include/clang/Basic/DiagnosticDriverKinds.td:304-305 def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">; +def err_analyzer_ch

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

2019-02-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I think this is awesome! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57858/new/ https://reviews.llvm.org/D57858 ___

[PATCH] D57860: [analyzer] Validate checker option names and values

2019-02-08 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. > (already in the works!) Document the frontend of the analyzer, sneak peak > here. Whoa!! *celebrates the graphomania epidemic* Repository: rC Clang CHANGES SINCE LAST ACTION https://review

[PATCH] D58067: [Analyzer] Crash fix for FindLastStoreBRVisitor

2019-02-11 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Aha, right, indeed, thanks, nice! We should eventually remove `operator==()` from the `SVal` class because it doesn't do anything you'd possibly imagine it to do. However we do need to come up with

[PATCH] D57890: [analyzer] Fix in self assignment checker

2019-02-11 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Sounds reasonable, but it also sounds like something that should be reproducible on the upstream clang. Do you have a code snippet that causes the problematic AST to appear? Even if we don't have the false positive up here in upstream, Is it something we can test via `-anal

[PATCH] D58069: [Sema] Mark GNU compound literal array init as an rvalue.

2019-02-11 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > The analyzer test change maybe indicates we could simplify the analyzer code > a little with this fix? Apparently a hack was added to support lvalues in > initializers in r315750, but I'm not really familiar with the relevant code. Nope, unfortunately, not much can be sim

[PATCH] D57579: [analyzer][WIP] Enable subcheckers to possess checker options

2019-02-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. Herald added a subscriber: jdoerfert. > since I couldn't reproduce the error Me neither, actually :/ This patch does provide a backup plan and it makes the code prettier, so we should definitely land it and i'm really greatful for it, but the opt

[PATCH] D58065: [analyzer] Document the frontend library

2019-02-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Had a look. Great stuff, just as planned :) Old fanboy wisdom: Try to avoid documenting bugs you want to fix! But i don't have many high-level comments here - only appreciation of the effort. In D58065#1394864 , @Szelethus wrote: >

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

2019-02-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. After staring at this for an hour or two, i didn't manage to force myself to understand how our cluster analysis works here, but i totally agree that it's most likely broken; i guess, we should eventually move away from the idea that everything works through base regions, b

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

2019-02-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Herald added a subscriber: jdoerfert. Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:352-355 + if (Checker->AllowedPad < 0) + Mgr.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input) +<< (llvm::Twine() + Chec

[PATCH] D58065: [analyzer] Document the frontend library

2019-02-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D58065#1400615 , @Szelethus wrote: > I've also wasted my second weekend trying to make Static Analyzer unit tests > run under check-clang-analysis :) I tried to have a quick look but got confused pretty quickly. In D58065#140061

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

2019-02-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: aaron.ballman, dcoughlin, george.karpenkov. Herald added a project: clang. Herald added a subscriber: cfe-commits. Continuing the trend started in D54912 , i'd like to stuff one more XNU/Mach/Darwin-specific attribut

[PATCH] D58366: [analyzer] MIGChecker: Make use of the server routine annotation.

2019-02-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. The attribute that's added in D58365

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-02-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, a.sidorin, szepet. Herald added a project: clang. As i mentioned in D58067#1393674

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

2019-02-18 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. This adds two visitors to the checker: - `trackExpressionValue()` in

[PATCH] D58392: [analyzer] MIGChecker: Fix false negatives for releases in automatic destructors.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. The problem that was fixed in D25326

[PATCH] D58397: [analyzer] MIGChecker: Pour more data into the checker.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Previous patches were about the infrastructure of the chec

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4047-4048 +that have their return value of type ``kern_return_t`` unconditionally returned +from the routine. The attribute can be applied to C++ methods, and in thi

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a reviewer: Charusso. NoQ marked an inline comment as done. NoQ added a subscriber: Charusso. NoQ added a comment. In D58367#1402796 , @Szelethus wrote: > 2. I guess most of the visitors could be changed to this format, do you have > plans to co

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. I'll take a closer look in a few days, sorry for the delays! The progress you're making is fantastic and i really appreciate your work, but i have an urgent piece of work of my own to deal with this week. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53076/new/ ht

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187497. NoQ marked 13 inline comments as done. NoQ added a comment. Fxd, thx! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58365/new/ https://reviews.llvm.org/D58365 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1310 +def MIGServerRoutine : InheritableAttr { + let Spellings = [Clang<"mig_server_routine">]; aaron.ballman wrote: > Should this be limited to specific targets, or is this a general con

[PATCH] D58366: [analyzer] MIGChecker: Make use of the server routine annotation.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187498. NoQ added a comment. Rebase due to an update in D58365 - namely, add support for Objective-C messages and blocks that are now allowed to wear the attribute. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58366/n

[PATCH] D58366: [analyzer] MIGChecker: Make use of the server routine annotation.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187501. NoQ added a comment. Whoops - make sure that the Objective-C test does actually test something. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58366/new/ https://reviews.llvm.org/D58366 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187502. NoQ added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58368/new/ https://reviews.llvm.org/D58368 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp clang/test/Analysis/mig.mm Index: clang/test/Analysis/mig.mm

[PATCH] D58392: [analyzer] MIGChecker: Fix false negatives for releases in automatic destructors.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187503. NoQ added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58392/new/ https://reviews.llvm.org/D58392 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp clang/test/Analysis/mig.mm Index: clang/test/Analysis/mig.mm ===

[PATCH] D58397: [analyzer] MIGChecker: Pour more data into the checker.

2019-02-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187505. NoQ added a comment. Rebase. Fix behavior when the return code is not constrained enough. Test the C++11 attribute syntax (just in case). Update comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58397/new/ https://reviews.llvm.org/D58397

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

2019-02-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187656. NoQ marked 5 inline comments as done. NoQ added a comment. Fxd. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58365/new/ https://reviews.llvm.org/D58365 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

2019-02-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/test/Sema/attr-mig.c:6 + +__attribute__((mig_server_routine)) kern_return_t var = KERN_SUCCESS; // expected-warning-re{{'mig_server_routine' attribute only applies to functions, Objective-C methods, and blocks{{$ + --

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

2019-02-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked 3 inline comments as done. NoQ added a comment. Thx! Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:109 +llvm::raw_svector_ostream OS(Str); +OS << "Deallocating object passed through parameter '" << PVD->getName() + << '\''; --

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-02-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Thx!~ In D58368#1404747 , @Charusso wrote: > This is a cool approach, but it is difficult to use this API in other > checkers. If you do not out-chain D58367 I > would like to see something like tha

[PATCH] D58397: [analyzer] MIGChecker: Pour more data into the checker.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:125 + // See if there's an annotated method in the superclass. + if (const auto *MD = dyn_cast(D)) dcoughlin wrote: > Perhaps we coul

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked 3 inline comments as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:47 REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool); Charusso wrote: > `;` is not necessary. Addressed in the earlier patch

[PATCH] D58366: [analyzer] MIGChecker: Make use of the server routine annotation.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187872. NoQ added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58366/new/ https://reviews.llvm.org/D58366 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp clang/test/Analysis/mig.mm Index: clang/test/Analysis/mig.mm

[PATCH] D58368: [analyzer] MIGChecker: Implement bug reporter visitors.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187873. NoQ marked an inline comment as done. NoQ added a comment. Address comments. @Charusso: I agreed not to rush for D58367 and implemented an old-style visitor here instead :) CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D58392: [analyzer] MIGChecker: Fix false negatives for releases in automatic destructors.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187875. NoQ added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58392/new/ https://reviews.llvm.org/D58392 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp clang/test/Analysis/mig.mm Index: clang/test/Analysis/mig.mm ===

[PATCH] D58397: [analyzer] MIGChecker: Pour more data into the checker.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187876. NoQ added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58397/new/ https://reviews.llvm.org/D58397 Files: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp clang/test/Analysis/mig.mm Index: clang/test/Analysis/m

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 187877. NoQ added a comment. Unblock the unrelated MIGChecker patches by untangling them from this core change. This patch will land after them and now includes an update to the checker that demonstrates the use of (and, well, tests) the new API. Comments were

[PATCH] D58529: [analyzer] MIGChecker: Enable by default.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. The TODOs for future polishing are: - Add a second `trackExpressionValue()` visitor to track argument values as they're copied around. - Introduce annotations for deallocator functions so that we no longer had to hardcode them and users could annotate their consuming functi

[PATCH] D58529: [analyzer] MIGChecker: Enable by default.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. NoQ added a comment. The TODOs for future polishing are: - Add a sec

[PATCH] D58366: [analyzer] MIGChecker: Make use of the server routine annotation.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ closed this revision. NoQ added a comment. Committed as rC354638 but i screwed up the link in the commit message. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58366/new/ https://reviews.llvm.org/D58366

[PATCH] D58529: [analyzer] MIGChecker: Enable by default.

2019-02-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ closed this revision. NoQ added a comment. Committed as rC354644 but i forgot to update the link as usual. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58529/new/ https://reviews.llvm.org/D58529 ___

[PATCH] D37120: [analyzer] Fix modeling arithmetic

2017-08-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. I guess we'd need more assertions to catch invalid symbols, will have a look. Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:371-373 +return makeSymExprValNN( +state, op, lhs.castAs(), +rhs.castAs(), re

[PATCH] D37120: [analyzer] Fix modeling arithmetic

2017-08-26 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Yeah, `LocAsInteger` is supported very poorly in most places. We can only get away with it because people rarely cast pointers to integers. Your reasoning about how `LocAsInteger` works poorly with eg. multiplication or bitwise operations (consider some sanitizer computing

[PATCH] D37103: [StaticAnalyzer] LoopUnrolling fixes

2017-08-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D37103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D36962: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (make more branches)

2017-08-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. LGTM! I guess later it'd be great to comment on every `numTimesReached()` why is it supposed to be reached exactly that many times, as it's often unclear, especially with tricky control flow. Becau

[PATCH] D37181: {StaticAnalyzer} LoopUnrolling: Keep track the maximum number of steps for each loop

2017-08-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. LGTM, thanks! Minor nit included. I hope our matchers actually cover all the cases (not a big deal if they don't though). Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:26 +#define MAXIMUM_STEP_UNROLLED 128 + I'd prefer a (`static

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

2017-08-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Thank you for the review! > Though it looks like some of the cases covered in the code do not have > corresponding tests (e.g.: the parenexprs). These are covered by tests in `inline-defensive-checks.c:150,156,169,179` (old code had `IgnoreParenCasts`). This function is ac

[PATCH] D37120: [analyzer] Fix modeling arithmetic

2017-08-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Repository: rL LLVM https://reviews.llvm.org/D37120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D37255: [analyzer] Fix bugreporter::getDerefExpr() again - a smaller targeted fix.

2017-08-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. Herald added a subscriber: xazax.hun. In order to be able to revert the risky refactoring in https://reviews.llvm.org/D37023 without bringing back the regression, i made a smaller (but uglier) isolated fix for the crash. Then https://reviews.llvm.org/D37023 will be ap

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

2017-08-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1104 +// expression classes separately. +if (!isa(Ex)) + for (auto Child : Ex->children()) { dcoughlin wrote: > What is special about ObjCBoxedExpr here? Naivel

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

2017-08-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 113101. NoQ added a comment. Fix a bit of ObjCBoxedExpr behavior. https://reviews.llvm.org/D35216 Files: lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/initializer.cpp test/Analysis/objc-boxing.m Index: test/Analysis/objc-boxing.m =

[PATCH] D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block.

2017-08-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 113102. NoQ marked an inline comment as done. NoQ added a comment. Avoid creating a new `RefVal` kind. In https://reviews.llvm.org/D36750#843427, @dcoughlin wrote: > > By the way, plist-based tests in retain-release.m are disabled since > > r163536 (~2012), and

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

2017-08-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D35216#856093, @rsmith wrote: > The `CXXStdInitializerListExpr` node has pretty simple evaluation semantics: > it takes a glvalue array expression, and constructs a > `std::initializer_list` from it as if by filling in the two members with a > p

[PATCH] D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block.

2017-09-01 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ planned changes to this revision. NoQ added a comment. This is all wrong. While `RetainCountChecker` is more function-local than, say, `MallocChecker`, we still can't say for sure that it is the bottom frame's function (or block) that should be owning the object in this case. Ideally it sho

[PATCH] D37437: [analyzer] Fix SimpleStreamChecker's output plist not containing the checker name

2017-09-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Cool. Thanks! > In the future probably it would be better to alter the signature of the > checkers' constructor to set the name in the constructor so it is possible to > create the BugType eagerly. Still, should we add an assertion so that we could be sure that every bug t

[PATCH] D37478: [analyzer] Implement pointer arithmetic on constants

2017-09-06 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. I've seen this recently, and while i agree that the fix is correct, i'm not entirely sure that the test cases are correct. As weird as this may sound, null dereference is not an attempt to read from or write to memory address 0. Instead, it is about using a null pointer as

[PATCH] D37805: [analyzer] PthreadLock: add printState().

2017-09-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. The self-debugging method to add dumps of the checker's internal state to program state dumps and exploded graphs. I'm looking into enabling this checker by default, so more patches would follow. https://reviews.llvm.org/D37805 Files: lib/StaticAnalyzer/Checkers/P

[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.

2017-09-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. `lck_mtx_lock()` returns `void`. The analyzer failed to model its effect because he was surprised that the return value is `Unknown`. Prepare for the aforementioned surprise and fix the tests accordingly. https://reviews.llvm.org/D37806 Files: lib/StaticAnalyzer/C

[PATCH] D37807: [analyzer] PthreadLock: Add the other XNU rwlock unlock functions.

2017-09-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. It turns out that XNU rwlocks, locked by `lck_rw_lock_shared()` and `lck_rw_lock_exclusive()`, can be unlocked through either the "unspecific" `lck_rw_done()` or the "specific" `lck_rw_unlock_shared()` and `lck_rw_unlock_exclusive()` calls. Add the two "specific" unlo

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

2017-09-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. Use `CallEvent` and `CallDescription` everywhere. Unhardcode argument numbers in AcquireLock() etc. Have a list of supported functions in one place. Other misc cleanup. No functional change intended anywhere. https://reviews.llvm.org/D37809 Files: lib/StaticAnalyz

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

2017-09-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 115042. NoQ added a comment. Don't forget to check that the function is a global C function in post-call. https://reviews.llvm.org/D37809 Files: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp test/Analysis/Inputs/system-header-simulator-for-pthread-lock

[PATCH] D37812: [analyzer] PthreadLock: Escape the pointers.

2017-09-13 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. As usual, we need to invalidate mutex states when something may touch them. Implement this boilerplate for the thread lock checker. The previous refactoring is handy for listing functions on which we don't need to invalidate mutex states because we model them instead.

[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.

2017-09-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:271 } -assert(lockFail && lockSucc); -C.addTransition(lockFail); - +// We might want to handle the case when the mutex lock function was inlined +// and returned an Unk

[PATCH] D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.

2017-09-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 115586. NoQ added a comment. Remove the changes in tests for now. I guess they'd need more cleanup anyway. https://reviews.llvm.org/D37809 Files: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp ===

[PATCH] D37963: [analyzer] PthreadLock: Don't track dead regions.

2017-09-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. Standard boilerplate: stop tracking mutex state when the mutex dies as a region. Clean up the destroyed symbol cleanup code a tiny bit. Note that this code is unaffected by the zombie symbol bug because whenever we need to take action, constraint manager is bound to m

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-09-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. The overall idea makes sense to me. I'd like you to join the effort with Peter who during his work on loop widening came up with a matcher-based procedure for finding out if a variable is changed anywhere; it currently lives in `LoopUnrolling.cpp` and we need only once impl

[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic

2017-09-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Looks good! I guess the accurate thing to do would be to return LocAsInteger of type `intptr_t` of an ElementRegion with index `-1` from SymbolicRegion around `p`. But i'm totally fine with another `UnknownVal` placeholder until this becomes an actual problem. Repository

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

2017-09-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 116512. NoQ added a comment. Fix some comments in tests. Devin: ok to commit? I hope i didn't miss anything in my reasoning about boxed expressions. https://reviews.llvm.org/D35216 Files: lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/initializer.cp

[PATCH] D36737: [analyzer] Store design discussions in docs/analyzer for future use.

2017-09-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 116520. NoQ added a comment. Update to use `.rst` formatting. https://reviews.llvm.org/D36737 Files: docs/analyzer/DesignDiscussions/InitializerLists.rst Index: docs/analyzer/DesignDiscussions/InitializerLists.rst

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 202092. NoQ added a comment. Remove outdated comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62638/new/ https://reviews.llvm.org/D62638 Files: clang/test/Analysis/exploded-graph-rewriter/edge.dot clang/test/Analysis/exploded-graph-rewriter/e

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 202094. NoQ added a comment. Use `os.path.join` for discovering the utility in tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62638/new/ https://reviews.llvm.org/D62638 Files: clang/test/Analysis/exploded-graph-rewriter/edge.dot clang/test/An

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. > I wrote some tests but i'm not really sure they're worth it. Mmm, on second thought, they probably won't work out of the box, because they might require installing python modules in order to work. I'm actually not sure if all machines have python3. I'll try but it'll most

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:326 +self._dump('Program point:') +self._dump('' + '') Charusso wrote: > I would create a table-buil

[PATCH] D62658: [analyzer] print() JSONify: ExplodedNode revision

2019-05-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Great, thx! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62658/new/ https://reviews.llvm.org/D62658 ___ cfe-commits m

[PATCH] D62716: [analyzer] Fix JSON dumps for dynamic types, add test.

2019-05-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Now they're valid JSON. Repository: rC Clang https://reviews.llvm

[PATCH] D62754: [analyzer] Fix JSON dumps for location contexts.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. `lctx_id` is a property of a location context, not of an item within i

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 202502. NoQ marked 12 inline comments as done. NoQ added a comment. Fxd! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62638/new/ https://reviews.llvm.org/D62638 Files: clang/test/Analysis/exploded-graph-rewriter/edge.dot clang/test/Analysis/explod

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Also switched to python2 because it seems to be the default. The differences are minimal. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62638/new/ https://reviews.llvm.org/D62638 ___ cfe-commits mailing list cfe-commit

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:151 +super(ExplodedGraph, self).__init__() +self.nodes = collections.defaultdict(ExplodedNode) +self.root_id = None Charusso wrote: > `nodes` -> `graph`

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D62638#1525843 , @Charusso wrote: > In D62638#1525823 , @NoQ wrote: > > > Also switched to python2 because it seems to be the default. The > > differences are minimal. > > > What about the s

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191 +.replace('\\>', '>') \ +.rstrip(',') +logging.debug(raw_json)

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

2019-05-31 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 202511. NoQ added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62638/new/ https://reviews.llvm.org/D62638 Files: clang/test/Analysis/exploded-graph-rewriter/edge.dot clang/test/Analysis/exploded-graph-rewriter/empty.dot clang/

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