[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D79330#2034414 , @martong wrote: > I am not sure if I can follow your concern here. > `sizeof(size_t)` is typically 8, so that is not a bug, n

[PATCH] D78120: [analyzer][StreamChecker] Don't make StreamTestChecker depend on StreamChecker for the time being

This revision was automatically updated to reflect the committed changes. Closed by commit rG2a12acda4c9f: [analyzer][StreamChecker] Don't make StreamTestChecker depend on StreamChecker… (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

Szelethus added a comment. In D79704#2034571 , @NoQ wrote: > In D79704#2032947 , @Szelethus wrote: > > > In D79704#2032271 , @NoQ wrote: > > > > > Blanket reply! `ParamRegion

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

Szelethus added a comment. Alright, I'm up to speed I think, cheers! In D79704#2037100 , @NoQ wrote: > > The code changes make me feel like we're doing a lot of chore (and make it > > super easy to forget checking for parameters explicitly). > > I wouldn

[PATCH] D80009: [Analyzer][StreamChecker] Changed representation of stream error state - NFC.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! Amazing job! Sorry for this dragging out for so long, but I'm very happy with how this patch and the overall direction turned out. I've left an annoying nit, and as always, feel fr

[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Szelethus added a comment. I think the warning about EOF could be a separate patch, and it could be implemented for most stream operations. The patch in large looks great, but I'm just not sure why we handle fwrite so differently. Could you please explain? Comment at: clang/l

[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Szelethus added a comment. In D80015#2041653 , @balazske wrote: > The difference of fread and fwrite comes from the fact that `fwrite` can > always succeed, `fread` does not succeed in EOF state. > The plan is to add the file functions sequentially. From

[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Szelethus added a comment. But just to assure you, this patch is practically perfect. I don't think we're going to have any hiccups moving forward with this. Again, thank you for all the patience! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8001

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

This revision was automatically updated to reflect the committed changes. Szelethus marked 14 inline comments as done. Closed by commit rG9d69072fb807: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker (authored by Szelethus). Changed prior to commit: https://reviews.llvm

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

Szelethus added a comment. In D75430#2028456 , @NoQ wrote: > Sorry i'm not very responsive these days >.< No worries, thanks! ^-^ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1052 +/// This is a call

[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This patch is great. LGTM! In D80015#2043263 , @balazske wrote: > If the unit of the change is adding `fread` and `fwrite` completely, the > war

[PATCH] D79415: [analyzer][MallocChecker] When modeling realloc-like functions, don't early return if the argument is symbolic

This revision was automatically updated to reflect the committed changes. Closed by commit rG2e5e42d4aeab: [analyzer][MallocChecker] When modeling realloc-like functions, don't early… (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

Szelethus added a comment. Gentle ping :^) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77474/new/ https://reviews.llvm.org/D77474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D78121: [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions into a checker option

This revision was automatically updated to reflect the committed changes. Closed by commit rG500479dba33a: [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D78121?vs=257369&id=264889#toc

[PATCH] D78121: [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions into a checker option

Szelethus added a comment. Taking a look as I'm speaking! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78121/new/ https://reviews.llvm.org/D78121 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D78121: [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions into a checker option

Szelethus added a comment. rG268fa40daa151d3b4bff1df12b62e5dae94685d7 should fix it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78121/new/ https://reviews.llvm.org/D78121

[PATCH] D78122: [analyzer][Nullability] Don't emit under the checker name NullabilityBase

This revision was automatically updated to reflect the committed changes. Closed by commit rGe4e1080a5837: [analyzer][Nullability] Don't emit under the checker name NullabilityBase (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D78122?vs=257372&id=264921#toc Reposit

[PATCH] D128064: [Static Analyzer] Small array binding policy

Szelethus added a comment. Herald added a subscriber: steakhal. No need for post commit fixes, just general observations since I noticed them. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2427 + uint64_t ArrSize = CAT->getSize().getLimitedValue(); + if (ArrSize >

[PATCH] D127643: [Static Analyzer] Structured bindings to data members

Szelethus added a comment. I read https://en.cppreference.com/w/cpp/language/structured_binding carefully, and there are a number of interesting rules that might deserve their own test case, even if this isn't the patch where you solve that issue, or believe that the solution handles it without

[PATCH] D122244: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! You did check whether a missing doc field will actually trigger this error, right? Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:82 +PrintFatal

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

Szelethus abandoned this revision. Szelethus added a comment. Very well :) Let's abandon this in its current state, I share this sentiment: In D120992#3368118 , @NoQ wrote: > What I'm trying to say here is, I honestly think re-doing it as CFG-based > sh

[PATCH] D116597: [analyzer] Don't track function calls as control dependencies

Szelethus updated this revision to Diff 419132. Szelethus marked an inline comment as done. Szelethus added a comment. Herald added a project: All. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116597/new/ https://reviews.llvm.org/D116597 Files:

[PATCH] D116597: [analyzer] Don't track function calls as control dependencies

Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5908-5909 +void CFG::dump(bool ShowColors) const { dump(LangOptions{}, ShowColors); } + /// print - A simple pretty printer of a CFG that outputs to an ostream.

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

Szelethus updated this revision to Diff 358876. Szelethus marked 3 inline comments as done. Szelethus added a comment. - Change parameter names to highlight that the node at the end of the function call is a `CallExitBegin`. - Change individual parameter callbacks to the entire call -- if a clien

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

Szelethus added a comment. In D105553#2867881 , @NoQ wrote: > In the mailing list I seem to have made a mistake about how this works: we > don't explicitly scan the AST for potential statements that could cause a > state change (eg., assignment operator

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

Szelethus updated this revision to Diff 358883. Szelethus added a comment. - Fixes according to reviewer comments! - Rebase on D105553 . - Primitively check whether the allocated memory was actually passed into the function. CHANGES SINCE LAST ACTION https://

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

Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:801 +// must be a superset of, but not necessarily equal to ExitOwners. +return !llvm::set_is_subset(ExitOwners, CurrOwners); + }

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

Szelethus updated this revision to Diff 358927. Szelethus marked an inline comment as done. Szelethus added a comment. - Fix testfiles. - Fix a typo in the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105819/new/ https://reviews.llvm.org/D105819 Files: clang/lib/StaticAnal

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:801 +// must be a superset of, but not necessarily equal to ExitOwners. +return !llvm::set_is_subset(ExitOwners, CurrOwners); + } Szelethus wrote: > NoQ wrot

[PATCH] D104925: [Analyzer] Improve report of file read at end-of-file condition.

Szelethus added a comment. The bug reports speak for themselves, they are awesome. Nice work! Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:28-30 +//===--===// +// Definition of state data str

[PATCH] D105003: [Analyzer] Improve report of "indeterminate file position" condition (alpha.unix.Stream).

Szelethus added a comment. I suppose the counter is outdated, similarly to D104925#2850420 ? Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:101-109 /// Indicate if the file has an "indeterminate file position indica

[PATCH] D104925: [Analyzer] Improve report of file read at end-of-file condition.

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/D104925/new/ https://reviews.llvm.org/D104925

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

Szelethus added a comment. An ever so gentle ping :^) 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.llvm.org/cgi-bin/mailman/li

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:380-391 +if (BT == &BT_UseAfterClose) + Message = "Stream closed here"; +else if (BT == &BT_UseAfterOpenFailed) + Message = "Assuming opening the stream fails here";

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

Szelethus added inline comments. Herald added a subscriber: manas. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:156-157 // the bug is reported. -virtual std::string describe(ProgramStateRef State, +virtual std::string describe(Descrit

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:236-242 + const BugMessageMap BugMessages = { + {&BT_FileNull, "Assuming opening the stream fails here"}, + {&BT_UseAfterClose, "Stream closed here"}, + {&BT_UseAfterO

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

Szelethus added a comment. Ping 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.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. I like the generalization still, but I don't agree with how you retrieve the `NoteTag` message. Its the wrong way around. This is how you invoke your function: void StreamCh

[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

Szelethus added a reviewer: vsavchenko. Szelethus added a comment. I like the idea, though I wonder whether `evalAssume` would be a better callback for this. That way, you'd only need to add an assumption when you reach a condition where one of the operands is standard. Though it may be more tr

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

Szelethus added reviewers: steakhal, NoQ, martong, vsavchenko. Szelethus added a comment. Herald added a subscriber: rnkovacs. One of the test files needs fixing: https://reviews.llvm.org/harbormaster/unit/view/931615/ Love the patch, though I agree that the note message needs some improvement.

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

Szelethus added a comment. Thanks! Here are some results: All runs can be found here: https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs Protobuf, Bitcoin, Xerces, TinyXML, PostgreSQL, FFMPEG, OpenSSL, Vim, Redis, Twin, curl: ---

[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Nice! Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:27 -static std::string getPackageFullName(const Record *R); +static std::string getPackageFullName(c

[PATCH] D121197: [clang][dataflow] Add analysis that detects unsafe accesses to optionals

Szelethus added a comment. Seems like all new files are missing the header blurb about the licence. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121197/new/ https://reviews.llvm.org/D121197 ___ cfe-comm

[PATCH] D122285: [analyzer] Add path note tags to standard library function summaries.

Szelethus added a comment. LGTM on my end, this is awesome! In D122285#3401754 , @steakhal wrote: >> The notes are prunable, i.e. they won't bring-in entire stack frames worth >> of notes just because they're there, but they will be always visible >> r

[PATCH] D116597: [analyzer] Don't track function calls as control dependencies

This revision was automatically updated to reflect the committed changes. Szelethus marked an inline comment as done. Closed by commit rGfd8e5762f86f: [analyzer] Don't track function calls as control dependencies (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D116597

[PATCH] D118880: [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

Szelethus created this revision. Szelethus added reviewers: NoQ, steakhal, martong, balazske, ASDenysPetrov. Szelethus added a project: clang. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, yaxunl. Szele

[PATCH] D119004: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

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

[PATCH] D118880: [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

Szelethus updated this revision to Diff 405979. Szelethus edited the summary of this revision. Szelethus added a comment. Move `CallDescription` specific changes to D119004 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118880/new/ https://reviews.llv

[PATCH] D119004: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

Szelethus added a comment. In D119004#3297025 , @steakhal wrote: > I strongly belive that this should be an overload to the existing 'matches' > API. Maybe add a comment that prefer the other overload if can. But having an > overload for that alread imp

[PATCH] D119004: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

Szelethus added a comment. Now that I remember, the ever so slightly different overloads of `ProgramState::getSVal` is a prime example I think. I always percieved that I have the means to invoke several of them at any point, but I never really knew which one. Though, to be fair, they were not d

[PATCH] D119004: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

Szelethus updated this revision to Diff 407106. Szelethus added a comment. - Rename from `.*Imprecise` to `.*AsWritten`. - Copy comments to relevant functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119004/new/ https://reviews.llvm.org/D119004 Files: clang/include/clang/Static

[PATCH] D118880: [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

Szelethus updated this revision to Diff 407121. Szelethus marked 8 inline comments as done. Szelethus added a comment. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118880/new/ https://reviews.llvm.org/D118880 Files: clang/lib/StaticAnalyzer/Chec

[PATCH] D118880: [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

Szelethus added inline comments. Comment at: clang/test/Analysis/NewDeleteLeaks.cpp:152 + // like to deallocate anything. + bar(); +} steakhal wrote: > I guess we would not have the warning if `bar` would accept the pointer `P`, > since that way it would escap

[PATCH] D130055: Clang extensions yolo, woot & kaboom

Szelethus added a subscriber: balazske. Szelethus added a comment. Yeah, I'm afraid no fun is allowed on this block. On another note, `kaboom` is interesting, shouldn't we assume all functions to be `kaboom` unless proven to be `woot`? @balazske's work on checking the return value of certain fu

[PATCH] D66473: [analyzer] Removed some dead code in BugReporter and related files

Szelethus added a comment. No way the entire `NodeResolver` is dead code! I spent so much time trying to understand it! I mean, now that I think about it, we literally deep copy every `ExplodedNode`, so why would we need the mapping to the original, right? Wow. Thank you so much for clearing th

[PATCH] D66473: [analyzer] Removed some dead code in BugReporter and related files

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2343 InterExplodedGraphMap ForwardMap; - TrimmedGraph = OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap); gribozavr wrote: > NoQ wrote: > > Btw these days we stro

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

Szelethus updated this revision to Diff 216273. Szelethus added a comment. - Change the condition postfix as discussed - `expected-note{{...}}` -> `expected-note-re^}}...{{$` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65575/new/ https://reviews.llvm.org/D65575 Files: clang

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

Szelethus added a comment. Hmm-hmm, I kinda like the comma, but would happily concede on this. Its been a while since my english exam :^) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65575/new/ https://reviews.llvm.org/D65575 ___ cfe-comm

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

Szelethus added a comment. In D65575#1638430 , @xazax.hun wrote: > LGTM! I think the UIs could do better displaying this info in the future but > this is not your job :) https://github.com/Ericsson/codechecker/pull/2279 CodeChecker now indents function

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

Szelethus updated this revision to Diff 216283. Szelethus added a comment. - Add the `FIXME` - Cascade test changes from D65575 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65724/new/ https://reviews.llvm.org/D65724 Files: clang/include/clang/Static

[PATCH] D66381: [analyzer] Enable control dependency condition tracking by default

Szelethus updated this revision to Diff 216418. Szelethus added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66381/new/ https://reviews.llvm.org/D66381 Files: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def clang/test/Analysis/analyzer-config.c cla

[PATCH] D65725: [analyzer] Mention whether an event is about a condition in a bug report part 2

Szelethus updated this revision to Diff 216421. Szelethus added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65725/new/ https://reviews.llvm.org/D65725 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/test/Analysis/track-control-dependency-cond

[PATCH] D65723: [analyzer][NFC] Add different interestingness kinds

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:92-101 /// Default tracking kind -- specifies that as much information should be /// gathered about the tracke

[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

This revision was automatically updated to reflect the committed changes. Closed by commit rL369574: [analyzer] Mention whether an event is about a condition in a bug report part 1 (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Chang

[PATCH] D66381: [analyzer] Enable control dependency condition tracking by default

Szelethus added a comment. In D66381#1639679 , @NoQ wrote: > The code looks good and there seems to be a lot of test coverage :] Thank you! It would not have been possible on my own! And actually, there are a couple minor nits, such as caching the retri

[PATCH] D65723: [analyzer][NFC] Add different interestingness kinds

This revision was automatically updated to reflect the committed changes. Closed by commit rL369583: [analyzer][NFC] Add different interestingness kinds (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

This revision was automatically updated to reflect the committed changes. Closed by commit rL369589: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is… (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Chang

[PATCH] D65725: [analyzer] Mention whether an event is about a condition in a bug report part 2

This revision was automatically updated to reflect the committed changes. Closed by commit rL369596: [analyzer] Mention whether an event is about a condition in a bug report part 2 (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Chang

[PATCH] D66131: [analyzer] Don't track the condition of foreach loops

This revision was automatically updated to reflect the committed changes. Closed by commit rL369613: [analyzer] Don't track the condition of foreach loops (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: http

[PATCH] D66572: [analyzer] BugReporter Separation Ep.I.

Szelethus added a comment. Super high level question: `CheckerManager` knows whether a checker, and I suspect a checker callback is path sensitive or not, do you think we can automate this decision (whether the bug report is path sensitive of syntactic)? For the purposes of clang-tidy, can we s

[PATCH] D66381: [analyzer] Enable control dependency condition tracking by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG0f9e530c0f4d: [analyzer] Enable control dependency condition tracking by default (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66381

[PATCH] D66572: [analyzer] BugReporter Separation Ep.I.

Szelethus added a comment. You got me convinced. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:223 + using visitor_iterator = VisitorList::iterator; + using visitor_range = llvm::iterator_range; + I think I added a visitor ran

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

Szelethus accepted this revision. Szelethus added a comment. @whisperity, any objections to this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66333/new/ https://reviews.llvm.org/D66333 ___ cfe-commits mailing list

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

Szelethus added a comment. What I meant is that the diff has to be made against the master branch or I won't be able to apply it locally. Say your repo is structured like this: * dcba2 (HEAD -> my_fix) * dcba1 * abcd4 (master) * abcd3 * abcd2 * abcd1 Then the diff has to be made wi

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

This revision was automatically updated to reflect the committed changes. Closed by commit rL369760: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior t

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

Szelethus added a comment. I seem to have been able to put this together by fetching the individual diffs and squashing them this time :) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014 _

[PATCH] D65065: [clang-tidy] Possibility of displaying duplicate warnings

This revision was automatically updated to reflect the committed changes. Closed by commit rL369763: [clang-tidy] Possibility of displaying duplicate warnings (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66714: [analyzer] Don't run the analyzer for -analyzer-list-enabled-checkers

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

[PATCH] D66715: [CFG] Add dumps for CFGElement and CFGElementRef

Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, dcoughlin, rnkovacs, Charusso, baloghadamsoftware. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp. Seems like we never had these, so here we go! I also did some refactoring as I

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

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

[PATCH] D66572: [analyzer] BugReporter Separation Ep.I.

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:75 /// individual bug reports. class BugReport : public llvm::ilist_node { public: Shouldn't we make this an abstract class? CHANGES SINCE LAST ACT

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

Szelethus added a comment. No worries, always happy to help! :) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D66715: [CFG] Add dumps for CFGElement and CFGElementRef

Szelethus added a comment. Mind that I'll probably commit without the unit test, last time I learned the hard way that the AST's lifetime ends by the time we retrieve the CFG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66715/new/ https://review

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Szelethus added a comment. In D66721#1644514 , @NoQ wrote: > No-no, you're disabling the checkers here but you should be silencing them. > This will be crashing because modeling is disabled. > > I also recommend checker options, even if they apply to mult

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

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. I really-really like this change. The results look great, though I'm not sure what happened here

[PATCH] D66565: [analyzer] pr43036: Fix support for operator `sizeof...'.

Szelethus accepted this revision. Szelethus added a comment. (++LGTM)++ Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66565/new/ https://reviews.llvm.org/D66565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

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

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

Szelethus added a comment. Please note that LLVM 9.0.0-final is due on the 28th of August. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66765/new/ https://reviews.llvm.org/D66765 ___ cfe-commits maili

[PATCH] D66404: [CFG] Make destructor calls more accurate

Szelethus added a comment. In D66404#1645851 , @comex wrote: > Heh, I guess I'll request commit access, although I'm not sure if I have > enough of a 'track record of submitting high quality patches'. You really do :) Repository: rC Clang CHANGES S

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

Szelethus added a comment. Note how I am partially at fault here, which is why the bug reports state that the crash originates from `TrackControlDependencyVisitor`, but even after that is fixed, the diagnostics construction flopped on an out-of-bounds error. Repository: rG LLVM Github Monore

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

Szelethus added a comment. In D66716#1647668 , @NoQ wrote: > I don't understand. Isn't widening supposed to happen //after we exit the > loop//? The loop isn't over yet when the bug is being reported. Why is this > problem widening-specific? Given that w

[PATCH] D66847: [analyzer] Fix analyzer warnings.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. > if the dyn_ part is really necessary here, then you crash with a null > dereference a few lines below because you didn't check the result Hmm, can we detect things like this?: if (i

[PATCH] D66847: [analyzer] Fix analyzer warnings.

Szelethus added a comment. Another common mistake is this: if (A) { const auto *B = dyn_cast_or_null(A); // warn: A is known to be non-null, prefer dyn_cast } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66847/new/ https://reviews.llvm.org/D66847 __

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

Szelethus updated this revision to Diff 217564. Szelethus added a comment. Fixing inlines! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66765/new/ https://reviews.llvm.org/D66765 Files: clang/docs/ClangStaticAnalyzer.rst clang/docs/ReleaseNotes.rst Index: clang/docs/ReleaseNotes

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

Szelethus marked 10 inline comments as done. Szelethus added a comment. In D66765#1646237 , @NoQ wrote: > I approve the patch and i don't see anything obvious that we're missing out > (@Szelethus, your GSoC isn't on by default back in 9.0, only in master,

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

Szelethus added a comment. In D66733#1647964 , @steakhal wrote: > @Szelethus The mispositioned report message was my fault. I used a different > version of clang for the analysis and to upload the results, which resulted > in some mispositioned reports.

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

Szelethus accepted this revision. Szelethus added a comment. Excellent detective work! Thanks! I'll do the honors. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66733/new/ https://reviews.llvm.org/D66733 ___ cfe-commits mailing list cfe-com

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

Szelethus added a comment. Sorry for not accepting this -- I actually read the code multiple times and didn't see anything that stood out! I didn't have the time to dig too deep, but if the tests are anything to go by, its gotta be alright. Repository: rL LLVM CHANGES SINCE LAST ACTION ht

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Szelethus added a comment. In D66042#1655119 , @sylvestre.ledru wrote: > @Charusso This probably should be added to the release notes: > https://clang.llvm.org/docs/ReleaseNotes.html#static-analyzer > and detailed in the doc. > Please let me know if yo

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:108 + + StringRef getDescription() const { return Description; } + gribozavr wrote: > What's the difference between description and short description?

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