[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

2023-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1189 +``onwership_returns``: Functions with this annotation return dynamic memory. +The second annotation parameter is the size of the returned memory in bytes. + aaron.ballman

[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

2023-08-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 548116. Szelethus marked 4 inline comments as done. Szelethus added a comment. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156787/new/ https://reviews.llvm.org/D156787 Files:

[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

2023-08-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, steakhal, donat.nagy, balazske, gamesh411. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, yaxunl. Herald added a reviewer: aaron.ballman. Herald

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

2023-07-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. But this renders much of my C++98 knowledge obsolete!11 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154325/new/

[PATCH] D153612: [clang][analyzer] Add and change NoteTags in StdLibraryFunctionsChecker.

2023-06-29 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. This is the quintessential example of a patch where while the test files look promising, we need some evaluation on the real world. I understand that its a chore -- but this is simply the nature of the beast. While the changes in the test file look promising. I'd be

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D152436#4443858 , @balazske wrote: > In D152436#4438956 , @NoQ wrote: > >> I'm somewhat skeptical of the decision made in D151225 >> because the

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-21 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D152436#4432736 , @balazske wrote: > From these two solutions, which one is better? (Show many unnecessary notes, > or show only necessary ones but lose some of the useful notes too.) How likely are the problems with the

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-12 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D152436#4411912 , @steakhal wrote: > In D152436#4408811 , @balazske > wrote: > >> In D152436#4408301 , @steakhal >> wrote: >> >>> I looked

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-09 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. > I am not sure about the exact requirements, this review can be a place for > discussion about what should be fixed (if any). D52984 added the "Making your checker better" section to the dev manual:

[PATCH] D151225: [clang][analyzer] Merge apiModeling.StdCLibraryFunctions and StdCLibraryFunctionArgs checkers into one.

2023-05-26 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. Ugh, yeah, so it has come to this. I championed my idea of granulalizing checkers into modeling and reporting components since what... 2018? I think the goal is still something to shoot

[PATCH] D149447: [clang][analyzer] Improve documentation of StdCLibraryFunctionArgs checker (NFC)

2023-05-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/analyzer/checkers.rst:2476-2477 +suppressed. However, the assumption about the argument is still modeled. +For instance, if the argument to

[PATCH] D149447: [clang][analyzer] Improve documentation of StdCLibraryFunctionArgs checker (NFC)

2023-05-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added inline comments. This revision now requires changes to proceed. Comment at: clang/docs/analyzer/checkers.rst:2457 If the user disables the checker then the argument violation warning is suppressed. However, the

[PATCH] D149321: [clang][analyzer] Display buffer sizes in StdCLibraryFunctionArgs checker

2023-04-27 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:889 + State, getArgSVal(Call, ArgN))) +Out << " (that is " << *Val << ")"; +} I think this would be correct. Repository: rG LLVM

[PATCH] D149321: [clang][analyzer] Display buffer sizes in StdCLibraryFunctionArgs checker

2023-04-27 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: steakhal, gamesh411. Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. I am Szelethus, and I approve this warning message! Now, on another issue, if we can output such a thorough message, we should start to

[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-04-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. LGTM! Most of the diagnostic messages are short but precise. I like this very much. Well done! Balázs actually tested the change on open source projects, but accidentally uploaded it to our internal server, so I have seen them, and

[PATCH] D144003: [clang][analyzer] Improve bug reports of StdLibraryFunctionsChecker.

2023-03-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Please run this on open source projects and upload the results. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:164-166 +/// This function is called when the current constraint represents the +/// opposite of a

[PATCH] D145069: [analyzer][NFC] Split the no state change logic and bug report suppression into two visitors

2023-03-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:449 // No diagnostic if region was modified inside the frame. if (!CallExitLoc || isModifiedInFrame(N)) return nullptr; We also lost this,

[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

2023-03-09 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/D143751/new/ https://reviews.llvm.org/D143751

[PATCH] D145069: [analyzer][NFC] Split the no state change logic and bug report suppression into two visitors

2023-03-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. We worked on this together, so I waited a bit for others to have a say in this, but this design seems like a no brainer to me. Please fix those comments, otherwise LGTM. Also, while the patch is LGTM (moving code around is okay), the comment says "system headers

[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

2023-02-27 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. A high level comment before getting into (even more) nitty gritty stuff. But shut me down if I misunderstood whats happening. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:124-125 // requirement would render the

[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

2023-02-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D144269#4143066 , @NoQ wrote: > The challenging part with note tags is how do you figure out whether your bug > report is taint-related. The traditional solution is to check the `BugType` > but in this case an

[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

2023-02-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ugh, I admit, its a little hard to follow what happened here. You moved a lot of code around (I agree with that!), but also changed code as well. Can you just summarize what is NOT just moved code and needs a more thorough look? Comment at:

[PATCH] D143194: [clang][analyzer] Make messages of StdCLibraryFunctionsChecker user-friendly

2023-02-14 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! Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:95 +QualType ArgT, BasicValueFactory , +

[PATCH] D143194: [clang][analyzer] Make messages of StdCLibraryFunctionsChecker user-friendly

2023-02-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. A small nit, otherwise LGTM. In D143194#4112538 , @balazske wrote: > Probably it is not always useful to explain why the argument is wrong. In > cases when we can find out that the value is exactly between two consecutive >

[PATCH] D143194: [clang][analyzer] Make messages of StdCLibraryFunctionsChecker user-friendly

2023-02-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: gamesh411. Szelethus added a comment. Awesome, been a long time coming!! Other than the minor observation of changing "of function" to "to", I'm inclined to accept this patch. We definitely should describe what the value IS, not just what is should be (aside from

[PATCH] D142354: [analyzer] Create a stub for an std::variant checker

2023-02-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. While we were there, we also dug into `std::any`, and learned that the analyzer can model it shockingly well. Hopefully we can submit a few patches that demonstrates it in a form of some test files. In D142354#4079758 ,

[PATCH] D142354: [analyzer] Create a stub for an std::variant checker

2023-01-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D142354#4078450 , @NoQ wrote: > Interesting, what specific goals do you have here? Are you planning to find > specific bugs (eg. force-unwrap to a wrong type) or just to model the > semantics? Hi! Meant to write this

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. LGTM! I think I prefer this solution anyways. Please commit (the entire patchstack). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140387/new/ https://reviews.llvm.org/D140387

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:507-511 + // FIXME: Use a return value from EvalFn instead of isDifferent. + // Some functions should not change state and not have any other + // (invalidation, including errno)

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-04 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. Please rerun the evaluation before commiting to confirm the results haven't changed! Otherwise, LGTM. Comment at: clang/test/Analysis/stream.c:89 void

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-04 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/D140395/new/ https://reviews.llvm.org/D140395

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. LGTM, granted you add that test in the followup commit. If possible, I'd prefer to have features tested in the patch that added them (but this is fine for now). Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838 +

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Mostly LGTM, but I see that you have tests for the predecessor patch here as well, so I'll accept both at once. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:89 /// The last file operation called in the stream. + /// Can be

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838 + Result += getArgDesc(ArgN); + Result += DK == Violation ? " should not be zero" : " is not zero"; + return Result.c_str(); I don't mean to make

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D140387#4021806 , @Szelethus wrote: > Would be possible to test the errno specific changes as well? I suppose the tests are done in the followup patch mostly? Comment at:

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2023-01-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Are you sure that the refactoring made no changes to the results? Could you maybe just run a nightly or something like that to confirm? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137790/new/

[PATCH] D140395: [clang][analyzer] Extend StreamChecker with some new functions.

2023-01-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Does this patch fix any false positives from before, or is this just all new stuff? I ask, because I wonder whats the shortest path towards popping these checkers out of alpha, and fix what we already have. By no means am I saying that we should postpone landing

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2023-01-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Would be possible to test the errno specific changes as well? Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:310 + const MemRegion *ErrnoR = State->get(); + if (!ErrnoR) +return State; When can this occur?

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2022-12-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D140387#4007751 , @balazske wrote: > This patch and D140395 is (almost) the > same code as D135360 and D135247 > . The

[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.

2022-12-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: gamesh411. Szelethus added a comment. Some of the changes are also present in D135247 . I suppose you're in the middle of splitting those patches apart and remaking the patch stack? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I have a fear that we may have too few results as well -- maybe we should expand our testing infrastructure with more POSIX API heavy codebases. Looking at a few new projects, https://github.com/audacity/audacity looks like a good candidate, but of course it often

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D137790#3992216 , @balazske wrote: > On the postgres results, the second is one that can be fixed in the checker > (add special cases to `StdLibraryFunctionsChecker` for zero `len` or `size` > `fread` and `fwrite`

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:219 + /// @param State The state of the generated node. + /// @param Pred The transition will be generated from the specified Pred node + /// to

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D137790#3992216 , @balazske wrote: > Some reports can be found here > (if the > link works and the data does not expire), the runs stored on 2022-12-09. > >

[PATCH] D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

2022-12-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D135360#3981420 , @balazske wrote: > My current approach is that the POSIX is more strict than the C standard > (POSIX allows a subset of what C allows). I do not see (errno related) > contradiction between these standards

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

2022-12-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953 if (FailureSt && !SuccessSt) { - if (ExplodedNode *N = C.generateErrorNode(NewState)) + if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

2022-12-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953 if (FailureSt && !SuccessSt) { - if (ExplodedNode *N = C.generateErrorNode(NewState)) + if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

2022-12-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. IIRC we talked about it would only really make sense to evaluate this patch stack as a whole, not piece by piece, but I'm not seeing results on open source projects here either. Can you please post them? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

2022-12-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. The patch looks OK now, I'll get to inspecting the others. In D135247#3977403 , @balazske wrote: > The "strange" test failures that showed up earlier were probably caused by a > bug that is fixed in the D137722

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

2022-12-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:953 if (FailureSt && !SuccessSt) { - if (ExplodedNode *N = C.generateErrorNode(NewState)) + if (ExplodedNode *N = C.generateErrorNode(NewState, NewNode))

[PATCH] D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

2022-12-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D135360#3862260 , @martong wrote: > In D135360#3839890 , @balazske > wrote: > >> I found some anomalies during development: >> >> - If the checker **StdCLibraryFunctions** is added

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

2022-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Sorry abour my previous reply, I messed up the thread I was replying to. I better see what is going on. Do you have a better handle on @martong's previous comment (D135247#3867603 )? Do we know why this strange behaviour

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

2022-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Do I understand correctly that for the time being, the strategy is to assume apiModeling to be enabled by default and not enforce it explicitly? I suppose I have the ol' reliable question: did you evaluate this on any project yet? In D135247#3839543

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-26 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 rGa504ddc8bf9d: [analyzer] Initialize regions returned by CXXNew to undefined (authored by Szelethus). Changed prior to commit:

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Seems like the issues mentioned above are real, but orthogonal to this patch. Would it be okay to address them in followup patches? @martong @NoQ Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:927 SVal RetVal = State->getSVal(CNE,

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Some early results: https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Unreviewed=Confirmed%20bug=New=Reopened=Unresolved=on=%2acxxnew_baseline=%2acxxnew_patched=New I'm still waiting on a few projects, but this is something. A few

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Just a note on the test files -- I've diverged from the usual stance of just changing what the new output is, to modifying the test files. The reason is that reading an undefined value is a fatal error, leading to the analyzer to stop analyzing prematurely, and I

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

2022-10-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, martong, steakhal, balazske, isuckatcs. Szelethus added a project: clang. Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.

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

2022-07-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a subscriber: balazske. Szelethus added a comment. Yeah, I'm afraid no fun is allowed on this block. On another note, `kaboom` is interesting, shouldn't we assume all functions to be `kaboom` unless proven to be `woot`? @balazske's work on checking the return value of certain

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

2022-07-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I read https://en.cppreference.com/w/cpp/language/structured_binding carefully, and there are a number of interesting rules that might deserve their own test case, even if this isn't the patch where you solve that issue, or believe that the solution handles it

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

2022-06-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Herald added a subscriber: steakhal. No need for post commit fixes, just general observations since I noticed them. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2427 + uint64_t ArrSize = CAT->getSize().getLimitedValue(); + if (ArrSize

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

2022-04-08 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 rGfd8e5762f86f: [analyzer] Dont track function calls as control dependencies (authored by Szelethus). Changed prior to commit:

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

2022-03-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5908-5909 +void CFG::dump(bool ShowColors) const { dump(LangOptions{}, ShowColors); } + /// print - A simple pretty printer of a CFG that outputs to an ostream.

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

2022-03-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 419132. Szelethus marked an inline comment as done. Szelethus added a comment. Herald added a project: All. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116597/new/ https://reviews.llvm.org/D116597 Files:

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

2022-03-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus abandoned this revision. Szelethus added a comment. Very well :) Let's abandon this in its current state, I share this sentiment: In D120992#3368118 , @NoQ wrote: > What I'm trying to say here is, I honestly think re-doing it as CFG-based >

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

2022-03-24 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! You did check whether a missing doc field will actually trigger this error, right? Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:82 +

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

2022-03-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. LGTM on my end, this is awesome! In D122285#3401754 , @steakhal wrote: >> The notes are prunable, i.e. they won't bring-in entire stack frames worth >> of notes just because they're there, but they will be always visible >>

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

2022-03-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Seems like all new files are missing the header blurb about the licence. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121197/new/ https://reviews.llvm.org/D121197 ___

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

2022-03-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. Nice! Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:27 -static std::string getPackageFullName(const Record *R); +static std::string

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

2022-03-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D120992#3364804 , @NoQ wrote: > This check checks must-properties/all-paths properties. This has to be a data > flow / CFG-based warning. I don't think there's a way around. Oof. I concede on that. Do you think there is

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

2022-03-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullPtrInterferenceChecker.cpp:166 +/// child is a sink node. +static bool unconditionallyLeadsHere(const ExplodedNode *N) { + size_t NonSinkNodeCount = llvm::count_if( xazax.hun

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

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

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

2022-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd832078904c6: [analyzer] Improve NoOwnershipChangeVisitors understanding of deallocators (authored by Szelethus). Herald added a project: All. Changed prior to commit:

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

2022-03-01 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG32ac21d04909: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs (authored by Szelethus). Repository: rG LLVM Github Monorepo

[PATCH] D120325: [analyzer] Don't crash if the analyzer-constraint is set to Z3, but llvm is not built with it

2022-02-25 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5048a58a6792: [analyzer] Dont crash if the analyzer-constraint is set to Z3, but llvm is not… (authored by Szelethus). Changed prior to commit:

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

2022-02-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h:102-103 + /// Returns true if the CallEvent is a call to a function that matches + /// the CallDescription. + /// steakhal wrote: > NoQ

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

2022-02-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 410833. Szelethus marked 5 inline comments as done. Szelethus added a comment. Remove a newline. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119004/new/ https://reviews.llvm.org/D119004 Files:

[PATCH] D119128: [analyzer] Fix taint propagation by remembering to the location context

2022-02-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Can we reopen this if the code is not upstream at this time? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119128/new/ https://reviews.llvm.org/D119128 ___ cfe-commits mailing

[PATCH] D120325: [analyzer] Don't crash if the analyzer-constraint is set to Z3, but llvm is not built with it

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

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

2022-02-09 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/test/Analysis/NewDeleteLeaks.cpp:152 + // like to deallocate anything. + bar(); +} steakhal wrote: > I guess we would not have the warning if `bar` would accept the pointer `P`, > since that way it would

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

2022-02-09 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 407121. Szelethus marked 8 inline comments as done. Szelethus added a comment. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118880/new/ https://reviews.llvm.org/D118880 Files:

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

2022-02-09 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 407106. Szelethus added a comment. - Rename from `.*Imprecise` to `.*AsWritten`. - Copy comments to relevant functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119004/new/ https://reviews.llvm.org/D119004 Files:

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

2022-02-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Now that I remember, the ever so slightly different overloads of `ProgramState::getSVal` is a prime example I think. I always percieved that I have the means to invoke several of them at any point, but I never really knew which one. Though, to be fair, they were not

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

2022-02-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D119004#3297025 , @steakhal wrote: > I strongly belive that this should be an overload to the existing 'matches' > API. Maybe add a comment that prefer the other overload if can. But having an > overload for that alread

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

2022-02-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 405979. Szelethus edited the summary of this revision. Szelethus added a comment. Move `CallDescription` specific changes to D119004 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118880/new/

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

2022-02-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, steakhal, xazax.hun, martong, ASDenysPetrov. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Szelethus requested review of this revision. Herald added

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

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

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

2022-02-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ping ^-^ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116597/new/ https://reviews.llvm.org/D116597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2022-01-24 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 402549. Szelethus added a comment. Fix tests, mention that this is purely a heuristic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116597/new/ https://reviews.llvm.org/D116597 Files: clang/include/clang/Analysis/CFG.h

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

2022-01-24 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3ad35ba4dea5: [Templight] Dont display empty strings for names of unnamed template parameters (authored by Szelethus). Changed prior to commit:

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

2022-01-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/lib/Frontend/FrontendActions.cpp:501 + +if (const auto *Decl = dyn_cast(NamedTemplate)) { + if (const auto *R = dyn_cast(Decl)) { Szelethus wrote: > martong wrote: > > martong wrote: > > > Should this

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

2022-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: dkrupp, NoQ, steakhal, gamesh411, martong, balazske. Szelethus added a project: clang. Herald added subscribers: manas, ASDenysPetrov, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, kristof.beyls,

[PATCH] D115716: [Analyzer][BugReporter] Replace the example bug report with the one used to generate PathDiagnostic

2022-01-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:76 + std::unique_ptr> { + const BugReport *Report = nullptr; +}; Some comments about this field would be welcome!

[PATCH] D115716: [Analyzer][BugReporter] Replace the example bug report with the one used to generate PathDiagnostic

2022-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. First off, your patch is great, and I'm pretty sure we want it! I remember working around here, and I either have never quite understood what makes `exampleReport` an example, or have long forgotten. Can we not just rename it to `chosenReport`, or simply `report`?

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

2022-01-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 397054. Szelethus added a comment. Add a default text, if another, unhandled unnamed identifier pops up. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115521/new/ https://reviews.llvm.org/D115521 Files: clang/lib/Frontend/FrontendActions.cpp

[PATCH] D115521: [Templight] Don't return string for name for unnamed template parameters

2021-12-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: sabel83, xazax.hun, rsmith, whisperity, martong, mikael-s-persson, mclow.lists. Szelethus added a project: clang. Herald added subscribers: steakhal, gamesh411, dkrupp, rnkovacs. Szelethus requested review of this revision. Herald added

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-15 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Szelethus marked 2 inline comments as done. Closed by commit rG29a8d45c5a23: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators (authored by Szelethus). Changed prior to commit:

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: Eugene.Zelenko. Szelethus added a comment. I'll intend to land this by friday unless there are objections! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113201/new/ https://reviews.llvm.org/D113201 ___ cfe-commits

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

2021-11-09 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8cc2de667ec2: [analyzer][docs] Fix the incorrect structure of the checker docs (authored by Szelethus). Changed prior to commit:

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

2021-11-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 385486. Szelethus added a comment. Add context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113397/new/ https://reviews.llvm.org/D113397 Files: clang/docs/analyzer/checkers.rst Index: clang/docs/analyzer/checkers.rst

  1   2   3   4   5   6   7   8   9   10   >