[PATCH] D75678: [analyzer] Skip analysis of inherited ctor as top-level function

Szelethus added a comment. I followed the discussion, on the other patch, and this seems to be the appropriate fix -- I lack the confidence to accept, but LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75678/new/ https://reviews.llvm.org/D756

[PATCH] D75697: [analyzer] Allow null false positive suppression for conditions

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

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

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

[PATCH] D75697: [analyzer] Allow null false positive suppression for conditions

Szelethus added a comment. I'm too sure what the implications of such a change is, so I'll get some real-life results before even thinking of commiting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75697/new/ https://reviews.llvm.org/D75697 __

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

Szelethus marked an inline comment as done. Szelethus added a comment. In D75698#1908335 , @NoQ wrote: > In my head this patch should ideally be reduced to a single if-statement: > "This value is a `SymbolDerived` //therefore// it was produced by > inval

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

Szelethus added a comment. Thanks for bringing it up, I'll attend to it -- though both of those changes are out of scope of this change, and I'd be strongly against solving any of those issues within this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75271/new/ https://revie

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

Szelethus abandoned this revision. Szelethus added a comment. This sounds exciting! Back to work then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75698/new/ https://reviews.llvm.org/D75698 ___ cfe-c

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

Szelethus added a comment. In D75356#1909193 , @balazske wrote: > To avoid problems I created a new version of this diff too that follows after > the other new ones: > https://reviews.llvm.org/D75682 > (Adding a new diff can make inline comment position

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Let's halt this for now -- we have so many revisions going on seemingly doing the same thing, its becoming really confusing. Please finish D75356

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Okay, very well, then lets abandon this patch. Please keep in mind that phabricator can mark whether a revision was mentioned somewhere else if you only copy the revision ID, n

[PATCH] D75614: [Analyzer][StreamChecker] Check for opened stream before operations.

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

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

Szelethus added a comment. Could you please fix up the dependencies of this revision? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75356/new/ https://reviews.llvm.org/D75356 ___ cfe-commits mailing li

[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. You seem to have uploaded the wrong diff :) In D75171#1908259 , @baloghadamsoftware wrote: > This is the so called "correct" solution.

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

Szelethus added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75356/new/ https://reviews.llvm.org/D75356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

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

Szelethus added a comment. In D75356#1909610 , @balazske wrote: > The D75682 is the one that should be used > now, If this patch is supposed to be a followup to D75682 , could you pleas

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

Szelethus added a comment. Actually, this is the diff: diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index ecd871e36ee..1f2c8c50a01 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/c

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

Szelethus marked an inline comment as done. Szelethus added a comment. In D68165#1902702 , @Charusso wrote: > I wish for a third map, something like `ReallocationMap`. Other than that it > is a great direction, I love it. Thanks! Hah, that is a neat ide

[PATCH] D75839: [Analyzer] Mark constant member functions const in CheckerManager

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM, go for it! I don't think you need to wait for any more reviews on this before commiting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added a subscriber: dcoughlin. Szelethus added a comment. In D75842#1912249 , @baloghadamsoftware wrote: > In D75842#1912246 , @xazax.hun wrote: > > > If we disable the dependency of a checker explicitly

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added a comment. The thing to note here is checkers that are not `isDisabled()` won't get enabled, only if they are `isEnabled()` as well :) Please add a unit test, otherwise LGTM. I just landed D67335 , so it should be easy to do. Repository: rG L

[PATCH] D67335: [analyzer][NFC] Refactor the checker registration unit test file

This revision was automatically updated to reflect the committed changes. Closed by commit rG58884eb64898: [analyzer][NFC] Refactor the checker registration unit test file (authored by Szelethus). Herald added subscribers: martong, steakhal. Changed prior to commit: https://reviews.llvm.org/D67

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

Szelethus accepted this revision. Szelethus added a comment. I have nothing else to add :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75529/new/ https://reviews.llvm.org/D75529 ___ cfe-commits maili

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added inline comments. Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:85 +//===--===// +// Unfulfilled dependency +//===---

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

Szelethus added a comment. Yay, this is very nice! Comment at: clang/test/Analysis/container-modeling-no-aggressive-binary-operation-simplification-warn.cpp:1-10 +// RUN: %clang_analyze_cc1 -std=c++11\ +// RUN: -analyzer-checker=core,cplusplus,alpha.cplusplus.ContainerModeling

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

Szelethus added a comment. Ah, okay I think I got why you chose this direction. Summaries are little more then a collection of constraints, and at select points in the execution we check and apply them one-by-one. If we want to preserve this architecture (and it seems great, why not?), inherita

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

Szelethus added a comment. Can we see more test cases for when after a call to `feof` we indeed can tell the stream is in an `EOF` state? Same for `ferror`. I feel like there is a lot of code we don't yet cover. Also, I see now that I didn't get in earlier revisions that `OtherError` actually

[PATCH] D75851: [Analyzer][StreamChecker] Added evaluation of fseek.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. We should totally dedicate an error kind of `fseek`, see (and please respond, if you could to) D75356#inline-689287 .

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added inline comments. Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:143 + std::string Diags; + EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags)); +} baloghadamsoftware wrote: > Szelethus wrote: > > I don't think this is

[PATCH] D75851: [Analyzer][StreamChecker] Added evaluation of fseek.

Szelethus added a comment. In D75851#1914874 , @balazske wrote: > I do not like to make difference between error from `fseek` and error from > any other function that may fail. `AnyError` can be used to handle every case. Okay, it took me a while, but I

[PATCH] D75851: [Analyzer][StreamChecker] Added evaluation of fseek.

Szelethus added a comment. In D75851#1915014 , @Szelethus wrote: > In D75851#1914874 , @balazske wrote: > > > After a failed file operation we do not know if it was feof or ferror kind > > of error. > > > Is this t

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

Szelethus added a comment. You've sold me on `AnyError`, we should have something like that with the addition of `NoteTags`. With that said, I feel uneasy about adding it in this patch and not testing it properly. I can also sympathize with your anxiety against bloating the `fseek` patch furthe

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

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Yay! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75171/new/ https://reviews.llvm.org/D75171 ___ cfe-commits mailing list cfe-com

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

Szelethus added a comment. Also, thank you guys for chipping in! Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:394 +StreamSym, StreamState::getOpenedWithOtherError()); +C.addTransition(StateEof); +C.addTransition(StateOther); b

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

Szelethus added a comment. I see that we're a bit stuck on naming, and that is largely my fault. Apologies, let us focus on high level stuff for a bit. In D75682#1916435 , @balazske wrote: > I do not understand totally this remove-of-AnyError thing. If h

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:151 + + using ValueConstraintPtr = std::shared_ptr; + /// The complete list of constraints that defines a single branch. martong wrote: > Szelethus wro

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

Szelethus added a comment. Just littering some more inlines, don't mind me :) Lets still wait on the dependency patch before updating. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:296 +def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs

[PATCH] D75063: [analyzer] StdLibraryFunctionsChecker: Add NotNull Arg Constraint

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:175 +using ValueConstraint::ValueConstraint; +bool CannotBeNull = true; + What does this do? Is it ever used in the patch? Repository: rG LLV

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

Szelethus added a comment. Could you please add the warning we discussed? That would be a great demonstration of this patch's capabilities, and wouldn't deserve its own patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75682/new/ https://review

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

Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:217 +mgr.template registerChecker(); } baloghadamsoftware wrote: > Why do we need `MGR` here as templat

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

Szelethus added a comment. So, as far as I understand, your thinking here is that `CXXOperatorCallExpr`s (which are global, non-member operators) when they are `>>`, they will propagate taintedness from the 0th argument to the 1st and the return value, correct? So, if I do this: struct Panda

[PATCH] D74615: [Analyzer] Add visitor to track iterator invalidation

Szelethus added a comment. You may have explained it in the summary, and I didn't get it, but why isn't putting a `NoteTag` in `invalidateAllIteratorPositions` sufficient? We could state something like `All iterators associated with 'V' are invalidated`. I'm not against a visitor, just playing

[PATCH] D74541: [Analyzer] Use note tags to track iterator increments and decrements

Szelethus added a comment. A few nits, otherwise the patch is great! Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:519-520 +const NoteTag *IteratorModeling::getChangeTag(CheckerContext &C, StringRef Text, + cons

[PATCH] D74541: [Analyzer] Use note tags to track iterator increments and decrements

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:519-520 +const NoteTag *IteratorModeling::getChangeTag(CheckerContext &C, StringRef Text, + const Expr *ItE, SVal It1, +

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

Szelethus added a comment. Its very interesting to see how the idea of interestingness propagation through `NoteTag`s works in practice -- I feel like many other checkers will need to adopt something like this, and its great that we have the first use case presented here. The patch from afar lo

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

Szelethus added inline comments. Comment at: clang/test/Analysis/iterator-modelling.cpp:169 clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}} // expected-note@-

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

Szelethus added a comment. The code from in `ExprInspectionChecker.cpp` is duplicated from D75677 , isn't it? Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:55 - ExplodedNode *reportBug(llvm::StringRef Msg, CheckerCo

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

Szelethus added a comment. Thanks for sticking this out! It just takes me a while to get to the mental space regarding this patch you are already in, sometimes through saying incorrect statements or stupid suggestions! :) If others could chip in in this discussion, it would be most appreciated,

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

Szelethus accepted this revision. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp:32 template void analyzerContainerDataField(const CallExpr *CE, CheckerContext &C, NoQ wrote: > `llvm::function_re

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

Szelethus marked 3 inline comments as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h:1-10 +//===-- CheckerRegistration.h - Checker Registration Function ---*- C++ -*-===// +// +// Part of the LLVM Project, under

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:139 + /// Add taint sources for extraction operator on pre-visit. + bool addOverloadedOpPre(const CallExpr *CE, CheckerContext &C) const; steakhal wrote: >

[PATCH] D64991: [analyzer][WIP] Implement a primitive reaching definitions analysis

Szelethus updated this revision to Diff 250517. Szelethus edited the summary of this revision. Szelethus added a comment. Upload final version. This revision stays here for archaeological purposes, as I'll split this up into manageable pieces. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D76287: [analysis][analyzer] Introduce the skeleton of a reaching definitions calculator

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

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

Szelethus added a comment. Please have my post-commit approval :^) Nice work! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74973/new/ https://reviews.llvm.org/D74973 ___ cfe-commits mailing list cfe-c

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

Szelethus added a comment. Rght I think I finally get it. You don't want to state split on `feof()` and `ferror()`, but rather on the stream operations! This is why you only want to tell the analyzer what the return value of these functions are going to be, because the state of the stream s

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. LGTM, aside from some checker tagging nightmare. Its a bit easy to mess up, please allow me to have a final look before commiting! :) Comment at: clang/inclu

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

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

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

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:217 +mgr.template registerChecker(); } baloghadamsoftware wrote: > NoQ wrote: > > Szelethus wrote: > >

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

Szelethus added a comment. Herald added a subscriber: ASDenysPetrov. In D75682#1926889 , @balazske wrote: > In D75682#1926732 , @Szelethus wrote: > > > Rght I think I finally get it. You don't want to state spli

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

Szelethus updated this revision to Diff 251053. Szelethus added a comment. Address inlines. 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/Fr

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

Szelethus updated this revision to Diff 251056. Szelethus added a comment. Seems like D75171 didn't add const methods after all, here they are. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75360/new/ https://reviews.llvm.org/D75360 Files: clang/inc

[PATCH] D67336: [analyzer][NFC] Introduce SuperChecker<>, a convenient alternative to Checker<> for storing subcheckers

Szelethus added a comment. Herald added subscribers: ASDenysPetrov, martong, steakhal. > I have mixed feelings. Removing boilerplate is good, but the very fact that > we're legalizing this pattern indicates that our checkers will keep bloating > up, while i always wanted to actually split them i

[PATCH] D75063: [analyzer] StdLibraryFunctionsChecker: Add NotNull Arg Constraint

Szelethus accepted this revision. Szelethus added a comment. LGTM! Please address the nits before commiting, but there is no need for another round of reviews. :) edit: from my end, that is. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:175 +

[PATCH] D76287: [analysis][analyzer] Introduce the skeleton of a reaching definitions calculator

Szelethus marked 9 inline comments as done. Szelethus added a comment. In D76287#1927340 , @xazax.hun wrote: > I think it is very crucial to make the intentions clear, how do you define > `definition` and `variable`? > Other than assignments we might inc

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

Szelethus added a subscriber: steakhal. Szelethus added a comment. In D75682#1929093 , @balazske wrote: > In D75682#1928716 , @Szelethus wrote: > > > - For streams where the precise state is unknown (they are not tr

[PATCH] D76379: [Analyzer] IteratorRangeChecker verify `std::advance()`, `std::prev()` and `std::next()`

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. The patch looks great, though I'd kindly ask you to wait a bit for someone with a bit more experience on `SVal`-smithing ;) Generally speaking, I think the word you are looking for is "v

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

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Whoo! The patch looks great and well thought out, the tests look like they cover everything and we also talked about plans for future patches. Excellent! I left a nit about merging the t

[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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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

<    10   11   12   13   14   15   16   >