[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

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

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

Szelethus added a comment. In D77866#2068394 , @NoQ wrote: > We (me, Valeriy, Devin)'ve just been talking about this and mostly of agreed > that `core.CallAndMessage` should ideally be removed (at least as a > `Checkers.td` entity) and individual checks

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

Szelethus added a comment. In D80905#2068320 , @NoQ wrote: > +Nithin because it may be relevant to the smart pointer checker if > inter-checker communication turns out to be necessary (eg., with the move > checker). If checkers rely on one another, wea

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

Szelethus added a comment. Mind that strong dependencies also ensure the order of evaluation, but that is a //guarantee// (**either** the dependency will be evaluated before the dependency, **or** none of them will be evaluated), and this patch is about //preference// (**if** both of them are e

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

Szelethus added a comment. >> checker callback evaluation order is ensured > > And it will be the same for all callbacks, right? Like, we're not doing this > thing yet where it intentionally goes like > > chk1->checkPreCall(); > chk2->checkPreCall(); > chk2->checkPostCall(); > chk1->che

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

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

[PATCH] D80901: [analyzer][NFC] Change checker dependency unit tests to check for the registration order

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

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

Szelethus added a comment. In D77066#2062849 , @martong wrote: > - Remove SValBuilder param I failed to emphasize that the point was to remove the new `CheckerContext` parameters :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

Szelethus accepted this revision. Szelethus added a subscriber: balazske. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! I think you could remove the extra parameter, but I don't think this warrants another round of review on my end. However, the dynamic size ch

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! This patch is great! I think you're doing a great job with this project. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:670 - State = State->s

[PATCH] D77061: [analyzer] Add core.CallAndMessage to StdCLibraryFunctionArgsChecker's dependency

Szelethus added a comment. Ugh, its a shame we never added a test case for this. Do you remember actually why we needed this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77061/new/ https://reviews.llvm.org/D77061 _

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus added a comment. LGTM, I just have one question in the inlines. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:111-114 +assert((!ES.isFEof() || !IsFilePositionIndeterminate) && + "FilePositionIndeterminate should be false in FEof case.")

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGefd1a8e66eaa: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling… (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:857-868 +// FEof and other states are possible. +// The path with FEof is the one that can continue. +// For this reason a non-fatal error is generated to continue the ana

[PATCH] D80548: [Analyzer][NFC] Remove the SubEngine interface

Szelethus added a comment. - claps in excitement * Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80548/new/ https://reviews.llvm.org/D80548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D78097: [analyzer][RetainCount] Remove the CheckOSObject option

This revision was automatically updated to reflect the committed changes. Closed by commit rG6f5431846bbf: [analyzer][RetainCount] Remove the CheckOSObject option (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D78097?vs=257272&id=266166#toc Repository: rG LLVM Git

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107 + /// This value applies to all error states in ErrorState except FEOF. + /// An EOF+indeterminate state is the same as EOF state. + bool FilePositionIndeterminate = false; ---

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107 + /// This value applies to all error states in ErrorState except FEOF. + /// An EOF+indeterminate state is the same as EOF state. +

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

Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. Reverted in rG429f03089951d62fb370026905c87f1f25cf220f because its late and this is a breaking change to scan-build.

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

Szelethus added a comment. This isn't so bad as far as news go, considering you saved a lot of work for me in terms of reproducing this! I'll look into it, thanks! I admit to not use `scan-build` much. :^) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:552 + + Index = StackFrame->getIndex(); + baloghadamsoftware wrote: > Szelethus wrote: > > This mustn't be serious. `StackFrameContext::getIndex()` has **no > > comments**

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

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:195 +// Say this 3 times fast. +State = State ? State : getState(); +addTransition(State, generateSink(State, getPredecessor())); mart

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

Szelethus updated this revision to Diff 265709. Szelethus marked 7 inline comments as done. Szelethus added a comment. Fix according to reviewer comments! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77474/new/ https://reviews.llvm.org/D77474 Files: clang/include/clang/Stati

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

This revision was automatically updated to reflect the committed changes. Szelethus marked an inline comment as done. Closed by commit rG1c8f999e0b59: [analyzer][CallAndMessage] Add checker options for each bug type (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D778

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

Szelethus marked 4 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:66 + // convert these to actual checkers. + enum CheckKind { +CK_FunctionPointer, martong wrote: > So, we ar

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

This revision was automatically updated to reflect the committed changes. Closed by commit rGeeff1a970a6b: [analyzer][CallAndMessage][NFC] Split up checkPreCall (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77846/new/ https:

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

Szelethus added a comment. I think adding a better getter to `StackFrameContext` would make the patch a tad nicer, but other than that, I don't have much to add to this patch, unfortunately :) The code looks nice and we definitely need something like this. Comment at: clang/l

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus added inline comments. Comment at: clang/test/Analysis/stream-error.c:182 +} else { + fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}} +} Szelethus wrote: > Here the user checked whether F is in eof or in ferror, and

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus added a subscriber: NoQ. Szelethus added a comment. I have more questions than actual objections, but here we go! The patch looks nice overall, we just need to iron a few things out ahead of time. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107 +

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107 + /// This value applies to all error states in ErrorState except FEOF. + /// An EOF+indeterminate state is the same as EOF state. + bool FilePositionIndeterminate = false; ---

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus added a comment. rG1d393eac8f6907074138612e18d5d1da803b4ad0 should fix this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75432/new/ https://reviews.llvm.org/D75432

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus marked an inline comment as done. Szelethus added a comment. This will take a while for me to fix (couple hours, wanna wait for creduce to finish running, and I needed to compile llvm on the server as well), but I'll get it done probably today. Comment at: clang/lib

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus added a comment. Thanks, I'll get right to it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75432/new/ https://reviews.llvm.org/D75432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

Szelethus added a comment. Ping, @NoQ @vsavchenko I'm confident with the previous patch, but I'd be glad if one of you could take a look before I land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78099/new/ https://reviews.llvm.org/D78099

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194 - if (!FD) + if (!Call.getOriginExpr()) return; Szelethus wrote: > balazske wrote: > > Szelethus wrote: > >

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194 - if (!FD) + if (!Call.getOriginExpr()) return; balazske wrote: > Szelethus wrote: > > Szelethus wrote: > >

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus marked 4 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:614 + static ProgramStateRef CallocMem(CheckerContext &C, const CallEvent &Call, ProgramStateRef State

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Szelethus marked 3 inline comments as done. Szelethus added a comment. I though I addressed the inlines months ago, but seems like I did not. I'll get this done post-commit. Oops. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194 - if (!FD) + if (!Call.get

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

Szelethus updated this revision to Diff 265214. Szelethus marked 4 inline comments as done. Szelethus added a comment. Fixes according to @balazske's comments, cheers! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77846/new/ https://reviews.llvm.org/D77846 Files: clang/lib/StaticAnal

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

Szelethus added a comment. In D78122#2046450 , @NoQ wrote: > We're observing a surprising disappearance of nullability reports today - > like, they didn't just change the name, they disappeared completely. > Investigating. I landed a lot of patches in

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:27 #include "llvm/Support/Casting.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" balazske wrote: > Are these new includes

[PATCH] D78101: [analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers rather then core.StackAddrEscapeBase

This revision was automatically updated to reflect the committed changes. Closed by commit rG3a6ee4fefec0: [analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers rather… (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

This revision was automatically updated to reflect the committed changes. Closed by commit rG39dd7265: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent (authored by Szelethus). Herald added a subscriber: ASDenysPetrov. Changed prior to commit: https://reviews.llvm.org/D

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe1a3a7e8c8b: [analyzer] Change the default output type to PD_TEXT_MINIMAL in the frontend… (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D76510?vs=251673&id=265087#toc Rep

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

Szelethus added a comment. I'm committing this now as-is, if something breaks, I'll revert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76510/new/ https://reviews.llvm.org/D76510 ___ cfe-commits mail

[PATCH] D77845: [analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent

This revision was automatically updated to reflect the committed changes. Closed by commit rG3d0d2fefc0a1: analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D77845?vs=256461&id=265074#toc Reposito

[PATCH] D75431: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

This revision was automatically updated to reflect the committed changes. Closed by commit rGf2be30def37d: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D75431?vs=254816&id=265075#toc Reposi

[PATCH] D78124: [analyzer][ObjCGenerics] Don't emit diagnostics under the name core.DynamicTypePropagation

This revision was automatically updated to reflect the committed changes. Closed by commit rG66224d309d08: [analyzer][ObjCGenerics] Don't emit diagnostics under the name core. (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D78124?vs=257375&id=265067#toc Repository:

[PATCH] D78123: [analyzer][NSOrCFError] Don't emit diagnostics under the name osx.NSOrCFErrorDerefChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rGb47d1baa535a: [analyzer][NSOrCFError] Don't emit diagnostics under the name osx. (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D78123?vs=257373&id=265066#toc Repository:

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

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

Szelethus added a comment. In D78120#2034455 , @Szelethus wrote: > In D78120#1983756 , @balazske wrote: > > > LGTM > > But a better approach can be to make a new kind of dependency. (Or split > > the StreamChecker

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

Szelethus added a comment. In D78120#1983756 , @balazske wrote: > LGTM > But a better approach can be to make a new kind of dependency. (Or split the > StreamChecker.) Definitely the latter, I just didn't wanna mess with your project :) But I'd be hap

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

Szelethus added a comment. In D78374#2033826 , @balazske wrote: > I think this is a expected way of how it works, failure of a stream operation > does not necessarily depend on result of a previous operation. So any > operation can fail or not, independe

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. > Variable-length array (VLA) should have a size that fits into a size_t value. > At least if the size is queried with sizeof, but it is better (and more > simple) to check it

[PATCH] D79072: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D79072#2025120 , @martong wrote: > I'd split this patch into two as well. > > 1. [NFC] Refactoring parts > 2. The actual extra additions about

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

Szelethus added inline comments. Comment at: clang/test/Analysis/vla-overflow.c:10 +// Size of this array should be the first to overflow. +size_t s = sizeof(char[x][x][x][x]); // expected-warning{{Declared variable-length array (VLA) has too large size}} +return s;

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:435-442 + const VarDecl *VD; + if (const auto *VR = + dyn_cast(cast(Sym)->getRegion())) { +VD = cast(VR->getDecl()); + } else if (const auto *

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Yeah, this patch should definitely have unit tests. All similar patches should. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79704/new/ https://reviews.llvm.org/D7970

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. I'm sorry for the late review -- please know that this isn't the first time me taking a look, this is a complex issue. I find navigating your phabricator comments a bit difficu

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

Szelethus added a comment. In D79704#2032271 , @NoQ wrote: > Blanket reply! `ParamRegion` is not a `DeclRegion` because it does not > necessarily have a corresponding `Decl`. For instance, when calling a > function through an unknown function pointer, yo

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

Szelethus added a comment. In D79704#2029305 , @baloghadamsoftware wrote: > Thank you for quickly looking into this. > > In D79704#2029230 , @Szelethus wrote: > > > - What identifies a `MemRegion`, `TypedValueRegio

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

Szelethus added a comment. > Currently, parameters of functions without their definition present cannot be > represented as regions because it would be difficult to ensure that the same > declaration is used in every case. To overcome this, we introduce a new kind > of region called ParamRegion

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

Szelethus marked 5 inline comments as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:59 enum CallEventKind { + CE_CXXDeallocator, CE_Function, xazax.hun wrote: > Szelethus wrote: > > I nee

[PATCH] D79072: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

Szelethus added a comment. We check now whether the argument of `typedef` and `sizeof` is an incorrect VLA, but what other examples are we potentially forgetting? The warning message states that "Declared variable-length array (VLA) has negative size", but we are not actually looking for declar

[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

Szelethus marked 3 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2126-2130 + + const CheckerRegistry &Registry = ErrorNode->getState() +->getAnalysisManager() +

[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

Szelethus updated this revision to Diff 262387. Szelethus added a comment. Make the solution handle dependencies from plugins as well via `CheckerRegistry`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78126/new/ https://reviews.llvm.org/D78126 Files: clang/include/clang/StaticAnal

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:478 + + // FIXME: Check for stream in EOF state? + balazske wrote: > balazske wrote: > > Szelethus wrote: > > > balazske wrote: > > > > Szelethus wrote: > > > > > Wi

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

Szelethus added a comment. I'm not familiar enough with `DynamicSize.cpp` to judge the changes there, but aside from a few nits, this LGTM. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:249-250 + // cannot apply the constraint. Actually, ot

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:478 + + // FIXME: Check for stream in EOF state? + balazske wrote: > Szelethus wrote: > > Will that be the next patch? :D Eager to see it! > It may be too big change

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

Szelethus added a comment. Sure, this is an improvement because we display more information, but I'd argue that it isn't really a more readable warning message :) How about th argument to the call to - cannot be represented with a character - is a null pointer - ... , which violates the funct

[PATCH] D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc

Szelethus added a comment. There a couple functional lines commented out, and we need more tests, so I take that the patch is a proof of concept for now? Because its pretty cool if so. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79432/new/ h

[PATCH] D78118: [analyzer] StdLibraryFunctionsChecker: Add option to display loaded summaries

Szelethus added a comment. It would be nice to have a user-facing option that lists the functions this checker is **capable** of modeling in another patch :) Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:299 + "DisplayLoadedSummaries", +

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1041 + EvalCallAsPure) + .ArgConstra

[PATCH] D79358: [analyzer] CERT: STR37-C

Szelethus added a comment. I'm afraid so. The patch otherwise looked really clean, sorry to ruin the day! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79358/new/ https://reviews.llvm.org/D79358 __

[PATCH] D79420: [analyzer] Make NonNullParamChecker as dependency for StdCLibraryFunctionsChecker

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. I'm a bit unsure about the naming, because it's not technically true that `NonNullParamChecker` is a dependency of `StdCLibraryFunctionsChecker`. The actual relationship is different, be

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

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

[PATCH] D79358: [analyzer] CERT: STR37-C

Szelethus added a reviewer: martong. Szelethus added a comment. Herald added a subscriber: rnkovacs. Adding @martong, because I fear that this is colliding with StdLibraryFunctionsChecker. The warnings added here seem to be, in essence, identical to D73898 . Re

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

Szelethus added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75430/new/ https://reviews.llvm.org/D75430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77641: [analyzer] StdLibraryFunctionsChecker: Associate summaries to FunctionDecls

Szelethus added a comment. I think you can go ahead and commit. You seem to have a firm grasp on this project, I believe your experience with ASTImporter has more then prepared you for digging functions out of the `std` namespace, or whatever else that might come up :^) Repository: rG LLVM

[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

Szelethus added a comment. I gave a lot of thought to the high level logic, but I need some time to really internalize whats happening here. In D78374#1993858 , @balazske wrote: > Finally I had to make the decision to remove the `ErrorKindTy` enum and us

[PATCH] D78638: [analyzer] Consider array subscripts to be interesting lvalues

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Everything's clear! Nice detective work! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78638/new/ https://reviews.llvm.org/D78638 _

[PATCH] D78638: [analyzer] Consider array subscripts to be interesting lvalues

Szelethus added a comment. How come rGe20b388e2f923bfc98f63a13fea9fc19aeaec425 doesn't solve this? Or, rather, how come it even worked if this patch is needed? Is the index being a global variable the issue? The change looks

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

Szelethus added a comment. Ping^2 :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75430/new/ https://reviews.llvm.org/D75430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D78280: [Analyzer][StreamChecker] Track streams that were not found to be opened.

Szelethus added a comment. The high level idea seems great after surveying the analyzer for similar issues, but I might need to think about this a bit longer. @baloghadamsoftware, IteratorChecker needs to solve similar problems, right? Do you have any input on this? Repository: rG LLVM Gith

<    1   2   3   4   5   6   7   8   9   10   >