[PATCH] D82445: [analyzer][solver] Track symbol equivalence

vsavchenko added a comment. In D82445#2284680 , @martong wrote: > Hi Valery, > > Together with @steakhal we have found a serious issue. > The below code crashes if you compile with `-DEXPENSIVE_CHECKS`. > The analyzer goes on an unfeasible path, the State

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

vsavchenko added a comment. I came up with exactly the same fix! Great job! I just wanted to refactor it and not having if (New.isEmpty()) // this is infeasible assumption return nullptr; ProgramStateRef NewState = setConstraint(St, Sym, New); return trackNE(NewState, Sym, Int, A

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

vsavchenko added a comment. After this "accepted" and a huge thank you 😄 Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1350-1371 ProgramStateRef trackEQ(ProgramStateRef State, SymbolRef Sym, const llvm::APSInt &Int,

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Amazing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88019/new/ https://reviews.llvm.org/D88019 _

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

vsavchenko added a comment. Hi @baloghadamsoftware Great job! I also wanted to support more operations for range-based logic. However, I don't think that this is the right place to make this kind of assumptions. A couple months ago, I added the `SymbolicRangeInferrer` component to aggregate

[PATCH] D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Awesome! I will submit the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85105/new/ https://reviews.llvm.org/D85105 ___ cfe

[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351 +bool SmartPtrModeling::handleEqOp(const CallEvent &Call, + CheckerContext &C) const { xazax.hun wrote: > I'd prefer to cal

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:404 +void SmartPtrModeling::handleBoolOperation(const CallEvent &Call, + CheckerContext &C) const { I suggest `BoolCon

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2231760 , @NoQ wrote: > I believe @vsavchenko's docker thingy already knows how to do that. Yep, it sure does! Additionally, there is a `benchmark` subcommand that can chart memory consumption for measured projects.

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2233157 , @steakhal wrote: > In D86295#2233076 , @vsavchenko > wrote: > >> In D86295#2231760 , @NoQ wrote: >> >>> I believe @vsavchenko

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2233398 , @steakhal wrote: > In D86295#2233244 , @vsavchenko > wrote: > >> Here is a short summary how to do regression testing (check that all >> warnings are the same): > >

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaec12c1264ac: [analyzer][tests] Add a notion of project sizes (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

vsavchenko added a comment. In D86295#2233666 , @steakhal wrote: > In D86295#2233401 , @vsavchenko > wrote: > >> Yep, I guess that is the cause. I'll take a look. Did you try it with this >> fast fix? > > I trie

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko created this revision. vsavchenko added reviewers: NoQ, dcoughlin, xazax.hun, Szelethus, ASDenysPetrov, steakhal. Herald added subscribers: cfe-commits, martong, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, mgrang, rnkovacs, szepet, baloghadamsoftware. Herald added a proje

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:88 + // structure is preferred. + using ImplType = llvm::SmallVector; + grandinj wrote: > Just curious - if they mostly contain 1

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added a comment. In D86465#2235289 , @NoQ wrote: > If the numbers are confirmed to be as good as what i've sneak-peeked so far, > that should be pretty amazing. Also whoa, test coverage!~ I'll add the charts with performance in the next couple

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:119 + + return makePersistent(std::move(Result)); +} NoQ wrote: > Given that we're certain from the start that the container will be > persistent, can we save

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:88 + // structure is preferred. + using ImplType = llvm::SmallVector; + vsavchenko wrote: > NoQ wrote: > > vsavchenko wrote: > > >

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:110 + iterator end() const { return Impl->end(); } + size_t size() const { return Impl->size(); } + steakhal wrote: > I think it shoul

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

vsavchenko added a comment. In D86533#2236009 , @Szelethus wrote: > @vsavchenko I just realized a significant portion of your work for this > release is the following list: > > git log llvmorg-11-init..llvmorg-11.0.0-rc2 --oneline -- > clang/utils/anal

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added a comment. Here are some benchmarking results. In docker: F12777445: tiny.png F12777444: small.png Natively on my Mac (no memory measurements due to this issue

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko updated this revision to Diff 288306. vsavchenko marked 21 inline comments as done. vsavchenko added a comment. Fix review remarks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86465/new/ https://reviews.llvm.org/D86465 Files: clang/

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

vsavchenko added a comment. In D86465#2238614 , @martong wrote: > These are really promising figures, nice work! (And the measurement stuff > itself is also a great addition, thanks for that!) Thanks 😊 > Not that it would matter much, but I was just won

[PATCH] D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h

This revision was automatically updated to reflect the committed changes. Closed by commit rG9300ca541164: [doxygen] Fix bad doxygen results for BugReporterVisitors.h (authored by OikawaKirie, committed by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

vsavchenko accepted this revision. vsavchenko added a comment. In D86533#2248856 , @Szelethus wrote: > I'd prefer to have a couple more green lights. In particular, another look on > the constraint manager and Objective C stuff would be great. Those part

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

vsavchenko requested changes to this revision. vsavchenko added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:311 +const auto *PE = cast(E); +return makeLoc(getRegionManager().getStringRegion(PE->

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

vsavchenko added a comment. Awesome! Can we cover `__builtin_unique_stable_name` as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87004/new/ https://reviews.llvm.org/D87004 ___ cfe-commits mailing

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

vsavchenko added inline comments. Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:101 +// Such functions have no function name of predefined expressions such as: '__func__' etc. +clang_analyzer_warnIfReached(); + } Also can you please add `//

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Thanks! Great job! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87004/new/ https://reviews.llvm.org/D87004 ___

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

vsavchenko accepted this revision. vsavchenko added a comment. LGTM! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https://reviews.llvm.org/D84415 ___ cfe-commits mailing list cfe-comm

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added a comment. @aaron.ballman Can you please take a look at the attribute side of this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 ___ cfe-commi

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

vsavchenko updated this revision to Diff 310481. vsavchenko added a comment. Replace let with const Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92639/new/ https://reviews.llvm.org/D92639 Files: clang/include/clang/Analysis/PathDiagnostic.h c

[PATCH] D92928: [analyzer] Highlight arrows for currently selected event

vsavchenko created this revision. Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. vsavchenko requested review of this revision. Herald added a project: clang. Herald added a sub

[PATCH] D92928: [analyzer] Highlight arrows for currently selected event

vsavchenko added a comment. Here is the HTML file for the test.F14639004: report-399795.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92928/new/ https://reviews.llvm.org/D92928 ___

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

vsavchenko created this revision. Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: aaron.ballman. vsavchenko requested review of this revision. Herald ad

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

vsavchenko added a comment. Right now, I reused existing `suppress` attribute. However I did it in a rather unconventional manner. I allowed 0 arguments in one spelling and >1 in another, which seems odd. I can see a couple other possible solutions here: - Choose a "keyword" that would be use

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko marked an inline comment as done. vsavchenko added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1774 + let LangOpts = [ObjC]; + let Documentation = [Undocumented]; +} aaron.ballman wrote: > No new undocumented attributes, please. Su

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 311565. vsavchenko marked 4 inline comments as done. vsavchenko added a comment. Add documentation for new attribute and fix review remarks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://re

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 311898. vsavchenko added a comment. Refine conventions for completion handlers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 311903. vsavchenko added a comment. Fix minor things Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analyses/CalledOnceCheck.h

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 311930. vsavchenko added a comment. Introduce a suppression for double call warning by nullifying the parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 File

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 312149. vsavchenko added a comment. Turn off conventional check heuristic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analy

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 312155. vsavchenko added a comment. Add more info to the docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analyses/CalledOn

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2907 +// start with it. +llvm::SmallVector Stack{DynTypedNode::create(*BugStmt)}; + jkorous wrote: > Since IIUC a node can have multiple parents - does that mean w

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added a comment. In D92039#2457867 , @aaron.ballman wrote: > Is the attribute considered to be a property of the parameter or a property > of the function the parameter is declared in? e.g., > > void someOtherFunc(void (^cb)(void)) { > i

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

vsavchenko added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2366 let Args = [VariadicStringArgument<"DiagnosticIdentifiers">]; let Documentation = [SuppressDocs]; } aaron.ballman wrote: > The documentation will need to be updated for th

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

vsavchenko added a comment. In D93110#2458613 , @aaron.ballman wrote: > Have you explored how this attribute will work with clang frontend > diagnostics or clang-tidy diagnostics? Actually, this attribute is not used anywhere in the codebase (even in `

[PATCH] D93464: [analyzer] Refine suppression mechanism to better support AST checks

vsavchenko created this revision. Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. vsavchenko requested review of this revision. Herald added a project: clang. Herald add

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added a comment. In D92039#2458392 , @aaron.ballman wrote: > > There have been efforts to add cross-TU support to the static analyzer, so > I'm less worried about cross-TU inter-procedural bugs over the long term as I > would expect that

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 312741. vsavchenko added a comment. Detect all conventions for captured parameters as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/cla

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added a comment. In D92039#2462889 , @aaron.ballman wrote: > Your test cases suggest that this is not inter-procedurally checked; it seems > to require all of the APIs to be annotated in order to get the same effect > that an inter-procedural

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added inline comments. Comment at: clang/test/SemaObjC/attr-called-once.m:7 +void test2(double x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}} + +void test3(void (*foo)() CALLED_ONCE); // no-error ---

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko created this revision. Herald added a subscriber: jdoerfert. vsavchenko requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Before this commit, expression statements could not be annotated with statement attributes. Whenever parser

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko updated this revision to Diff 313316. vsavchenko added a comment. Herald added a subscriber: arphaman. Fix block.c test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/ https://reviews.llvm.org/D93630 Files: clang/lib/Parse/Pa

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko added a comment. @aaron.ballman I totally agree, but I also would like to understand. `__attribute__` is a GNU extension, right? Then why does it affect the grammar of C? I always thought that attributes should be somewhat transparent for parsers, but it looks like in this situatio

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko added a comment. In D93630#2468197 , @aaron.ballman wrote: > However, taking a step back -- what attributes would need this functionality > (and couldn't be written on something within the expression statement)? It is still good old `suppress

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko updated this revision to Diff 313362. vsavchenko added a comment. Maintain previous behavior unless preceeded by a statement attribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/ https://reviews.llvm.org/D93630 Files: cla

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added inline comments. Comment at: clang/include/clang/Analysis/Analyses/CalledOnceCheck.h:32 +/// \enum SwitchSkipped -- there is no call if none of the cases appies. +/// \enum LoopEntered -- no call when the loop is entered. +/// \enum LoopSkipped -- no call when th

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:163 + } + static bool canBeCalled(Kind K) { +return (K & DefinitelyCalled) == DefinitelyCalled && K != Reported; NoQ wrote: > "Can be called" sounds like "Someone's allowed

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko updated this revision to Diff 313550. vsavchenko added a comment. Refine condition for statement attributes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/ https://reviews.llvm.org/D93630 Files: clang/lib/Parse/ParseStmt.cpp

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

vsavchenko added a comment. In D93630#2468853 , @aaron.ballman wrote: > Yeah, I kind of figured that might be the cause. I'm not 100% convinced (one > way or the other) if the suppress attribute should get a GNU spelling. The > `[[]]` spellings are avai

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

vsavchenko added a comment. LGTM! But I'd like other folks to take a look at it first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91898/new/ https://reviews.llvm.org/D91898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko created this revision. vsavchenko added reviewers: aaron.ballman, dexonsmith, rsmith, Bigcheese, rjmccall, NoQ, doug.gregor, ravikandhadai, dcoughlin. Herald added subscribers: cfe-commits, Charusso, mgorny. Herald added a project: clang. vsavchenko requested review of this revision. T

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko added a comment. In D92039#2414314 , @Quuxplusone wrote: > Probably irrelevant comment from the C++ world: If I needed this concept in > C++, I'd probably piggyback on the existing semantic analysis of `std::move`: > I'd design a `class OnceCa

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 307408. vsavchenko added a comment. Fix tests and a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analyses/CalledOnceChe

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 307569. vsavchenko added a comment. Herald added a subscriber: jdoerfert. Add more comments and fix another test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Fil

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko marked 2 inline comments as done. vsavchenko added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:76 + +// Defined elements should maintain the following properties: +// 1. for any Kind K: NoReturn | K == K NoQ wrote: > M

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

vsavchenko updated this revision to Diff 307594. vsavchenko marked an inline comment as done. vsavchenko added a comment. Add another comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/i

[PATCH] D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64

vsavchenko added a comment. That's a good fix! How did this happen though that max value of `off_t` was even used for `fd`. Seems pretty odd! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92307/new/ https://reviews.llvm.org/D92307 __

[PATCH] D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64

vsavchenko accepted this revision. vsavchenko added a comment. In D92307#2422603 , @martong wrote: > In D92307#2422468 , @vsavchenko > wrote: > >> That's a good fix! >> How did this happen though that max value of

[PATCH] D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis.

vsavchenko accepted this revision. vsavchenko added a comment. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92432/new/ https://reviews.llvm.org/D92432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

vsavchenko created this revision. vsavchenko added reviewers: NoQ, xazax.hun, Szelethus, steakhal, martong, ASDenysPetrov. Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. vsavchenko requested review of this revision. Herald

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

vsavchenko added a comment. Here is how it looks for that test file: F14534708: report-6ea17d.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92639/new/ https://reviews.llvm.org/D92639 __

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

vsavchenko added a comment. In D92639#2433303 , @OikawaKirie wrote: > It is really a good idea! Thanks 😊 > The operations that would not leave an event in the report are now clearly > printed. > > But there are three arrows that confuse me in the exampl

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

vsavchenko updated this revision to Diff 309497. vsavchenko added a comment. Fix incorrect comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92639/new/ https://reviews.llvm.org/D92639 Files: clang/include/clang/Analysis/PathDiagnostic.h cl

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

vsavchenko added a comment. I honestly don't see a reason why it's not a part of `forFunction`. `forFunction` matches C++ methods and lambdas, Obj-C methods and blocks don't seem that much more special to have an extended matcher just for those. I really don't think that it will break someone's

[PATCH] D102303: [ASTMatchers] Fix formatting around forFunction().

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. It always pleases me when more code becomes compliant to our style guides. Thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102303/n

[PATCH] D102294: [clang-tidy] bugprone-infinite-loop: React to ObjC ivars and messages in condition.

vsavchenko added a comment. Tests look thorough, it should only reduce the number of warnings and should only affect Obj-C. So, I'd say that it's safe and sound! Great job! Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:64-66 + } else if (isa(Cond) ||

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

vsavchenko added a comment. Great job on the patch! Thanks! Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:166-167 -static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) { +static bool isCapturedByReference(const VarDecl *VD, ExplodedNode *N, +

[PATCH] D99260: [analyzer] Fix false positives in inner pointer checker (PR49628)

vsavchenko added a comment. In D99260#2751967 , @steakhal wrote: > In D99260#2704102 , @NoQ wrote: > >> In https://bugs.llvm.org/show_bug.cgi?id=45786 the godbolt link shows that >> there are still problems with `a

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:147 +/// where N = size(LHS), M = size(RHS) +RangeSet unite(RangeSet Original, RangeSet RHS); +/// Create a new set by uniting gi

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:250 +/// guarantee this. +ContainerType unite(const ContainerType &LHS, const ContainerType &RHS); ASDenysPetrov wrote: > vsav

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

vsavchenko added a comment. In D102213#2755963 , @NoQ wrote: > I totally agree that changing forFunction() to work correctly is the right > solution but backwards compatibility breakage is very real because > forFunction() accepts a Matcher whereas forC

[PATCH] D102240: [analyzer][solver] Prevent use of a null state

vsavchenko updated this revision to Diff 345145. vsavchenko added a comment. Modify test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102240/new/ https://reviews.llvm.org/D102240 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

[PATCH] D102240: [analyzer][solver] Prevent use of a null state

vsavchenko added a comment. In D102240#2750976 , @steakhal wrote: > Dam, we will be always haunted by these. Yep, it was one place where we didn't expect a null state, but after one of the later patches, it became possible. 🤞it's the final one. > Looks

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

vsavchenko added a comment. In D102213#2757253 , @NoQ wrote: > In D102213#2756254 , @vsavchenko > wrote: > >>> Additionally, forCallable(functionDecl()) is not equivalent to >>> forFunction() (the former silentl

[PATCH] D102240: [analyzer][solver] Prevent use of a null state

This revision was automatically updated to reflect the committed changes. Closed by commit rG45212dec01b9: [analyzer][solver] Prevent use of a null state (authored by vsavchenko). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102240/new/ https://rev

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. OK, I agree with Aaron that having a separate matcher is reasonable. Thanks for the patient explanation :) LGTM from my side then. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D102214: [clang-tidy] bugprone-infinite-loop: forFunction() -> forCallable().

vsavchenko accepted this revision. vsavchenko added a comment. This revision is now accepted and ready to land. Now, when we are done with `forCallable`, let's get to this one. It looks great to me, thanks for fixing it! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://r

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

vsavchenko added a comment. Thanks for addressing my comments! I still have some left though Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:171 + const Decl *D = LocCtxt->getDecl(); + const auto *MD = dyn_cast_or_null(D); + assert(MD && MD->getParent()->isLambda

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186 + return FD->getType()->isReferenceType(); +} else { + assert(false && "Unknown captured variable"); +} AbbasSabra wrote: > vsavchenko wrote: >

[PATCH] D102683: [analyzer] Check the checker name, rather than the ProgramPointTag when silencing a checker

vsavchenko added a comment. Oh, I'm late to the party! Glad to see you back @Szelethus ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102683/new/ https://reviews.llvm.org/D102683 ___ cfe-commits mailing

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

vsavchenko added a comment. My take on this: for whatever approach, we definitely will be able to construct a more nested/complex example so that it doesn't work. For this patch, I'm wondering about something like this: int foo() { if (z != 0) return 0; if (x + y + z != 0)

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h:36 + +class FindWhereConstrained : public BugReporterVisitor { +private: nit: class name should be a noun (functions and methods are verbs) Comment at: c

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

vsavchenko added a comment. In D102696#2773304 , @martong wrote: >> As for the solver, it is something that tormented me for a long time. Is >> there a way to avoid a full-blown brute force check of all existing >> constraints and get some knowledge abo

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

vsavchenko added a comment. In D102696#2774169 , @martong wrote: > In D102696#2773340 , @vsavchenko > wrote: > >> In D102696#2773304 , @martong >> wrote: >> As for

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h:37 + +class GetNoteVisitor : public BugReporterVisitor { +private: Sorry for picking on it again, but is it visiting "get notes"? Otherwise, it is just a placeholder name

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

vsavchenko added a comment. To be honest, I don't think that it solves the problem I mentioned before. The fact that conditions and branching are part of `operator++` now, doesn't cancel them. I noticed that you made the first loop, so we don't need to check for `Min` in the main loop, and th

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:257 + // BoundCounter is 0 for outer `)`. + // BoundCounter is 1 for outer '(' or inner `)`. + // BoundCounter is 2 for inner `(`. ASDenysPetrov wrote: > vsa

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h:57 + + void visitIfStmt(const ExplodedNode *Node, const ProgramStateRef State, + BugReporterContext &BRC, const Stmt *S, IMO, these three `visit` functio

<    1   2   3   4   5   6   7   8   9   >