[PATCH] D68172: Don't install example analyzer plugins

2019-09-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. The patch looks alright, I won't formally accept because if I knew how these worked, it wouldn't have caused so much pain to so many people :) In D68172#1686793 , @aaronpuchert wrote: > In D68172#1686772

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

2019-09-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 222514. Szelethus added a comment. I'm starting to be really happy with the current direction! I think I'll start splitting this up soon, I'm confident that the current interface (after some polishing) is general enough to develop incrementally. - Introdu

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

2019-10-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. A few nits inline, otherwise the patch is awesome, thank you!! Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:60-61 private: - void Fopen(CheckerContext &C, const CallExpr *CE) const; - void Tmpfile(CheckerContext &C, const CallExp

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

2019-10-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:127-131 + for (auto P : Call.parameters()) { +QualType T = P->getType(); +if (!T->isIntegralOrEnumerationType() && !T->isPointerType()) + return nullptr; + } --

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

2019-10-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. The logic of the patch is fine in my eyes, but I dislike the formatting. Could you please make the code obey the LLVM coding style, and after that use clang-format-diff.py? https://llvm.org/docs/CodingStandards.html https://clang.llvm.org/docs/ClangFormat.html#script-f

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

2019-10-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:109-110 bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { + if (!Call.isGlobalCFunction()) +return false; + balazske wrote: > Szele

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

2019-10-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D68162#1686810 , @NoQ wrote: > Thank you, fantastic finding! > > > in fact we know it //compile time// > > Yeah, but is it accidental or is there a good reason behind always having > this information at compile time? 'Cause i

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

2019-10-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 5 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:378-379 + + using CheckFn = void (MallocChecker::*)(CheckerContext &C, const CallExpr *CE, + Pro

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

2019-10-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 222781. Szelethus retitled this revision from "[analyzer][MallocChecker][NFC] Communicate the allocation family information to auxiliary functions with template parameters" to "[analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary f

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

2019-10-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 222791. Szelethus added a comment. Rebase, unforget to support 3-arg malloc, for which apparently were no tests at all. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68163/new/ https://reviews.llvm.org/D68163 Files: clang/include/clang/StaticA

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

2019-10-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 222794. Szelethus marked an inline comment as done. Szelethus added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68165/new/ https://reviews.llvm.org/D68165 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Index: c

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

2019-10-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1487 - if (Att->getModule()->getName() != - MemFunctionInfo.CD_malloc.getFunctionName()) + if (Att->getModule()->getName() != "malloc") return nullptr;

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

2019-10-07 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa82810c56bc9: [analyzer][MallocChecker] Improve warning messages on double-delete errors (authored by Szelethus). Herald added a subscriber: Charusso. Herald added a project: clang. Changed prior to commi

[PATCH] D68591: [analyzer] PR43551: Do not dereferce void* in UndefOrNullArgVisitor

2019-10-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, Charusso, dcoughlin, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Exactly what it s

[PATCH] D66049: [analyzer] PR41729: Fix some false positives and improve strlcat and strlcpy modeling

2019-10-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! In D66049#1701730 , @dkrupp wrote: > I also analyzed openssl with the baseline and this version, but did not find > any new warnings. > Se

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

2019-10-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I forgot to emphasise that the entire point of the patch was to get rid of `getAllocationFamily`, at least the way it used to work, because it was a mess and prevented me from moving forward with changing things to a `CallDescriptionMap`. In D68162#1693110

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

2019-10-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:288-290 + SymbolRef Sym = RetVal.getAsSymbol(); + stateNotNull = stateNotNull->set(Sym, StreamState::getOpened()); + stateNull = stateNull->set(Sym, StreamState::getOpenFailed());

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

2019-10-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This is amazing, thanks!! LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67156/new/ https://reviews.llvm.org/D67156 ___ cfe-c

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

2019-10-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D66733#1706521 , @sylvestre.ledru wrote: > I added it to the release notes here : https://reviews.llvm.org/rC374593 > I am wondering if the option( WarnForDeadNestedAssignments ) to disable it > is really necessary? > I ha

[PATCH] D69318: [analyzer] Add SufficientSizeArrayIndexingChecker

2019-10-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: baloghadamsoftware, xazax.hun, rnkovacs, Charusso, dcoughlin. Szelethus added a comment. In D69318#1718220 , @gamesh411 wrote: > Please feel free to add more reviewers. Here you go! Repository: rG LLVM Github Monorepo CHA

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

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77845: [analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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] 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

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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] 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

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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] D78101: [analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers rather then core.StackAddrEscapeBase

2020-05-19 Thread Kristóf Umann via Phabricator via cfe-commits
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] D78122: [analyzer][Nullability] Don't emit under the checker name NullabilityBase

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/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

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-05-20 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
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".

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
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".

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

2020-05-21 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked an inline comment as done. Closed by commit rG1c8f999e0b59: [analyzer][CallAndMessage] Add checker options for each bug type (authored by Szelethus). Changed prior to commit: https://reviews.llvm.org/D778

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

2020-05-22 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

2020-05-22 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

2020-05-22 Thread Kristóf Umann via Phabricator via cfe-commits
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] 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

2020-05-22 Thread Kristóf Umann via Phabricator via cfe-commits
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] 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

2020-05-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. Reverted in rG429f03089951d62fb370026905c87f1f25cf220f because its late and this is a breaking change to scan-build.

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

2020-05-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/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] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

2020-05-26 Thread Kristóf Umann via Phabricator via cfe-commits
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] D78097: [analyzer][RetainCount] Remove the CheckOSObject option

2020-05-26 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80548: [Analyzer][NFC] Remove the SubEngine interface

2020-05-26 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

2020-05-26 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

2020-05-26 Thread Kristóf Umann via Phabricator via cfe-commits
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".

2020-05-27 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77061: [analyzer] Add core.CallAndMessage to StdCLibraryFunctionArgsChecker's dependency

2020-05-27 Thread Kristóf Umann via Phabricator via cfe-commits
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".

2020-05-27 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! 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] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

2020-05-29 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

2020-05-30 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80901: [analyzer][NFC] Change checker dependency unit tests to check for the registration order

2020-05-31 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

2020-05-31 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

2020-06-01 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-06-01 Thread Kristóf Umann via Phabricator via cfe-commits
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

2020-06-02 Thread Kristóf Umann via Phabricator via cfe-commits
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] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

2020-06-02 Thread Kristóf Umann via Phabricator via cfe-commits
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] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-06-02 Thread Kristóf Umann via Phabricator via cfe-commits
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] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

2020-06-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. We can close this now, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78374/new/ https://reviews.llvm.org/D78374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

2020-07-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I don't speak snek, but I approve this message! Its also great that we have dedicated tiny/small projects to analyze locally. Comment at: clang/utils/analyzer/ProjectMap.py:31 +SMALL: 1min-10min +BIG: 10min-1h +HUGE: >1h --

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

2020-07-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I suppose LLVM could be a HUGE project? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83942/new/ https://reviews.llvm.org/D83942 ___ cfe-commits mailing list cfe-commits@list

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

2020-07-18 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I hope you don't mind if I bring up a point you mentioned during a daily meeting. > The error checking shouldn't be present on one of execution, but rather on > all of them. I think your recent comment highlights why that wouldn't be sufficient, unfortunately. Also,

[PATCH] D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions

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

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

2020-07-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I find the summary here a bit lacking. We detected this issue in D83120 , where a lot of discussion can be found, so its worth linking in. On its own, I'm not immediately sold on whether this is the correct solution, and even if it is,

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I chased my own tail for weeks before realizing that there is indeed another instance when a live **statement** is stored, other then `ObjCForCollectionStmt`... void clang_analyzer_eval(bool); void test_lambda_refcapture() { int a = 6; [&](int &a) { a =

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-21 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added a comment. In D82598#2162496 , @xazax.hun wrote: > In D82598#2162239 , @Szelethus wrote: > > > I chased my own tail for weeks before realizing that there i

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-21 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320 for (Stmt *Child : S->children()) { -if (Child) - AddLiveStmt(val.liveStmts, LV.SSetFact, Child); +if (const auto *E = dyn_cast_or

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ouch. Let me know how severe this is, because this is a big milestone in my project. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82598/new/ https://reviews.llvm.org/D82598 ___

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

2020-07-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added a comment. Yay! This checker has become a major headache as the analyzer grew. Not on a RetainCount scale, but on a MallocChecker one. Cheap shots, I know :) The patch looks great! I have some miscellaneous comments, but nothing blockin

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

2020-07-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. You could do it in the code, but if the modeling wouldn't be present from CStringModeling, CStringChecker wouldn't work properly. So you should make it a strong dependency, just as you did in this patch. My comment was mainly a response to @NoQ :)

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

2020-07-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I personally preferred the previous diff, the reporting part of the checker and the string length modeling was separated far more cleanly. In D84316#2171267 , @NoQ wrote: > Mmm, none of these benefits sound like they outweigh c

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

2020-07-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D84316#2171270 , @NoQ wrote: > Imagine something like re-using the state trait implementation between > `MallocChecker` and `StreamChecker` because they both model "resources that > can be deallocated twice or leaked" - rega

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Thank you so much, you really went out of your way to dig that out! I'll try to patch this somehow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82598/new/ https://reviews.llvm.org/D82598

[PATCH] D82122: [analyzer][Liveness][RFC][NFC] Propose to turn statement liveness into expression liveness

2020-07-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. This was accidentally committed as a part of rGb6cbe6cb0399d4671e5384dcc326af56bc6bd122 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82122/new/ htt

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Wow, I never realized I accidentally landed that assert (D82122#2172360 ), but I guess its great to have that covered. Would you prefer to have that reverted as I'm looking to fix this for good? Repository: rG LLVM Github M

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

2020-07-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D82598#2172416 , @NoQ wrote: > I still wonder what made this statement live in my example. There must have > been some non-trivial liveness analysis going on that caused a statement to > be live; probably something to do wit

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: NoQ, vsavchenko. Szelethus accepted this revision. Szelethus added a comment. Lets make sure that we invite others from the community to participate, here and on other patches as well. Otherwise, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

2020-07-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Let me double down -- just because we let some select people know about an option, I still don't think it should be user facing. I'm strongly against the philosophy that a core

[PATCH] D77391: [analyzer][AnalysisOrder] Display the CallEvent type for preCall/postCall

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

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

2020-04-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 254780. Szelethus marked 2 inline comments as done. Szelethus added a comment. Herald added a subscriber: mgorny. - Add `PostStmt` - Add PostCall for `CXXDeallocatorCall` - Add a unittest, which is kind of pointless as-is, but I realized only later that mos

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

2020-04-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested review of this revision. Szelethus added a comment. Since this change had a few blocking issues, I'm placing it back to review for greater visibility. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:67 CE_CXXAllocator, + CE

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

2020-04-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 254816. Szelethus marked 2 inline comments as done. Szelethus added a comment. Herald added a subscriber: ASDenysPetrov. Inlines addressed, rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75431/new/ https://reviews.llvm.org/D75431 Files:

[PATCH] D77411: [analyzer] StdLibraryFunctionsChecker: Add test for function with default parameter

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

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

2020-04-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:647-653 CmdLineOption ]>, Ah, okay, I see which one you refer to. We should totally make this non-user facing as well. Repository: rG LLVM Github Mo

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

2020-04-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Hmm, upon taking a look at alignment new in previous patches, I think implementing a checker for MEM57-CPP. Avoid using default operator new for over-aligned types

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

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

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

2020-04-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Btw this branch is about 7 months old, and I just now remembered why I haven't rushed publishing it. My ultimate idea (as opposed to the two detailed in the summary) was to introduce a subchecker system in MallocChecker, which also strongly ties into D67336

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

2020-04-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:59 enum CallEventKind { + CE_CXXDeallocator, CE_Function, I need to move this below `CE_Function`, as i

[PATCH] D77410: [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl

2020-04-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. LGTM! Maybe its worth glancing over the rest of the code (comments in particular) whether anything would be outdated by this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77410/ne

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