[PATCH] D69318: [analyzer] Add SufficientSizeArrayIndexingChecker

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SufficientSizeArrayIndexingChecker.cpp:27 +: public Checker> { + mutable std::unique_ptr BT; + gamesh411 wrote: > balazske wrote: > > The bug type can be initialized here: > > `BT

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

balazske added a comment. We must check on every execution path that a specific condition on a value is satisfied (or: find one where the condition is not satisfied). This would be the most simple form of this checker. This can be done with the path sensitive check and `checkDeadSymbols`. If th

[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/stream.c:274-284 // Check that "location uniqueing" works. // This results in reporting only one occurence of resource leak for a stream. void check_leak_noreturn_2() {

[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/stream.c:274-284 // Check that "location uniqueing" works. // This results in reporting only one occurence of resource leak for a stream. void check_leak_noreturn_2() {

[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/stream.c:274-284 // Check that "location uniqueing" works. // This results in reporting only one occurence of resource leak for a stream. void check_leak_noreturn_2() {

[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/stream.c:274-284 // Check that "location uniqueing" works. // This results in reporting only one occurence of resource leak for a stream. void check_leak_noreturn_2() {

[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

balazske marked an inline comment as done. balazske added a comment. The problems with bug reporting should be fixed in another patch. Comment at: clang/test/Analysis/stream.c:274-284 // Check that "location uniqueing" works. // This results in reporting only one occurence of

[PATCH] D83120: [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak report.

balazske updated this revision to Diff 278110. balazske added a comment. Fixed commit message and added FIXME about bug report location problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83120/new/ https://reviews.llvm.org/D83120 Files: clan

[PATCH] D83115: [Analyzer] Report every bug if only uniqueing location differs.

This revision was automatically updated to reflect the committed changes. Closed by commit rG22a084cfa337: [Analyzer] Report every bug if only uniqueing location differs. (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83115/new

[PATCH] D83120: [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak report.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/stream.c:274-284 // Check that "location uniqueing" works. // This results in reporting only one occurence of resource leak for a stream. void check_leak_noreturn_2() {

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske added a comment. I do not understand fully this "globally". A new option should be added that affects all checkers that detect some kind of resource leak? And then implement that kind of report uniqueness in all checkers that detect resource leak. Other possible solution: Leave the cur

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

balazske added a comment. So for this patch it would be OK to have the uniqueing location as it is now. A next large change can be to add the global resource leak report uniqueing feature, this changes anyway more existing checkers (including this one). (Still I want to finish other improvement

[PATCH] D81061: [Analyzer][VLASizeChecker] Fix problem with zero index assumption.

balazske updated this revision to Diff 272361. balazske added a comment. - Rebase - Using condition type for `evalBinOp` - Comments updated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81061/new/ https://reviews.llvm.org/D81061 Files: clang/lib

[PATCH] D81407: [Analyzer][StreamChecker] Add note tags for file opening.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe935a540ea29: [Analyzer][StreamChecker] Add note tags for file opening. (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D81407?vs=271291&id=272355#toc Repository: rG LLVM Gi

[PATCH] D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition.

balazske created this revision. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp. Herald added a project: clang. balazske added reviewers: gamesh411, martong. Herald added a subscriber: rnkovacs. Parent map of ASTContext is built once. If this happens and later the TU i

[PATCH] D82561: [analyzer][CrossTU] Lower CTUImportThreshold default value

balazske added a comment. In D82561#2115578 , @xazax.hun wrote: > The analyzer inlines small functions within a TU regardless of the > thresholds. I think it would be sensible to do the same across TUs in the > case we don't do this already. That means

[PATCH] D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition.

balazske added a comment. I did not found problems related to traversal scope. At AST import `Decl`'s are added only, but the existing ones should remain valid. So the top-level decls in TraversalScope should remain valid after import. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D82741: [Analyzer][StreamChecker] Use BugType instead of BuiltinBug (NFC) .

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. balazske retitle

[PATCH] D82741: [Analyzer][StreamChecker] Use BugType instead of BuiltinBug (NFC) .

This revision was automatically updated to reflect the committed changes. Closed by commit rGd1df56023132: [Analyzer][StreamChecker] Use BugType instead of BuiltinBug (NFC) . (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82741

[PATCH] D82845: [Analyzer][StreamChecker] Report every leak, clean up state.

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. balazske added a

[PATCH] D82845: [Analyzer][StreamChecker] Report every leak, clean up state.

balazske added a comment. Why does this not work? I get only warning for the first resource leak (in the test `f_leak_2`). How to fix this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82845/new/ https://reviews.llvm.org/D82845 ___

[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

balazske added a comment. It looks like that the dependency manipulation and computation functions, and the checker and option add functions can be member of `CheckerRegistryData`. It would be a bit more simple (no `Data.` call), and these belong naturally to there. Comment

[PATCH] D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:723 + // Parent map is invalidated after changing the AST. + ToDecl->getASTContext().getParentMapContext().clear(); + balazske wrot

[PATCH] D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:723 + // Parent map is invalidated after changing the AST. + ToDecl->getASTContext().getParentMapContext().clear(); + martong wrote

[PATCH] D82845: [Analyzer][StreamChecker] Report every leak, clean up state.

balazske added a comment. I checked it with simple debug print, the `LeakedSyms` will contain 2 items with different `StreamOpenNode`. So I think these should be independent problem reports for the bug reporter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D82845: [Analyzer][StreamChecker] Report every leak, clean up state.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:969 + // Do not warn for non-closed stream at program exit. + if (Pred && Pred->getCFGBlock() && Pred->getCFGBlock()->hasNoReturnElement())

[PATCH] D82568: [clang][CrossTU] Invalidate parent map after get cross TU definition.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf3b344661048: [clang][CrossTU] Invalidate parent map after get cross TU definition. (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D825

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 258011. balazske added a comment. - Adding tests. - Type alias is supported. - Updated comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files: clang/lib/Ana

[PATCH] D78189: [analyzer] StdLibraryFunctionsChecker: Add statistics

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:67 +#define DEBUG_TYPE "StdLibraryFunctionsChecker" +STATISTIC(NumCall, "The # of calls handled by the checker"); +STATISTIC(NumFoundSummary, "The # of calls with associat

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

balazske accepted this revision. balazske added a comment. This revision is now accepted and ready to land. I am not familiar with ObjC but the code change looks OK if the tests pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78123/new/ https:/

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

balazske 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())); ``` i

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) aaron.ballman wrote: > Can you also add tests for: > ``` > int vla_unevaluated(int x) { > // Eval

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) balazske wrote: > aaron.ballman wrote: > > Can you also add tests for: > > ``` > > int vla_unevalua

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

balazske added a comment. I am not sure if this is the best approach: In a similar case (no opening function encountered) it is possible that the stream is already "escaped", there could be references to it from outside the analyzed function (to which it was passed, or got from another non-anal

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

balazske added a comment. The new option will simply list the (found) functions that have a summary based check enabled. (The term "Loaded" may be misleading in the current implementation, somebody can think if it is loaded from file?) A more detailed output would be better (display the signatu

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

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. Functions fread

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

balazske marked 2 inline comments as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:222 {StreamState::FEof, StreamState::FError, StreamState::NoError}}}, - {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0,

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

balazske added a comment. Finally I had to make the decision to remove the `ErrorKindTy` enum and use boolean flags instead for every possible error (including no error). This is needed because we do not know always what error is possible if it is "unknown". It could be determined from the last

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

balazske updated this revision to Diff 259491. balazske added a comment. - Stream error is stored in separate boolean flags instead of enum. - Removed `PossibleErrors`. - Added "FilePositionIndeterminate". - Updated fread, fwrite, fseek, clearerr. - Added tests. Repository: rG LLVM Github Mono

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 259513. balazske marked 5 inline comments as done. balazske added a comment. Updated tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files: clang/lib/Analysis

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) aaron.ballman wrote: > NoQ wrote: > > balazske wrote: > > > balazske wrote: > > > > aaron.ballman wrote: > > > > > Can you also add tests for:

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added a comment. In D77809#1999050 , @xazax.hun wrote: > Actually, sorry. I just realized that the alignof problem is introduced by > this patch. I'd love to see the solution committed together with this patch > (it could be a separate patch but

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 259584. balazske added a comment. - Separating test files, fixing alignof problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files: clang/lib/Analysis/CFG.cpp

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 259820. balazske marked an inline comment as done. balazske added a comment. - Separating test files, fixing alignof problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VA = FindVA(VA->getElementType().getTypePtr())) { + if (CFGBlock *newBlock = addStmt(VA->getSizeExpr())) +LastBlock = newBlock; aaron.ballman wrote: > newBlock -> N

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3b9b3d56efaa: [Analyzer] Include typedef statements in CFG build. (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D77809?vs=259820&id=260269#toc Repository: rG LLVM Github M

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

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. The check of VL

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

balazske marked 3 inline comments as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:207-210 + {{"fread", 4}, + {&StreamChecker::preFread, +std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, +

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

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:651 +// Get the declaration of a function proto as written in the source file. +StringRef ToString(const FunctionDecl *FD) { + const auto &SM = ACtx.getSource

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

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. Variable-length

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

balazske added a comment. Some results with an improved version of this checker: tmux: 6 results duckdb: 23 results (1-2 false positive) vim: 1 result emacs: 25 results (more false positives mostly because ? operator that is easy to fix) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D84979: [analyzer][NFC] Refine CStringLength modeling API

balazske added a comment. Really I am still not totally familiar how the checkers work and if it is good to have these function names. I was thinking about any checker that needs a string length information. It could get the data from CStringChecker using a "get" function or test if there is da

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

balazske added a comment. Herald added a subscriber: whisperity. Test results for tmux are available here

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

balazske updated this revision to Diff 283856. balazske added a comment. Herald added a subscriber: whisperity. NFC changes, added some tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84520/new/ https://reviews.llvm.org/D84520 Files: clang/

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

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:55-60 + /// Test if an encountered binary operator where the return value is involved + /// is a valid check statement. The return val

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

balazske updated this revision to Diff 284283. balazske added a comment. Preserve original error messages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84520/new/ https://reviews.llvm.org/D84520 Files: clang/lib/StaticAnalyzer/Checkers/Derefere

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

balazske marked an inline comment as done. balazske added a comment. Do the null pointer and invalid pointer dereference belong to the same checker, that is called //NullDereference//? Comment at: clang/test/Analysis/misc-ps-region-store.m:1160 struct list_pr8141 *items; -

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

balazske updated this revision to Diff 284375. balazske marked an inline comment as done. balazske added a comment. - Renames. - Changed behavior if function call is compared to returned value. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new

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

balazske marked 3 inline comments as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:63 + const BinaryOperator *BinOp, + const llvm::APSInt

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

balazske updated this revision to Diff 284588. balazske marked an inline comment as done. balazske added a comment. Rebase (bug category is LogicError). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84520/new/ https://reviews.llvm.org/D84520 Files

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

This revision was automatically updated to reflect the committed changes. Closed by commit rG497d060d0a74: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker. (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

balazske updated this revision to Diff 284741. balazske marked 2 inline comments as done. balazske added a comment. - Rebase - Changed `testBinOpForCheckStatement` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new/ https://reviews.llvm.org/D7

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

balazske added a comment. More results in CodeChecker: emacs_errorreturn

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

balazske updated this revision to Diff 285057. balazske added a comment. Improved comments, code clean-up. Improved detection of explicit cast to void. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new/ https://reviews.llvm.org/D72705 Files:

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2040 + +if (ConstStructTimevalPtrTy && StructTimespecPtrTy) + // int nanosleep(const struct timespec *rqtp, struct timespec *rmtp); Should be `C

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

balazske added a comment. In D72705#2213300 , @whisperity wrote: > In D72705#2210255 , @balazske wrote: > >> More results in CodeChecker: emacs_errorreturn >>

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

balazske accepted this revision. balazske added a comment. This revision is now accepted and ready to land. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84248/new/ https://reviews.llvm.org/D84248

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

balazske added a comment. There is a TODO comment at the error message, it needs improvement. The current form is still something for developers, not for end users. For final version I would accept textual descriptions (as mentioned above), not names like "BufferSize" and words like "ArgN" insi

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

balazske added a comment. Even some macro-like construct can improve readability. The current way of adding functions is source of copy-paste errors because more things are repeated, these should be written only once. And the `if`s for the existence of types can be eliminated (automated) someho

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

balazske added a comment. It is better now, but the refactoring change should be in a separate revision (it affects the other already added functions too). Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:350 + } else { +*this = Signatu

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

balazske added a comment. Question: Is there a way to check for overflow in the symbolic domain (that is the `ArrSize` value)? Or get the maximal possible value of a `SVal` if it is constrained? (The `ArrSize` that is a multiplication of every dimension size can not be checked simply for too bi

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

balazske updated this revision to Diff 262058. balazske marked 2 inline comments as done. balazske added a comment. Small review related fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78374/new/ https://reviews.llvm.org/D78374 Files: clang

[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

balazske added a comment. The empty signature is used to make add of C API functions easy because only the function name must be specified, not all types of parameters? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79423/new/ https://reviews.llvm.

[PATCH] D79425: [analyzer] StdLibraryFunctionsChecker: Add overload for adding the same summary for different names

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:672 +operator()(Name, S); +} } addToFunctionSummaryMap(ACtx, FunctionSummaryMap); Theoretically it is possible to have a //multiple names//

[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

balazske added a comment. An empty signature means really that the whole signature is "irrelevant"? It is like all types and number of arguments is irrelevant. Probably that would be a better name, instead of `empty` use `isIrrelevant`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:697 if (auto *FD = dyn_cast(D)) { - if (S.matchesSignature(FD)) { + if (S.Sign.matches(FD) && S.validate(FD)) { auto Res = Map.inse

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

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:152-158 /// Get the value of the stream argument out of the passed call event. /// The call should contain a function that is described by Desc. SVal getStreamArg(const FnDescriptio

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

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

[PATCH] D79433: [analyzer] StdLibraryFunctionsChecker: Add summaries for POSIX

balazske added a comment. The checker or at least the source file should be split somehow, adding of summaries should be separated from the other parts, otherwise the file will be too long. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79433/new/

[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

balazske added a comment. Probably something like `IsCStdLibraryFunction` can be included in the signature, and empty signature is allowed only for functions that are in a C standard library. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79423/new

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

balazske added a comment. The "indeterminate file position" case is now handled separately and is always error. The current implementation looks to be wrong because the indeterminate position is set at `fread` when a **FEOF** error is produced. This can be solved probably only with more state s

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

balazske added a comment. I am not sure if this FEOF thing is such a big problem. The modeling is not exact but practically this does not cause big problems and still better than before this patch. The problem is, if a `fread` is made and the stream is already in **EOF** state the (modeled) `fr

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

balazske marked 4 inline comments as done. balazske added a comment. I do not know what other than `typedef` or `sizeof` (and variable declaration) can contain VLA. To my knowledge VLA is not normally supported in C++ and should not be used anyway (there are better ways for doing it) so some fun

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

balazske updated this revision to Diff 262661. balazske marked 2 inline comments as done. balazske added a comment. Added better comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79072/new/ https://reviews.llvm.org/D79072 Files: clang/lib/

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

balazske marked 3 inline comments as done. balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:577 + if (isa(*DS->decl_begin())) { +ExplodedNodeSet DstPre; +getCheckerManager().runCheckersForPreStmt(DstPre, Pred, DS, *this);

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

balazske added a comment. This change is relatively small and the refactoring like part (introduction of `checkVLA` if I think correct?) is connected to its use. The other change is add of a new function (callback). This is probably small enough to go into one change and we can see why the new

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

balazske added a comment. I do not know if it would me **much** cleaner. - First part: Move calculation of `ArraySize` into `checkVLA` and rename `checkVLASize` to `checkVLAIndexSize`. - Second part: Add the check for total array size to `checkVLA`. The first part in itself does not make sense

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

balazske 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" Are these new includes needed? Reposito

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

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:481 + ProgramStateRef State = C.getState(); + if (isa(Call) || isa(Call)) { + The one thing where I am not sure is if this condition is really needed for ev

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

balazske accepted this revision. balazske 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/D78121/new/ https://reviews.llvm.org/D78121 __

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

balazske accepted this revision. balazske added a comment. This revision is now accepted and ready to land. Did not found problems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78124/new/ https://reviews.llvm.org/D78124

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

balazske updated this revision to Diff 263151. balazske added a comment. Added target dependent tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://reviews.llvm.org/D79330 Files: clang/lib/StaticAnalyzer/Checkers/VLASizeCheck

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

balazske added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:403 /// frame. May fail; returns null on failure. - const VarRegion *getParameterLocation(unsigned Index, -unsigned BlockCoun

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

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp:176-180 if (const auto *DeclReg = Reg->getAs()) { if (isa(DeclReg->getDecl())) Reg = C.getState()->getSVal(SV.castAs()).getAsRegion(); + } else if (const auto *ParamReg

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

balazske added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:362 - return ASTUnit::LoadFromASTFile( - std::string(ASTFilePath), CI.getPCHContainerOperations()->getRawReader(), - ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()); + aut

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

balazske updated this revision to Diff 263447. balazske added a comment. Added state split before fread to make warning for error state possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78374/new/ https://reviews.llvm.org/D78374 Files: cla

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

balazske added a comment. 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, independent of the previous error state (the "ferror" may happen because a temporary disk erro

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

balazske marked 2 inline comments as done. balazske added a comment. The intent is to model the fread-fwrite function failure by returning the error value and set the stream into error state. The error state is a composite of **ferror** and **feof**. The questions are now, at what case do these

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

balazske added a comment. The problem with small changes is that still we (at least I who writes the code) should know the final goal (and this can be hard if I have multiple not related problems to work on and everything goes forward by little pieces spread out in a long time). (Also the revie

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

balazske added a comment. The indeterminate position is mentioned only at fread and fwrite. I do not know if it is reasonable to make the indeterminate position in other cases. Th indeterminate position is separate from error flags because of "clearerr" that clears the error flag but not the in

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