[PATCH] D75271: [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.

Szelethus added a comment. Mind that `AnalysisManager`, similarly to `CheckerManager` before D75360 , must be constructed with an `ASTContext` which is not available at the moment when the `shouldRegister*` functions run, and they must run no later than when the

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

Szelethus added a comment. Phabricator didn't pick up on it (because I accidentally used `mv` instead of `git mv`), but `CheckerRegistration.h` was moved to `AnalyzerHelpFlags.h`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75360/new/ https://reviews.llvm.org/D75360 __

[PATCH] D75271: [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.

Szelethus updated this revision to Diff 247292. Szelethus retitled this revision from "[Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions" to "[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.". Szelethus edited the summary of this

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

Szelethus updated this revision to Diff 247291. Szelethus added a comment. Unforget git add. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75360/new/ https://reviews.llvm.org/D75360 Files: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h clang/include/clang/StaticAnalyzer/F

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Szelethus commandeered this revision. Szelethus edited reviewers, added: baloghadamsoftware; removed: Szelethus. Szelethus added a comment. Yoink. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75271/new/ https://reviews.llvm.org/D75271 _

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

Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, rnkovacs, balazske, martong, dcoughlin. Szelethus added a project: clang. Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, w

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! It makes much more sense to do the checking in `preCall` and the modeling in `evalCall`, the comments in the patch are precise and helpful, the tests cover everything. While I feel

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Let's change this to either `CheckerManger` or `AnalysisManager`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75271/new/ https

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Szelethus added a comment. In D75271#1896182 , @Szelethus wrote: > > Also this entire callback should be removed ideally: it has to be a virtual > > function defaulting to `return true;` and if someone needs this feature > > could rewrite the behavior. I

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Szelethus added a reviewer: Charusso. Szelethus added a comment. In D75271#1895900 , @Charusso wrote: > I am so sorry to mention, but we need the `AnalysisManager` to pass around > which manages the analysis, therefore it knows both the `LangOptions` and

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus added a comment. To summarize, we're moving every stream check (illegal call to a stream management function with a null stream, or a closed stream, etc) to `preCall`, and leave only the modeling portions in `evalCall`. Makes sense! You did an amazing job here! I like the new infrastr

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Im sorry, i though we talked about internal code and we're stuck with the checker interface -- this should definitely be solved by changing the parameter of the `shouldRegister

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

Szelethus added a comment. Aha, so we're moving every check as to whether the stream is closed to `preCall`? Makes sense, if not much else changed. Could you please run `clang-format-diff.py` after adding the tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D75158: [analyzer][StreamChecker] Using function description objects - NFC.

Szelethus accepted this revision. Szelethus added reviewers: baloghadamsoftware, NoQ, martong, Charusso, xazax.hun. Szelethus added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Cool! Though this patch says little without followups, so maybe we s

[PATCH] D75021: [clang][analyzer] Enable core.builtin even with -no-default-checks

Szelethus added a comment. @Charusso implemented a flag that can //silence// checkers in D66042 . I can offer that as an alternative while we're working on separating modeling and reporting in the checker insfrastructure. Repository: rG LLVM Github Monorepo

[PATCH] D75021: [clang][analyzer] Enable core.builtin even with -no-default-checks

Szelethus added reviewers: NoQ, Szelethus, baloghadamsoftware. Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. This doesn't seem to be in line with the current idea behind `core` checkers. Turning them off is strongly disenco

[PATCH] D68163: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:259 /// calls. bool isCalled(const CallDescription &CD) const; NoQ wrote: > Szelethus wrote: > > Sz

[PATCH] D68163: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

This revision was automatically updated to reflect the committed changes. Closed by commit rGe5513336aee4: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D68163?vs=222791&id=246441#toc Repository:

[PATCH] D68163: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

Szelethus marked 5 inline comments as done. Szelethus added inline comments. Herald added subscribers: martong, steakhal. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:259 /// calls. bool isCalled(const CallDescription &CD) const;

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

Szelethus added a comment. In D69662#1890007 , @NoQ wrote: > In D69662#1889545 , @Szelethus wrote: > > > Sorry for the slack :) > > > > One should never count on the invocation order of callback funcions in > > bet

[PATCH] D73729: [analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'

Szelethus accepted this revision. Szelethus added a comment. Looks great! Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73729/new/ https://reviews.llvm.org/D73729 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

Szelethus added a comment. In D75045#1891064 , @balazske wrote: > In D75045#1891056 , @NoQ wrote: > > > In D75045#1891055 , @NoQ wrote: > > > > > How many such platforms can

[PATCH] D68162: [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary functions with parameters

This revision was automatically updated to reflect the committed changes. Closed by commit rG9fd7ce7f4449: [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary… (authored by Szelethus). Herald added subscribers: martong, steakhal. Changed prior to commit: https://review

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

Szelethus added a comment. In D71524#1889566 , @boga95 wrote: > @steakhal's revision is on the top of this. Changing the order will only > cause unnecessary work on both sides. He recently rebased on top of master. I'm no fan of creating unnecessary wor

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

Szelethus added a comment. Herald added a subscriber: martong. In D69662#1744479 , @NoQ wrote: > In D69662#1736601 , @balazske wrote: > > > Anyway the checks that do not use BindExpr (all except the open functions)

[PATCH] D74760: [Analyzer] Fix for iterator modeling and checkers: handle negative numbers correctly

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This patch is a good testament to how well those debug functions turned out. LGTM. Comment at: clang/lib/StaticAnalyzer/Checkers/Iterator.cpp:203 auto &SVB = State-

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

Szelethus added a reviewer: boga95. Szelethus added a comment. In D74131#1884372 , @steakhal wrote: > If this patch is good to go, could someone commit it? > I don't have commit access (yet). I think you can apply for a commit access, you have a history

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

Szelethus requested changes to this revision. Szelethus added a reviewer: steakhal. Szelethus added a comment. This revision now requires changes to proceed. This patch is really cool, but I still feel anxious a bit about duplicating so much functionality, especially since we're working very hard

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Herald added a subscriber: martong. Wow. Its a joy to see you do C++. LGTM. Are you planning to introduce `CallDescriptionMap` at one point? :) Comment at: clang/lib/S

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

Szelethus added a comment. FYI I've been seeing your patches to this checker and I will respond to them, but I need to do some learning on my own before having the confidence to accept or request changes. Working on it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D75045: [analyzer] Improved check of `fgetc` in StreamChecker.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. I'm a bit confused as to what this patch aims to do. Once again, I'd kindly ask you to discuss for a bit before updating this patch. --- The rule states that after reaching `E

[PATCH] D71433: [analyzer] CERT: POS34-C

Szelethus added a comment. Herald added a subscriber: martong. I think for an alpha checker this is ready to land if you're ready -- do you have commit access or need assistance? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71433/new/ https://reviews.llvm.org/D71433 ___

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:114-117 + const auto MacroIt = llvm::find_if( + PP.macros(), [&](const auto &K) { return K.first->getName() == Macro; }); + if (MacroIt == PP.macro_end()) +return llvm::None;

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

Szelethus added inline comments. Comment at: clang/test/Analysis/std-c-library-functions-eof.c:11 +// Unorthodox EOF value. +#define EOF (-2) + Maybe you could throw in a test where the definition isn't surrounded by parentheses? Repository: rG LLVM Github

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! Very nice! I think you can commit as you please, granted you add core checkers to the test `RUN:` lines. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibrar

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM, I like everything here, you worded the notes very nicely and the test cases seems to cover everything I could find! Please wait for @NoQ's approval, since he's the ranking member o

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added reviewers: xazax.hun, Charusso, dcoughlin. Szelethus added a comment. Also, allow me to add a few other folks, because they are very active and knowledgeable individuals :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new/ h

[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

Szelethus requested changes to this revision. Szelethus added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:67-69 +/// Try to parse the value of a defined preprocessor macro. We

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. One of the things that stood out for me was the lack of the usage of the `check::BranchCondition` callback, but there you'd have to grind out whether it is relevant to a return value, so I'm probably wrong on that regard. So I guess I don't have any immediate high lev

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Szelethus added a comment. Do we have a test where 2 containers are present but only one of them should be marked as interesting? void deref_end_after_pop_back(std::vector &V, std::vector &V2) { const auto i = --V.end(); const auto i2 = --V2.end(); V.pop_back(); // expected-not

[PATCH] D73629: [analyzer] vfork checker: allow execve after vfork

Szelethus added a comment. LGTM granted the test is supplied, nice catch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73629/new/ https://reviews.llvm.org/D73629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus closed this revision. Szelethus added a comment. Cheers! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73966/new/ https://reviews.llvm.org/D73966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus added a comment. I missed out on the transition to github, so I suspect that the commit access will only be extended to it after tagging rc2. I think it would be better if you committed this on my behalf, thanks! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73966/new/ ht

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. In D72705#1863499 , @balazske wrote: > If more discussion is needed it is better to use "Discourse" instead of this > review? I've been meaning to suggest that we use either that or discord for casual chatting (for simple que

[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:42-49 // The following standard C functions are currently supported: // // fgetc getline isdigit isupper // fread isalnum isgraph isxdigit //

[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation

Szelethus added a comment. Thanks! Bug report generation seems far less of a mess than is used to be :) Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:44 -/// BugReporterVisitors are used to add custom diagnostics along a path. +/// Bug

[PATCH] D71433: [analyzer] CERT: POS34-C

Szelethus accepted this revision. Szelethus added a comment. This is a very neat checker, the source code reads so easily, we might as well put it as the official CERT rule description. I think adding the non-compliant and compliant code examples would be nice. I also wrote some inline comments

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

Szelethus added a comment. Hmm, have you branched off of D71524 ? If so, this patch should definitely land first. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:165 /// Given a pointer argument, return the value it poi

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

Szelethus added a comment. A portion of my concerns are answered by this patch: D72035 . Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:103-132 struct FunctionData { FunctionData() = delete; FunctionData(cons

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. I wouldn't like to see reports emitted by a checker that resides in `apiModeling`. Could we create a new one? Some checkers, like the `IteratorChecker`, `MallocChecker` and `CS

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731 + } + return C.getNoteTag([Text, Name](BugReport &BR) -> std::string { + SmallString<256> Msg; baloghadamsoftware wrote: > NoQ wrote: > > Szelethus wro

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

Szelethus added a comment. After tests are added and `clang-format-diff.py` is ran, this would be an amazing patch, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74131/new/ https://reviews.llvm.org/D74131 ___

[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This is amazing. LGTM, granted that @NoQ is happy with the patch as well. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:33 // // This c

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. In D72705#1861750 , @balazske wrote: > Uploading new diff with added comments and renaming. Great, very much appreciated! It took a loong time, but I think I got whats going on. So, step by step, this is what we're going if a

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus updated this revision to Diff 242869. Szelethus added a comment. //Actually//, **actually** upload the correct one. Getting rusty eh. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73966/new/ https://reviews.llvm.org/D73966 Files: clang/docs/ReleaseNotes.rst Index: clang/d

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus updated this revision to Diff 242860. Szelethus marked an inline comment as done. Szelethus added a comment. //Actually// update the revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73966/new/ https://reviews.llvm.org/D73966 Files: clang/docs/ReleaseNotes.rst Index

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus marked 3 inline comments as done. Szelethus added inline comments. Comment at: clang/docs/ReleaseNotes.rst:405 +- New checker: ``fuchsia.HandleChecker`` to detect leaks related to Fuchsia + handles. xazax.hun wrote: > NoQ wrote: > > D74004 > > > > 1

[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

Szelethus added a comment. I dont think any of my comments were addressed -- could you follow up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70818/new/ https://reviews.llvm.org/D70818 ___ cfe-commit

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

Szelethus added a comment. I strongly agree with this comment: D70878#1780513 , maybe the placement of functions like `getArg` and `getNumArgs` would be most appropriate in `CallDescription`. How about we try to cut down on duplicating functionalities?

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Szelethus added subscribers: steakhal, boga95. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731 + } + return C.getNoteTag([Text, Name](BugReport &BR) -> std::string { + SmallString<256> Msg; NoQ wrote

[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

Szelethus added a comment. I think its very good that this conversation came up, and it might just happen that we'll end up removing some taint when we have a better understanding of how this works. For now, I think we can put this aside :) Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

Szelethus added a comment. Also, I really like this patch, its well documented, small and very well tested! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70818/new/ https://reviews.llvm.org/D70818 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D70818: [Analyzer] Model STL Algoirthms to improve the iterator checkers

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:644 Dependencies<[ContainerModeling]>, - Documentation, - Hidden; - -def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">, - HelpText<"Check for use of invalidate

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. In D72705#1838324 , @balazske wrote: > I am still unsure about how this checker works if the function to check is > "modeled" or evaluated by another checker. Then the function may have already > a constrained value at the Post

[PATCH] D73547: [Analyzer] Split container modeling from iterator modeling

Szelethus added a comment. I'm late to the party, but have looked at the code and I really, really-really like what you did in this patch! It solves one of my biggest complaints about this project. LGTM! Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:1389

[PATCH] D73359: [analyzer]StreamChecker refactoring (NFC).

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Herald added a subscriber: Charusso. LGTM! The code looks a lot cleaner. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73359/new/ https://

[PATCH] D73350: [analyzer] Small StreamChecker refactoring (NFC).

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Cool! Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:157 + ProgramStateRef StateNotNull, StateNull; + std::tie(StateNotNull, StateNull) = C.getConst

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Apologies for inserting myself in here :) Please use the "[analyzer]" tag instead of "[clang][checkers]" in future changes, because we've used that traditionally for years, and

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus updated this revision to Diff 242539. Szelethus marked 10 inline comments as done. Szelethus retitled this revision from "[analyzer][WIP] Add 10.0.0 release notes." to "[analyzer] Add 10.0.0 release notes.". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73966/new/ https://revie

[PATCH] D73966: [analyzer] Add 10.0.0 release notes.

Szelethus added inline comments. Comment at: clang/docs/ReleaseNotes.rst:408 + +- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage + provided for default placement new is sufficiently large. Charusso wrote: > balazske wrote: > > Szeleth

[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/docs/ReleaseNotes.rst:408 + +- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage + provided for default placement new is sufficiently large. N

[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

Szelethus created this revision. Szelethus added reviewers: hans, NoQ, Charusso, xazax.hun, baloghadamsoftware, balazske, gamesh411, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, phosek, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Szele

[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

Szelethus added a reviewer: dcoughlin. Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/docs/ReleaseNotes.rst:427 + +- ObjectiveC++ changes: + I tried my best here but didn't get far. :) Repository: rG LLVM Github

[PATCH] D73536: [analyser][taint] Remove taint from symbolic expressions if used in comparisons

Szelethus added a comment. In D73536#1845031 , @NoQ wrote: > > Describing value constraints in the taint config file is unfeasible. > > This is the only correct way to go, because, as you yourself point out, every > sink function (or other use of tainted

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D71791#1795093 , @xazax.hun wrote: > I decided to fix the unittests. Having CFGs full of dangling pointers to the > AST does not look fun at a

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

Szelethus added a comment. > I am not sure what would be the best way to test this change here. I'll admit, I'm a bit out of touch with Clang and will unfortunately be for a while, but the short answer is that the unit tests we have for the CFG are kinda bad. The constructed CFG has a longer li

[PATCH] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. For reasons other than being a part of the project, CodeChecker is objectively an amazing tool to use with the analyzer. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

Szelethus added a comment. In D69962#1744679 , @NoQ wrote: > In D69962#1742291 , @Szelethus wrote: > > > Try to add a non-sanitizer built tablegen: > > `-DCLANG_TABLEGEN=/path/to/non/sanitized/clang-tblgen` > > `-

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

Szelethus added a comment. Accepted accidentally. Well, in any case, I trust your judgement on this! I'd prefer not to commit the test file as-is. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69962/new/ https://reviews.llvm.org/D69962 ___

[PATCH] D70047: [Analyzer] Use a reference in a range-based for

Szelethus accepted this revision. Szelethus added a comment. Whoo! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70047/new/ https://reviews.llvm.org/D70047 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D69962#1741503 , @NoQ wrote: > In D69962#1739440 , @NoQ wrote: > > > In D69962#1738618

[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM, provided that the inlines are addressed! Thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:102-103 /// system call etc. - bool che

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

Szelethus added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5882 + // FIXME: Should we return the terminator here? + if (size() == 0) What would that even be? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69962/ne

[PATCH] D69962: [CFG] Fix a flaky crash in CFGBlock::getLastCondition().

Szelethus added a comment. Nice catch! Though, wouldn't the memory sanitizer buildbots break on this reliably? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69962/new/ https://reviews.llvm.org/D69962 ___ cfe-commits

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Szelethus added a comment. In D69813#1734272 , @Charusso wrote: > In D69813#1734193 , @Szelethus wrote: > > > Hmm, so this checker is rather a collection of CERT rule checkers, right? > > Shouldn't the checker name

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Szelethus added a comment. Hmm, so this checker is rather a collection of CERT rule checkers, right? Shouldn't the checker name contain the actual rule name (STR31-C)? User interfacewise, I would much prefer smaller, leaner checkers than a big one with a lot of options, which are barely support

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Szelethus added a comment. YES PLEASE. Debug checkers that only dump from `check::EndAnalysis` won't rely on the analysis not actually crashing. Which is ironically exactly when we want to use them. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp:199 +

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp:199 + /// + /// This callback should be used to construct the checker's fields. + /// Szelethus wrote: > Hmmm, what use case do you have in mind? Do we actu

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Szelethus added a comment. Changes to `MallocChecker` really highlight the positive effects of this patch. Nice! Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:451 static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE, -

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

Szelethus added a comment. In D69662#1731974 , @balazske wrote: > I wanted to remove `eval::Call` because only one checker can do this > otherwise it is undefined behavior (according to the not very new "Analyzer > Guide"). If it is essentially needed in

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

Szelethus accepted this revision. Szelethus added a comment. Let's make it 3! Thank you so much for sticking by, I guess one of the reasons why a patch so obviously great and desired took so long is that we still didn't figure out how we imagine the `CallDescriptionMap` to be integrated into lar

[PATCH] D65912: [clang-tidy] Add new check for math constants

Szelethus resigned from this revision. Szelethus added a comment. I have no authority in clang-tidy, but the idea is nice! You may wanna reupload with full context though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65912/new/ https://reviews.ll

[PATCH] D69308: [analyzer] Test cases for the unsupported features for Clang Static Analyzer

Szelethus added a comment. LGTM given that the inlines are fixed. In D69308#1722139 , @NoQ wrote: > Thanks for the tests! > > Both of these features are relatively hard. > > ... Would love to see this comment in its entirety on the open projects page :^

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

Szelethus added a comment. Its becoming a bit difficult to navigate inlines, could you please mark them as done as you address them? Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:61 + using FnCheck = bool (StreamChecker::*)(const CallEvent &, +

[PATCH] D69602: [analyzer] Test case for lambda capture by value modelling

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM, can we remove the open projects entry under the same breath? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69602/new/ https://review

[PATCH] D69318: [analyzer] Add SufficientSizeArrayIndexingChecker

Szelethus added reviewers: baloghadamsoftware, xazax.hun, rnkovacs, Charusso, dcoughlin. Szelethus added a comment. In D69318#1718220 , @gamesh411 wrote: > Please feel free to add more reviewers. Here you go! Repository: rG LLVM Github Monorepo CHA

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

Szelethus added a comment. In D66733#1706521 , @sylvestre.ledru wrote: > I added it to the release notes here : https://reviews.llvm.org/rC374593 > I am wondering if the option( WarnForDeadNestedAssignments ) to disable it > is really necessary? > I ha

[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This is amazing, thanks!! LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67156/new/ https://reviews.llvm.org/D67156 ___ cfe-c

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:288-290 + SymbolRef Sym = RetVal.getAsSymbol(); + stateNotNull = stateNotNull->set(Sym, StreamState::getOpened()); + stateNull = stateNull->set(Sym, StreamState::getOpenFailed());

[PATCH] D68162: [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary functions with parameters

Szelethus added a comment. I forgot to emphasise that the entire point of the patch was to get rid of `getAllocationFamily`, at least the way it used to work, because it was a mess and prevented me from moving forward with changing things to a `CallDescriptionMap`. In D68162#1693110

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