[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

2021-02-09 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Herald added a subscriber: nullptr.cpp. LGTM! As a side note, are we aware of anyone that uses short messages instead of the full length one? Comment at: clang/lib/An

[PATCH] D90691: [analyzer] Add new checker for unchecked return value.

2021-02-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: NoQ, vsavchenko. Szelethus added a comment. Looks like a neat checker! I guess the only question is the function matching, and I don't dislike it in its current state. @martong, do you have any thoughts on this? On another note, have you checked this on some projects

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

2021-02-12 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG33e731e62dae: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that… (authored by Szelethus). Changed prior to commit: https

[PATCH] D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order

2021-02-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I don't insist on this patch, though I will end up removing the FIXME even if I leave the actual code unchanged, as it seems to be outdated. Comment at: clang/lib/Analysis/LiveVariables.cpp:522 - // FIXME: we should enqueue using post order. - fo

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

2021-02-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96163/new/ https://reviews.llvm.org/D96163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94673: [analyzer][CTU] API for CTU macro expansions

2021-02-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! You may wanna wait for someone more knowledgeable about CTU to give it another accept. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94673/new/ https://reviews.llvm.org/D

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

2021-02-18 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. As far as content goes, here are my thoughts: There is a huge block of comments in the source code: https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Chec

[PATCH] D100829: [analyzer][docs] Highlight some differences between ArrayBound and V2

2021-06-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added inline comments. This revision now requires changes to proceed. Herald added a subscriber: manas. Comment at: clang/docs/analyzer/checkers.rst:2107 - // note: requires unix.Malloc or - // alpha.unix.MallocWithAnnotat

[PATCH] D103616: [analyzer] Reimplement trackExpressionValue as ExpressionHandler

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2069-2070 +// this point. +// TODO: Shouldn't we track control dependencies of every bug location, +// rather than only tracked expressions? +if (LVState->getAnalysi

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:367-368 +/// \param Origin Only adds notes when the last store happened in a +///different stackframe to this one. Disregarded if the tracking kind +///

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2263-2264 Tracker::Tracker(PathSensitiveBugReport &Report) : Report(Report) { addHighPriorityHandler(); + addLowPriorityHandler(); // TODO: split trackExpressionValue and Fi

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I know I'm late to the party, and am just mostly thinking aloud. Awesome patch series! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:331-341 +/// Visitor that tracks expressions and values. +class TrackingBugR

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2263-2264 Tracker::Tracker(PathSensitiveBugReport &Report) : Report(Report) { addHighPriorityHandler(); + addLowPriorityHandler(); // TODO: split trackExpressionValue and Fi

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

2021-06-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Sorry for the absence, took my time to catch up with this series! Really interesting insight into how a new interface is being designed! I need some time to digest things, but can't wait to help on the remaining patches. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D104439: [analyzer][NFC] Demonstrate a move from the analyzer-configs `.def` file to a TableGen file

2021-06-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, vsavchenko, martong, balazske, steakhal, xazax.hun. Szelethus added a project: clang. Herald added subscribers: manas, dang, ASDenysPetrov, gamesh411, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whi

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

2021-06-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Aha, alright! So the tracker tracked back to where the tracked expression got computed to node `N`, which is the return value of some identity function. Unless its explicitly told that there is a link in between the return value and the parameter, the tracker can't fi

[PATCH] D102914: [analyzer] Make checker silencing work for non-pathsensitive bug reports

2021-06-17 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9cca5c1391d6: [analyzer] Make checker silencing work for non-pathsensitive bug reports (authored by Szelethus). Repository: rG LLVM Github Monorep

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

2021-06-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:945-947 +ExprEngine &Eng = C.getStateManager().getOwningEngine(); +// Let's mark this place with a special tag. +Tag = Eng.getDataTags().make(CE,

[PATCH] D104550: [analyzer] Implement getType for SVal

2021-06-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. This is not an urgent, but still a very important issue: `SVal` is likely among the first things a new developer in the analyzer sees. Its common around the university here that some students barely halfway through their BSc are given smaller tasks, like the implement

[PATCH] D104439: [analyzer][NFC] Demonstrate a move from the analyzer-configs `.def` file to a TableGen file

2021-06-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ping. The main question with this patch really is whether we want TableGen at all or not, as for what we generate precisely and how we'd utilize it is a discussion for later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D104550: [analyzer] Implement getType for SVal

2021-06-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D104550#2833020 , @vsavchenko wrote: > Do you think that "for the sake of incremental approach"-like comment can > help? A `TODO:` should be good enough for now. I see this is still a moving target. Repository: rG LLVM

[PATCH] D104136: [analyzer] Add better tracking for RetainCountChecker leak warnings

2021-06-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:953-957 + // Let's traverse... + for (const ExplodedNode *N = ExprNode; + // ...all the nodes corresponding to the given expression...

[PATCH] D104550: [analyzer] Implement getType for SVal

2021-06-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D104550#2838845 , @vsavchenko wrote: >> Another thing is that we can garantee returning `QualType`. I mean, we can >> replace `Optional` with `QualType` itself. `QualType` has a default ctor and >> `isNull` predicate, whic

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

2021-07-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D106262#2915764 , @balazske wrote: > The bug type is passed to `constructNoteTag` only to identify what message > will be displayed. The bug types that are related to the current function (a > message should be here if the

[PATCH] D105819: [analyzer] MallocChecker: Add a visitor to leave a note on functions that could have, but did not change ownership on leaked memory

2021-08-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Maybe if I added this under an alpha checker option? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105819/new/ https://reviews.llvm.org/D105819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

2021-08-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. How about we just change the NoteTag to this: "Failed stream operation could have left the error or eof flags set, or the file position indicator indeterminate"? We could add a `NoteTags` to `feof` and `ferror` that narrows this down, for example: fread(F); // note

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

2021-08-09 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Alright, I think we're almost ready to go! I left a few comments, please make sure to mark those done that you feel are properly addressed. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:393-394 + /// in this sense if a resource lea

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

2021-08-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:696 // Add transition for the failed state. Optional RetVal = makeRetVal(C, CE).castAs(); assert(RetVal && "Value should be NonLoc."); balazske wrote: > Szel

[PATCH] D105819: [analyzer] MallocChecker: Add a visitor to leave a note on functions that could have, but did not change ownership on leaked memory

2021-08-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 366269. Szelethus set the repository for this revision to rG LLVM Github Monorepo. Szelethus added a comment. - Add the new feature behind a new, off-by-default alpha checker option. - Restore `test/Analysis/self-assign.cpp` (to be formatted in a standalone

[PATCH] D105553: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract NoStateChangeVisitor class

2021-08-16 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc019142a89b4: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract… (authored by Szelethus). Repository: rG LLVM Github Monor

[PATCH] D104550: [analyzer] Implement getType for SVal

2021-06-29 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. Really appreciate the unit tests! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104550/new/ https://reviews.llvm.org/D104550 ___ cfe-commits m

[PATCH] D105552: [analyzer][NFC] NoStoreFuncVisitor: Compact parameters for region pretty printing into a class

2021-07-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, vsavchenko, steakhal, martong, ASDenysPetrov. Szelethus added a project: clang. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity.

[PATCH] D105553: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract NoStateChangeVisitor class

2021-07-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, vsavchenko, martong, steakhal, ASDenysPetrov. Szelethus added a project: clang. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, sunfish, rnkovacs, szepet, baloghadamsoftware, xazax.hun, wh

[PATCH] D105552: [analyzer][NFC] NoStoreFuncVisitor: Compact parameters for region pretty printing into a class

2021-07-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D105552#2861924 , @vsavchenko wrote: > Sorry about being pretty harsh here. I do think that `NoStoreFuncVisitor` > needs refactoring and better abstractions, I just don't think that this is > the way to go. Nah, you're t

[PATCH] D105552: [analyzer][NFC] NoStoreFuncVisitor: Compact parameters for region pretty printing into a class

2021-07-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Actually, one thing might be worth considering. The grand idea is to split up `NoStoreFuncVisitor` so that its clients only need to define these 3 (quoting from D105553 ): > - What constitutes as a change to an entity > - What the dia

[PATCH] D105553: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract NoStateChangeVisitor class

2021-07-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 357202. Szelethus edited the summary of this revision. Szelethus added a comment. - Don't depend on D105552 . - Merge with a followup patch; it was a poor splitting point, and the diffs didn't really convey what actually ch

[PATCH] D105819: [analyzer] MallocChecker: Add a visitor to leave a note on functions that could have, but did not change ownership on leaked memory

2021-07-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, vsavchenko, steakhal, martong, ASDenysPetrov. Szelethus added a project: clang. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity,

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: aaron.ballman, alexfh, steakhal, whisperity. Szelethus added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, martong, gamesh411, dkrupp, rnkovacs, xazax.hun. Szelethus requested review of this revision. Herald added

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 385459. Szelethus edited the summary of this revision. Szelethus added a comment. Clarify the summary. Delete unnecessary includes. More fitting `iterator` names in the test files. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113201/new/ https://

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

2021-11-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, steakhal, dkrupp, martong, vsavchenko, balazske. Szelethus added a project: clang. Herald added subscribers: manas, ASDenysPetrov, gamesh411, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, w

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

2021-11-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 385481. Szelethus added a comment. Fix incorrect sectioning. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113397/new/ https://reviews.llvm.org/D113397 Files: clang/docs/analyzer/checkers.rst Index: clang/docs/analyzer/checkers.rst ===

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

2021-11-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 385486. Szelethus added a comment. Add context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113397/new/ https://reviews.llvm.org/D113397 Files: clang/docs/analyzer/checkers.rst Index: clang/docs/analyzer/checkers.rst

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

2021-11-09 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8cc2de667ec2: [analyzer][docs] Fix the incorrect structure of the checker docs (authored by Szelethus). Changed prior to commit: https://reviews.l

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: Eugene.Zelenko. Szelethus added a comment. I'll intend to land this by friday unless there are objections! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113201/new/ https://reviews.llvm.org/D113201 ___ cfe-commits

[PATCH] D97936: [analyzer][docs][NFC] Fix typo in checkers.rst

2021-03-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Neat! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97936/new/ https://reviews.llvm.org/D97936 __

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-15 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked 2 inline comments as done. Closed by commit rG29a8d45c5a23: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators (authored by Szelethus). Changed prior to commit: https://reviews

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Seems like the issues mentioned above are real, but orthogonal to this patch. Would it be okay to address them in followup patches? @martong @NoQ Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:927 SVal RetVal = State->getSVal(CNE, L

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked an inline comment as done. Closed by commit rGa504ddc8bf9d: [analyzer] Initialize regions returned by CXXNew to undefined (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D135375

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, martong, steakhal, balazske, isuckatcs. Szelethus added a project: clang. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Her

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Just a note on the test files -- I've diverged from the usual stance of just changing what the new output is, to modifying the test files. The reason is that reading an undefined value is a fatal error, leading to the analyzer to stop analyzing prematurely, and I thin

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Some early results: https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxne

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D137790#3992216 , @balazske wrote: > Some reports can be found here > (if the > link works and the data does not expire), the runs stored on 2022-12-09. > >

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:219 + /// @param State The state of the generated node. + /// @param Pred The transition will be generated from the specified Pred node + /// to the

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D137790#3992216 , @balazske wrote: > On the postgres results, the second is one that can be fixed in the checker > (add special cases to `StdLibraryFunctionsChecker` for zero `len` or `size` > `fread` and `fwrite` arguments

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I have a fear that we may have too few results as well -- maybe we should expand our testing infrastructure with more POSIX API heavy codebases. Looking at a few new projects, https://github.com/audacity/audacity looks like a good candidate, but of course it often tur

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2022-12-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: gamesh411. Szelethus added a comment. Some of the changes are also present in D135247 . I suppose you're in the middle of splitting those patches apart and remaking the patch stack? Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2022-12-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D140387#4007751 , @balazske wrote: > This patch and D140395 is (almost) the > same code as D135360 and D135247 > . The

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Would be possible to test the errno specific changes as well? Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:310 + const MemRegion *ErrnoR = State->get(); + if (!ErrnoR) +return State; When can this occur? Maybe

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Does this patch fix any false positives from before, or is this just all new stuff? I ask, because I wonder whats the shortest path towards popping these checkers out of alpha, and fix what we already have. By no means am I saying that we should postpone landing this,

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Are you sure that the refactoring made no changes to the results? Could you maybe just run a nightly or something like that to confirm? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137790/new/ https://reviews.llvm.org/D

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D140387#4021806 , @Szelethus wrote: > Would be possible to test the errno specific changes as well? I suppose the tests are done in the followup patch mostly? Comment at: clang/lib/StaticAnalyzer/Checkers

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838 + Result += getArgDesc(ArgN); + Result += DK == Violation ? " should not be zero" : " is not zero"; + return Result.c_str(); I don't mean to make

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Mostly LGTM, but I see that you have tests for the predecessor patch here as well, so I'll accept both at once. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:89 /// The last file operation called in the stream. + /// Can be null

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. LGTM, granted you add that test in the followup commit. If possible, I'd prefer to have features tested in the patch that added them (but this is fine for now). Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838 + Resul

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140395/new/ https://reviews.llvm.org/D140395 _

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Please rerun the evaluation before commiting to confirm the results haven't changed! Otherwise, LGTM. Comment at: clang/test/Analysis/stream.c:89 void check_freopen_1

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:507-511 + // FIXME: Use a return value from EvalFn instead of isDifferent. + // Some functions should not change state and not have any other + // (invalidation, including errno) e

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. LGTM! I think I prefer this solution anyways. Please commit (the entire patchstack). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140387/new/ https://reviews.llvm.org/D140387 ___

[PATCH] D142354: [analyzer] Create a stub for an std::variant checker

2023-01-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D142354#4078450 , @NoQ wrote: > Interesting, what specific goals do you have here? Are you planning to find > specific bugs (eg. force-unwrap to a wrong type) or just to model the > semantics? Hi! Meant to write this comm

[PATCH] D142354: [analyzer] Create a stub for an std::variant checker

2023-02-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. While we were there, we also dug into `std::any`, and learned that the analyzer can model it shockingly well. Hopefully we can submit a few patches that demonstrates it in a form of some test files. In D142354#4079758 , @steak

[PATCH] D143194: [clang][analyzer] Make messages of StdCLibraryFunctionsChecker user-friendly

2023-02-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: gamesh411. Szelethus added a comment. Awesome, been a long time coming!! Other than the minor observation of changing "of function" to "to", I'm inclined to accept this patch. We definitely should describe what the value IS, not just what is should be (aside from the

[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

2023-08-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, steakhal, donat.nagy, balazske, gamesh411. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, yaxunl. Herald added a reviewer: aaron.ballman. Herald add

[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

2023-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 548116. Szelethus marked 4 inline comments as done. Szelethus added a comment. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156787/new/ https://reviews.llvm.org/D156787 Files: clang/include/clang/Basic/Att

[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

2023-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1189 +``onwership_returns``: Functions with this annotation return dynamic memory. +The second annotation parameter is the size of the returned memory in bytes. + aaron.ballman wro

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

2023-07-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. But this renders much of my C++98 knowledge obsolete!11 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154325/new/ https://reviews.llvm

[PATCH] D76509: [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

2020-03-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, martong, balazske, rnkovacs, dcoughlin, steakhal. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, usaxena95, Charusso, gamesh411, dkrupp, donat.nagy, kadircet, m

[PATCH] D76510: [analyzer] Change the default output type to PD_TEXT_MINIMAL, error if an output loc is missing for PathDiagConsumers that need it

2020-03-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, rnkovacs, martong, balazske, dcoughlin. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sid

[PATCH] D76509: [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

2020-03-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:148 void DigestAnalyzerOptions() { -if (Opts->AnalysisDiagOpt != PD_NONE) { - // Create the PathDiagnosticConsumer. - C

[PATCH] D76509: [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 251973. Szelethus added a comment. Unbreak clang-tidy. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76509/new/ https://reviews.llvm.org/D76509 Files: clang/include/clang/StaticAnalyzer/Core/Analyses.def clang/lib/StaticAnalyzer/Core/CMakeLis

[PATCH] D70411: [analyzer] CERT: STR31-C

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/docs/analyzer/checkers.rst:1935 + +alpha.security.cert.str.31c +""" balazske wrote: > There are already more checkers that can check for CERT related problems but > not specially made for

[PATCH] D76605: [analyzer] Display the checker name in the text output

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, baloghadamsoftware, balazske, martong, xazax.hun, dcoughlin, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sid

[PATCH] D76605: [analyzer] Display the checker name in the text output

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 3 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:98 const PathDiagnostic *PD = *I; + std::string WarningMsg = + (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() +

[PATCH] D76509: [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 15 inline comments as done. Szelethus added a comment. Thanks, @martong! Comment at: clang/include/clang/StaticAnalyzer/Core/Analyses.def:61 + +ANALYSIS_DIAGNOSTICS(TEXT_MINIMAL, "text-minimal", + "Emits minimal diagnostics to stderr, statin

[PATCH] D76510: [analyzer] Change the default output type to PD_TEXT_MINIMAL, error if an output loc is missing for PathDiagConsumers that need it

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:26 #include "clang/CrossTU/CrossTranslationUnit.h" +#include "clang/Driver/DriverDiagnostic.h" #include "clang/Frontend/CompilerInstan

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG57b8a407493c: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D75360?vs=251056&id=252073#toc R

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D75360#1937415 , @JDevlieghere wrote: > It appears this broke the modules build: > http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12905/console > > Can you please take a look? Yup, on it. Repository: rG LLVM

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D75360#1937557 , @JDevlieghere wrote: > Oh, I'm sorry, I just reverted it like a minute ago. Its fine, I'll recommit when I have a fix as we

[PATCH] D76509: [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

2020-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked 6 inline comments as done. Closed by commit rG7bf871c39f73: [analyzer][NFC] Move the text output type to its own file, move code to… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.

[PATCH] D76605: [analyzer] Display the checker name in the text output

2020-03-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 3 inline comments as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:313-316 +ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name", +"Display the checker name

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

2020-03-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Herald added a subscriber: ASDenysPetrov. In D71524#1924508 , @steakhal wrote: > I think `CallDescription` can only identify objects/functions which has > `IdefntifyerInfo` in them. AFAIK operators don't have such. Though someho

[PATCH] D63279: [Analyzer] Unroll for-loops where the upper boundary is a variable with know value

2020-03-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Herald added a subscriber: ASDenysPetrov. (note: I forgot to submit this a couple weeks ago) LLVM is crashing on me due to the issue mentioned in D75678 , but on Bitcoin, Xerces, CppCheck and Rtags I observed no difference in between t

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542 +BR.markInteresting(It1); +if (const auto &LCV1 = It1.getAs()) { + BR.markInteresting(LCV1->getRegion()); +} NoQ wrote: >

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

2020-03-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D75682#1931108 , @balazske wrote: > Adding special test functions is not as easy, then `StreamState` should be > accessible from another checker. It could be added to the same file, or new > file but then moving the data str

[PATCH] D76604: [Analyzer] Model `size()` member function of containers

2020-03-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:456 + SVal RetVal) const { + const auto *ContReg = Cont.getAsRegion(); + if (!ContReg) martong wrote: > Just out of curiosity:

[PATCH] D71612: [analyzer] Add PlacementNewChecker

2020-03-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Herald added subscribers: ASDenysPetrov, steakhal. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:1 +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugTyp

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Tried to recommit -- I'll keep a close eye on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75360/new/ https://reviews.llvm.org/D75360 ___ cfe-commits mailing list cfe-

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2aac0c47aed8: Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D75360?vs=252073&id=252861#toc Repository:

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D76830#1943133 , @balazske wrote: > FIXME: There is a test file "kmalloc-linux.c" but it seems to be > non-maintained and buggy (has no //-verify// option so it passes always but > produces multiple warnings). Crap, even I

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/test/Analysis/kmalloc-linux-1.c:15 + +// FIXME: malloc checker expects kfree with 2 arguments, is this correct? +// (recent kernel source code contains a `kfree(const void *)`) balazske wrote: > Szelethus wrote:

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687 + if (ArgValKnown) { +if (!KernelZeroSizePtrValue) + KernelZeroSizePtrValue = Szelethus wrote: > martong wrote: > > balazske wrote: > > > martong wrote

<    8   9   10   11   12   13   14   15   16   >