[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

steakhal updated this revision to Diff 435611. steakhal marked 2 inline comments as done. steakhal added a comment. - Use `assert` macro for introducing constraints. - Add tests for store binding invalidations, besides constraint invalidations. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:979-980 +if (Ty.isConstQualified() && Ty->isArithmeticType()) { // TODO: We could walk the complex types here and see if everything is // constified. + sReg = getGlobals

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. It's getting in really good shape. Nice work. I've got some nits about docs and checker dependencies and also a major comment inline. The documentation needs to be updated to kee

[PATCH] D127389: [analyzer] Print the offending function at EndAnalysis crash

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG07a7fd314a11: [analyzer] Print the offending function at EndAnalysis crash (authored by steakhal). Changed prior to commit: https://reviews.llvm.o

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd50d9946d1d7: [analyzer] Deprecate `-analyzer-store region` flag (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D126216: [analyzer][NFC] Remove unused RegionStoreFeatures

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb73c2280f5f3: [analyzer][NFC] Remove unused RegionStoreFeatures (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D126067: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG07b4a6d0461f: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag (authored by steakhal). Repository: rG LLVM Github Mon

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

steakhal added a comment. In D126215#3572984 , @thakis wrote: > Looks like this breaks building clang-tidy: > http://45.33.8.238/linux/78232/step_4.txt > > Please take a look, and revert for now if it takes a while to fix. Thanks. It should be fixed by

[PATCH] D127485: [analyzer][NFC] Remove unused Analyses enum

steakhal created this revision. steakhal added a reviewer: martong. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: All. steakhal

[PATCH] D127486: [analyzer][NFC] Inline AnalyzerOptions::getUserMode()

steakhal created this revision. steakhal added a reviewer: martong. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: All. steakhal

[PATCH] D126067: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag

steakhal added a comment. In D126067#3573056 , @thakis wrote: > One of your changes broke check-clang: > http://45.33.8.238/linux/78236/step_7.txt > > Please take a look and revert for now if it takes a while to fix. > > (Please run tests locally before

[PATCH] D158156: [analyzer] Add C++ array delete checker

steakhal added a comment. The code looks good now. Let's have one last round. Do you have real-world reports? Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:762-763 +def CXXArrayDeleteChecker : Checker<"ArrayDelete">, + HelpText<"Reports destructions of a

[PATCH] D159549: [analyzer] Fix false negative when accessing a nonnull property from a nullable object

steakhal added a comment. For new revisions, prefer creating a PR at GitHub. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:903-904 if (!TrackedNullability && - getNullabilityAnnotation(ReturnType) == Nullability::Nullable) { + getNullabilityA

[PATCH] D158156: [analyzer] Add C++ array delete checker

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. It's good enough already, given you apply my last suggestion. References would be nice to support for notes, but not a blocker. Comment at: clang/lib/StaticAnalyzer/Check

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

steakhal added a comment. In D153612#4505307 , @balazske wrote: > No major problems were indicated with the patch stack, I plan to merge all of > these soon. Small problems can be still corrected before or when the checker > is put out from the alpha pa

[PATCH] D155445: [analyzer][docs] Add CSA release notes

steakhal updated this revision to Diff 541356. steakhal marked 2 inline comments as done. steakhal added a comment. In D155445#4508728 , @OikawaKirie wrote: > LGTM for my part. Thx. > > Since I am not very familiar with other changes, I have no detailed

[PATCH] D155445: [analyzer][docs] Add CSA release notes

steakhal added inline comments. Comment at: clang/docs/ReleaseNotes.rst:922-923 +- The ``CStringChecker`` will invalidate less if the copy operation is + inferable to be bounded. For example, if the argument of ``strcpy`` is known + to be of certain length and that is in-bounds

[PATCH] D155574: [clang][ASTImporter] Fix import of recursive field initializer.

steakhal accepted this revision. steakhal 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/D155574/new/ https://reviews.llvm.org/D155574 ___

[PATCH] D155547: [analyzer] Model lifetime of a variable declared in for condition in CFG correctly

steakhal added inline comments. Comment at: clang/test/Analysis/lifetime-cfg-output.cpp:1 // RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-temporary-dtors=false,cfg-rich-constructors=false -analyz

[PATCH] D155847: [analyzer] Fix crash in GenericTaintChecker when propagatig taint to AllocaRegion

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Makes sense to me. Please, someone else also have a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155847/new/ https://reviews.llvm.org

[PATCH] D155848: [clang][analyzer]Fix non-effective taint sanitation

steakhal added a comment. Thanks, all good! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155848/new/ https://reviews.llvm.org/D155848 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D155445: [analyzer][docs] Add CSA release notes

steakhal added a comment. After removing all commit refs, here is how it looks: F28416948: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155445/new/ https://reviews.llvm.org/D155445 __

[PATCH] D155445: [analyzer][docs] Add CSA release notes

This revision was automatically updated to reflect the committed changes. Closed by commit rG862b93a8095c: [analyzer][docs] Add CSA release notes (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D155445?vs=541356&id=543390#toc Repository: rG LLVM Github Monorepo CHA

[PATCH] D145229: [analyzer] Improve the documentation of the alpha.security.taint.TaintPropagation checker

steakhal added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2471 + #ifdef __clang_analyzer__ +void csa_mark_sanitized(const void *); + #endif Have you considered unconditionally having this function with an empty body? That way it would hav

[PATCH] D145229: [analyzer] Improve the documentation of the alpha.security.taint.TaintPropagation checker

steakhal added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2469-2472 + // User csa_mark_sanitize function is for the analyzer only + #ifdef __clang_analyzer__ +void csa_mark_sanitized(const void *); + #endif I was thinking of this when I

[PATCH] D155661: [ASTImporter] Fix friend class template import within dependent context

steakhal added a comment. @balazske Could you please have a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155661/new/ https://reviews.llvm.org/D155661 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D154838: [analyzer] Add check for null pointer passed to %p of printf family

steakhal added a comment. Looks pretty good. Comment at: clang/docs/analyzer/checkers.rst:704 +optin.portabilityMinor.UnixAPI +" +Finds non-severe implementation-defined behavior in UNIX/Posix functions. This line should be just as long,

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

steakhal added a comment. Great progress. Now, moving to the docs. Have you checked that the docs compile without errors and look as you would expect? Please post how it looks. Please make a note about this new checker in the clang's release docs, as we usually announce new checkers and major c

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Correct. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157702/new/ https://reviews.llvm.org/D157702 ___

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

steakhal added a comment. Do we have other typos like this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157702/new/ https://reviews.llvm.org/D157702 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D157702: [clang][analyzer][NFC] Change PlacementNewChecker into PlacementNew in documentation

steakhal added a comment. In D157702#4580384 , @PiotrZSL wrote: > In D157702#4580310 , @steakhal > wrote: > >> Do we have other typos like this? > > I don't think so, only mess in overall documentation: > https:

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

steakhal accepted this revision. steakhal added a comment. Looks safe and good. I'm interested in the diff though. Let me know once you have the results. I wanna have a look before we land this. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:184 +// s

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

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. As I don't use this checker, thus I cannot speak of my experience. However, the reasons look solid and I'm fine with moving this checker to `unix.StdCLibraryFunctions`. Let some time for ot

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

steakhal added a comment. I'm sorry starting the review of this one only now, but I'm quite booked. Is it still relevant? If so, I'll continue. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:116-117 +const NoteTag *Note = +C.getNoteTag([Reg

[PATCH] D150647: [WIP][analyzer] Fix EnumCastOutOfRangeChecker C++17 handling

steakhal added a comment. Isn't D153954 superseded this one? If so, consider abandoning it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150647/new/ https://reviews.llvm.org/D150647

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

steakhal added a comment. I notices some inconsistency between this `-fstrict-flex-arrays=N` flag and what the `RecordDecl::hasFlexibleArrayMember()` returns for an example like this: typedef unsigned long size_t; void *malloc(size_t); void free(void *); void field(void) { struct

[PATCH] D157888: [NFC][Clang] Fix static analyzer concern about null value dereference

steakhal added a comment. Hmm. I guess the assertion is to silence some tool. And I think actually that function might very well also return null in some cases. Why do you think it cannot or at least should not return null in your context? I couldn't infer this from the context, neither from the

[PATCH] D157888: [NFC][Clang] Fix static analyzer concern about null value dereference

steakhal added a comment. If `getStmtForDiagnostics()` in general, never returns null, then shouldn't we mark the API with an appropriate attribute? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157888/new/ https://reviews.llvm.org/D157888 _

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. The test coverage is also impressive. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 __

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

steakhal added a comment. Thanks for the xrefs! It turns out I've already seen that one :D I'll look into it once more, with a fresh set of eyes. Thanks again! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/ https://reviews.llvm.org/D1268

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

steakhal accepted this revision. steakhal added a comment. In D156312#4588406 , @donat.nagy wrote: > After investigating this issue, I added the testcases > `signed_aritmetic_{good,bad}` which document the current sub-optimal state. > The root cause of

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

steakhal added a comment. 🎉 🚀 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D158295: [NFC][clang][analyzer] Avoid potential dereferencing of null pointer value

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. Thanks for the PR. I went over the code and concluded that it can never be null. However, the code could be improved, while making this explicit. Comment at: c

[PATCH] D158285: [NFC][CLANG] Fix wrong orders of function arguments positions

steakhal requested changes to this revision. steakhal added subscribers: tbaeder, ABataev. steakhal added a comment. This revision now requires changes to proceed. I think we have two 2 TPs, that were interesting to see. I've CCd the code owners of the affected parts to confirm this. As a genera

[PATCH] D158360: [docs] Update the static analyzer bug reporting page

steakhal added a comment. Makes sense to me. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158360/new/ https://reviews.llvm.org/D158360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D158360: [docs] Update the static analyzer bug reporting page

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Makes sense to me, thanks! I'm not sure though how much we wanna invest into maintaining these handwritten htmls. I would prefer moving away from these. It would likely also fit more nice

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

steakhal accepted this revision. steakhal added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157104/new/ https://reviews.llvm.org/D157104 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

steakhal added a comment. Hold on, make sure this is clang-format compliant before committing. (Check buildkite) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157104/new/ https://reviews.llvm.org/D157104 ___

[PATCH] D158285: [NFC][CLANG] Fix wrong orders of function arguments positions

steakhal added a comment. In D158285#4603477 , @aaron.ballman wrote: >> I've seen a few similar patches from you @Manna and probably some related >> folks. >> Could you please clarify the motivation and the expectations? Or feel free >> to point me to

[PATCH] D158156: [analyzer] Add C++ array delete checker

steakhal added a comment. I share the raised concerns. And I think we are aligned. PS: sorry if any of my comments are dups, or outdated. I only had a little time last week, and things have progressed since then I noticed. I still decided to submit my possibly outdated and partial review commen

[PATCH] D158156: [analyzer] Add C++ array delete checker

steakhal added a comment. One more thing to mention. Its usually illadvised to rename files or do changes that would seriously impact git blame, unless we have a really good reason doing so. Aesthetics usually don't count one, especially if the name is not user-facing. However, maintainability

[PATCH] D158499: [analyzer] Compute FAM dynamic size

steakhal added a comment. Thanks for submitting this. A funny thing is that in my free time I was also working on this last week. I'll have a look at this more in depth during the week. For the mean time here is my version, committed pretty much a couple days ago to my fork. https://github.com/l

[PATCH] D158499: [analyzer] Compute FAM dynamic size

steakhal added a comment. In D158499#4606337 , @danix800 wrote: > In D158499#4606291 , @steakhal > wrote: > >> Thanks for submitting this. >> A funny thing is that in my free time I was also working on this last

[PATCH] D158499: [analyzer] Compute FAM dynamic size

steakhal added a comment. In D158499#4608847 , @danix800 wrote: > We have `getDynamicExtentWithOffset` to use, which can handle more general > dynamic memory based cases than this fix. > > I'll abandon this one. > > There are issues worth clarifying and f

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

steakhal added a comment. In D154603#4609872 , @gamesh411 wrote: > - try to consolidate the multiple warnings coming from this checker's > `checkLocation` callback > > category based filtering ( example from > lib/StaticAnalyzer/Checkers/GenericTaintChe

[PATCH] D157385: [clang][CFG] Cleanup functions

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. This is a great improvement. When I saw that clang now supports it and e.g. the CSA operates on the CFG, I also considered adding this. Now I don't need to do it, so many thanks!

[PATCH] D156724: [StaticAnalyzer] Fix incorrect link to "note" diagnostics in HTML output

steakhal added a comment. Could you please add a test for the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156724/new/ https://reviews.llvm.org/D156724 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D156724: [StaticAnalyzer] Fix incorrect link to "note" diagnostics in HTML output

steakhal added a comment. In D156724#4547891 , @gruuprasad wrote: > Sure, Initially I didn't find the folder `StaticAnalysis` in the `clang/test` > folder, found that relevant tests are in `test/Analysis`. Correct, thanks! Repository: rG LLVM Githu

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

steakhal added a comment. I haven't looked at the content but I have the feeling that we should probably backport this to release/17.x Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156787/new/ https://reviews.llvm.org/D156787

[PATCH] D156201: [ASTImporter] Fix corrupted RecordLayout introduced by circular referenced fields

steakhal added a comment. Have you considered back porting this to clang-17? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156201/new/ https://reviews.llvm.org/D156201 ___ cfe-commits mailing list cfe-co

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

steakhal added inline comments. Comment at: clang/test/Analysis/malloc-annotations.c:151-155 +void af6(void) { + int *p = my_malloc(12); + my_hold(p); + *p = 4; +} Please, consider elaborating on this test with some comments, as it's not clear to me at first

[PATCH] D156724: [StaticAnalyzer] Fix incorrect link to "note" diagnostics in HTML output

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Looks correct. Please merge it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156724/new/ https://reviews.llvm.org/D156724

[PATCH] D156724: [StaticAnalyzer] Fix incorrect link to "note" diagnostics in HTML output

steakhal added a comment. In D156724#4556774 , @gruuprasad wrote: > @steakhal, ok, this is my first contribution, I don't have the commit access > yet. > Can you please merge this? What should be the commit author? Repository: rG LLVM Github Monor

[PATCH] D156724: [StaticAnalyzer] Fix incorrect link to "note" diagnostics in HTML output

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe73ae745b0d6: [analyzer] Fix incorrect link to "note" diagnostics in HTML output (authored by gruuprasad, committed by steakhal). Repository: rG

[PATCH] D153954: [clang][analyzer] Fix empty enum handling in EnumCastOutOfRange checker

steakhal accepted this revision. steakhal added a comment. LGTM Comment at: clang/test/Analysis/enum-cast-out-of-range.cpp:203 + +enum class empty_unfixed {}; + donat.nagy wrote: > Consider using "specified" and "unspecified" instead of "fixed" and > "unfixed"

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

steakhal added inline comments. Comment at: clang/docs/analyzer/checkers.rst:35 +core.BitwiseShift (C, C++) +""" + Comment at: clang/docs/analyzer/checkers.rst:44-50 +Moreover, if the pedantic mode is activated by +

[PATCH] D157114: [clang][ASTImporter] Improve StructuralEquivalence algorithm on repeated friends

steakhal resigned from this revision. steakhal added a comment. In general, I'm not really involved with the ASTImporter. Please, put me as a reviewer if the ASTImporter directly affects the Static Analyzer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D150446: [analyzer] Check ArraySubscriptExprs in ArrayBoundCheckerV2

steakhal added a comment. In D150446#4560892 , @donat.nagy wrote: > I'm abandoning this commit because it amalgamates several unrelated changes > and I think it'd be better to handle them separately: > > 1. First, there is a very simple, independent imp

[PATCH] D157129: [NFC] Fix unnecessary copy with auto.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I can only vouch for the SttaticAnalyzer portion. Those parts makes sense to me. However, I've seen cases where it doesn't resonate. As a rule of thumb, if we are "copying" only at

[PATCH] D157129: [NFC] Fix unnecessary copy with auto.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I went over the patch and I found only a single debatable case for taking by reference. To clarify why I would prefer taking by value: value semantics is good for local reasoning

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

steakhal added a comment. Thanks for the sample reports. I'm fine if we want to make it a non-alpha (released) checker. An orthogonal question is, whether we want to have it under the code package. I'm not sure there are official guidance for elevating a checker to code, but here are my assumpti

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

steakhal added a comment. Additionally, it might make sense to first "release" the checker, and after another llvm release, turn this into a Core checker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152436/new/ https://reviews.llvm.org/D152436

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

steakhal added a comment. I haven't gone deep into the implementation, but by only looking at the quality of the code and the structure I assume it implements the right behavior. I've checked the tests though, and they look good. I only found irrelevant nits. Awesome work. --- Have you consider

[PATCH] D158156: [analyzer] Add C++ array delete checker

steakhal added a comment. I have concerns mostly about the cast visitor. Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:192 + + const Stmt *S = N->getStmtForDiagnostics(); + if (!S) Discookie wrote: > steakhal wrote: > > Aren't you actuall

[PATCH] D159108: [analyzer] CStringChecker should check the first byte of the destination of strcpy, strncpy

This revision was automatically updated to reflect the committed changes. Closed by commit rGc3a87ddad62a: [analyzer] CStringChecker should check the first byte of the destination of… (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D159109: [analyzer] CStringChecker buffer access checks should check the first bytes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0954dc3fb921: [analyzer] CStringChecker buffer access checks should check the first bytes (authored by steakhal). Repository: rG LLVM Github Monor

[PATCH] D158156: [analyzer] Add C++ array delete checker

steakhal added inline comments. Comment at: clang/test/Analysis/ArrayDelete.cpp:85-88 +Derived *d3 = new DoubleDerived[10]; +Base *b3 = d3; // expected-note{{Conversion from derived to base happened here}} +delete[] b3; // expected-warning{{Deleting an array of polym

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. Thanks for the ping. I have some concerns inline. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:1000-1008 + CheckerOptions<[ +CmdLine

[PATCH] D152435: [analyzer][CStringChecker] Adjust the invalidation operation on the super region of the destination buffer during string copy

steakhal accepted this revision. steakhal added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1078 +const MemRegion *R) { +return MemRegion::FieldRegio

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. The tests are failing on main. Check the CI bots if they pass on your mach

[PATCH] D152435: [analyzer][CStringChecker] Adjust the invalidation operation on the super region of the destination buffer during string copy

steakhal added a comment. In D152435#4467672 , @OikawaKirie wrote: > I will submit it again later to update the code as you suggested. Feel free to commit it with that modifications included. > Besides, > >> Build Status >> Buildable 242737 >> Bui

[PATCH] D153017: [analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

This revision was automatically updated to reflect the committed changes. Closed by commit rG77a599ae5828: [analyzer] Fix false negative when using a nullable parameter directly without… (authored by tripleCC, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

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

steakhal added a comment. There remained a few more cases using raw-for loops, but those were slightly more tricky to uplift as they either use weird iterators or employ some fancy look-ahead/behind or iterate in parallel over different sequences etc. I'll also make a measurement to confirm it d

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

steakhal added a comment. The transformations were done by hand. So I'd encourage you all to have a look and find places where it breaks. Also, it might have personal biases about the style I settled with. Feel free to challenge. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

steakhal created this revision. steakhal added reviewers: NoQ, xazax.hun, jansvoboda11. Herald added subscribers: PiotrZSL, carlosgalvezp, manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, mgrang, szepet, baloghadamsoftware. Herald added a reviewer: Szeleth

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

steakhal planned changes to this revision. steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501 public: +const iterator &operator*() const { return *this; } +

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

steakhal added a comment. In D154437#4471225 , @jansvoboda11 wrote: > Has this not been addressed by D142861 > already? Hm, The same failure. This must be related. I'll have a look. But, you can try it yourself that t

[PATCH] D153273: [analyzer] Rework support for CFGScopeBegin, CFGScopeEnd, CFGLifetime elements

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I'd appreciate some review on this, given that a lot of you would be affected by the changes of CFG. By changes I mean, fixes for goto statements, properly calling destructors and stuff.

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

steakhal added a comment. Fixed remarks, cleaned up residual includes, rebased. Should be read to land. Final thoughts? Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501 public: +const iterator &operator*() const { return *this; } +

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

steakhal abandoned this revision. steakhal added a comment. Ah, D142861 is only on `main`, that's why we don't have that. I'll cherry-pick that then. @jansvoboda11 Thanks for the xref! Confirmed, that fixes the issue. Abandoning this one. Repository: rG LLV

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

steakhal added a comment. @tripleCC Could you clarify the status. Do you need some help or review? Do you have concerns? You made some inline comments that I couldn't put into context. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154221/new/ http

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I'd say, as tests pass and I and @xazax.hun think that this is an improvement - along with you - then we should merge this as-is and do corrections once reported. Comme

[PATCH] D154478: [analyzer][NFC] Use unique_ptrs for PathDiagnosticConsumers

steakhal created this revision. steakhal added reviewers: xazax.hun, NoQ. Herald added subscribers: PiotrZSL, carlosgalvezp, manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware. Herald added a reviewer: Szelethus. Herald added a rev

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

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp:54 + for (PathDiagnosticConsumer *Consumer : PathConsumers) { +delete Consumer; } steakhal wrote: > xazax.hun wrote: > > Hm, I wonder whether we actually want to

[PATCH] D154481: [analyzer] Remove deprecated analyzer-config options

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

[PATCH] D154481: [analyzer] Remove deprecated analyzer-config options

steakhal added a comment. I'll mention this change in the Release Notes separately as we are getting closer and closer to branching off for the clang-17 release on 25th of July. Read more here . Repository: rG LLVM G

[PATCH] D154481: [analyzer] Remove deprecated analyzer-config options

steakhal added a comment. The option was deprecated in D138659 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154481/new/ https://reviews.llvm.org/D154481 ___ cfe-commit

[PATCH] D154478: [analyzer][NFC] Use unique_ptrs for PathDiagnosticConsumers

steakhal planned changes to this revision. steakhal added a comment. Hm, it seems like it broke some CSA unittests. I'll look into that once I have some time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154478/new/ https://reviews.llvm.org/D1544

[PATCH] D154481: [analyzer] Remove deprecated analyzer-config options

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7cd1f3ad22e4: [analyzer] Remove deprecated analyzer-config options (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D1544

[PATCH] D153954: [WIP][clang][analyzer] Relax alpha.cplusplus.EnumCastOutOfRange checker

steakhal added a comment. In D153954#4480103 , @donat.nagy wrote: > I think the old "report when the value stored in an enum type is not equal to > one of the enumerators" behavior would be useful as an opt-in checker that > could be adopted by some pr

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