[PATCH] D104439: [analyzer][NFC] Demonstrate a move from the analyzer-configs `.def` file to a TableGen file

steakhal added reviewers: jansvoboda11, myhsu. steakhal added subscribers: jansvoboda11, myhsu. steakhal added a comment. Herald added a project: All. > We can put all the acceptable values in the help text AND generate the enum > class WITHOUT any redundancy. +1 > While not demonstrated, it'd

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

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I'm convinced! Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1968 +static const Expr *peelOffOuterExpr(const Expr *Ex, const ExplodedNode *N) { + E

[PATCH] D122805: [analyzer][ctu] Only import const and trivial VarDecls

steakhal added a comment. Please add a test case where the class is trivial but the global variable of such is non-const, thus the initialized expression is not imported. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:112 -// Returns true if the variable or a

[PATCH] D118050: [analyzer] Avoid checking addrspace pointers in cstring checker

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

[PATCH] D122805: [analyzer][ctu] Only import const and trivial VarDecls

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D122805#3419662 , @martong wrote: > In D122805#3419129 , @steakhal > wrote: > >> Please add a test ca

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

steakhal added a comment. Couls you please provide some context about why did we have this suppression in the first place? Other than that, I'm on board with this, but we should really investigate the historical reasons why we had this. If it turns out to be a valid usecase, but a rather speci

[PATCH] D122805: [analyzer][ctu] Only import const and trivial VarDecls

steakhal added a comment. In D122805#3421746 , @martong wrote: > In D122805#3421676 , @martong wrote: > >> In D122805#3420111 , @steakhal >> wrote: >> >>> In D122805#3419

[PATCH] D122805: [analyzer][ctu] Only import const and trivial VarDecls

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/D122805/new/ https://reviews.llvm.org/D122805 ___

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

steakhal accepted this revision. steakhal added a comment. LGTM. Please note that we dont guarantee abi nor api compatibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123155/new/ https://reviews.llvm.org/D123155 _

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

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:872-883 +if (NewState && NewState != State) { + StringRef Note = Case.getNote(); + const NoteTag *Tag = C.getNoteTag( + // Sorry couldn't help myse

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

steakhal added a comment. In D122841#3431617 , @NoQ wrote: > I looked up the history. I believe this refers to > https://clang.llvm.org/docs/LanguageExtensions.html#memory-references-to-specified-segments: > >> Annotating a pointer with address space #25

[PATCH] D126534: [analyzer] Deadstore static analysis: Fix false positive on C++17 assignments

steakhal added a comment. Looks good to me, but I think you should wait for @NoQ for approval as he had some concerns previously. @NoQ, do you think it's in a good shape for landing? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126534/new/ https://reviews.llvm.org/D126534 ___

[PATCH] D130470: [clang][analyzer] Add more wide-character functions to CStringChecker

steakhal added a comment. Other than nits, I think it looks great. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1359 +void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE, +bool IsWide) const { // int memcm

[PATCH] D128535: [analyzer] Improve loads from reinterpret-cast fields

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. steakhal marked 2 inline comments as done. Closed by commit rGa80418eec001: [analyzer] Improve loads from reinterpret-cast fields (authored by steakhal). Changed prior

[PATCH] D130476: [NFC] Fix some C++20 warnings

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGba198e35fd38: [NFC] Fix some C++20 warnings (authored by Godin, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D130470: [clang][analyzer] Add more wide-character functions to CStringChecker

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1334 + bool IsWide) const { // void *memmove(void *dst, const void *src, size_t n); // The return value is the address of the destination buffer.

[PATCH] D130470: [clang][analyzer] Add more wide-character functions to CStringChecker

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:136-137 {{CDF_MaybeBuiltin, "memcpy", 3}, - std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, false)}, + std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, +

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

steakhal added a comment. Please, consider stating the motivation behind this change. For me, by looking at the modified test, it feels like we would lose legitimate findings (aka. true-positive reports) by applying this change. From my perspective, the store to `i` is //dead//, since it will be

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

steakhal added inline comments. Comment at: clang/test/Analysis/dead-stores.cpp:225 +void functionReferenceParameter(int &i) { + i = 5; // no warning +} By checking it on godbolt, this line never raised a warning. https://godbolt.org/z/n7fP5od5q You must be ref

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D131067#3705783 , @t-8ch wrote: > In D131067#3702044 , @steakhal > wrote: > >> Please, consider stati

[PATCH] D130974: [analyzer] Fix for the crash in #56873

steakhal added a comment. Some checker should have caught the uninitialized value earlier than the `defaultEvalCall()`. I guess, the `MallocCkecher` could have checked for it in `PreStmt`. Or alternatively, the `CallAndMessageChecker::preCall()` already does something like this in the `PreVisitP

[PATCH] D131553: [analyzer] exploded-graph-rewriter: Fix python3 string encoding issues

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

[PATCH] D130974: [analyzer] Fix for the crash in #56873

steakhal added a comment. In D130974#3709502 , @isuckatcs wrote: >> Some checker should have caught the uninitialized value earlier than the >> defaultEvalCall(). >> I guess, the MallocCkecher could have checked for it in PreStmt. >> Or alternatively, th

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

steakhal added a comment. Aside from that `Returned pointer value points outside the original object with size of 10 'int' objects` reads somewhat unnatural I have no objections. A couple of nits here and there but overall I'm pleased to see more verbose bug reports. Comment

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

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

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal updated this revision to Diff 362791. steakhal added a comment. Remove the CXXThisRegion edge case handling. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107078/new/ https://reviews.llvm.org/D107078 Files: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp clang

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal marked an inline comment as done. steakhal added a comment. Thanks for the quick response. Comment at: clang/test/Analysis/copy-elision.cpp:9-10 +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 \ +// RUN:-analyzer-config elide-con

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal planned changes to this revision. steakhal marked an inline comment as done. steakhal added a comment. I'm going to check how the notes look like on real code. Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:397-399 + Report->addNote("The

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal requested review of this revision. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:397-399 + Report->addNote("The temporary object gets destroyed at the end of the " + "full expression", +

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

steakhal updated this revision to Diff 414065. steakhal added a comment. Herald added a subscriber: carlosgalvezp. Register the checker with the alias name :D CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121214/new/ https://reviews.llvm.org/D121214 Files: clang-tools-extra/clang-tid

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

steakhal added a comment. What I wanted is to highlight the relationship between the `bugprone-shared-ptr-array-mismatch` check and the `cert-mem51` rule. I thought these aliases are the _programmatic_ way of encoding this relationship, but I understand that it might be a false advertisement giv

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

steakhal planned changes to this revision. steakhal added a comment. In D121214#3372091 , @whisperity wrote: > In D121214#3369871 , @steakhal > wrote: > >> Drop the alias-related changes and preserve the note in

[PATCH] D121214: [clang-tidy][docs][NFC] Refer to the CERT rule in bugprone-shared-ptr-array-mismatch docs

steakhal updated this revision to Diff 414337. steakhal retitled this revision from "[clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch" to "[clang-tidy][docs][NFC] Refer to the CERT rule in bugprone-shared-ptr-array-mismatch docs". steakhal added a comment.

[PATCH] D121372: [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck

steakhal created this revision. steakhal added reviewers: whisperity, aaron.ballman, njames93, LegalizeAdulthood. steakhal added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, martong, rnkovacs, xazax.hun. Herald added a project: All. steakhal requested review of this revis

[PATCH] D121373: [clang-tidy][docs] Fix wrong url in DontModifyStdNamespaceCheck

steakhal created this revision. steakhal added reviewers: whisperity, aaron.ballman, njames93, LegalizeAdulthood. steakhal added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, martong, rnkovacs, xazax.hun. Herald added a project: All. steakhal requested review of this revis

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

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

[PATCH] D121459: [analyzer] Remove HasAlphaDocumentation tablegen enum value

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

[PATCH] D121214: [clang-tidy][docs][NFC] Refer to the CERT rule in bugprone-shared-ptr-array-mismatch docs

This revision was automatically updated to reflect the committed changes. Closed by commit rG200e7a847012: [clang-tidy][docs][NFC] Refer to the CERT rule in bugprone-shared-ptr-array… (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D121372: [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck

This revision was automatically updated to reflect the committed changes. Closed by commit rGd321f0992684: [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12137

[PATCH] D121373: [clang-tidy][docs] Fix wrong url in DontModifyStdNamespaceCheck

This revision was automatically updated to reflect the committed changes. Closed by commit rG665879b9e7d1: [clang-tidy][docs] Fix wrong url in DontModifyStdNamespaceCheck (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121373/ne

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

steakhal updated this revision to Diff 415747. steakhal added a comment. Fix the single case when the `Sep` was not forwarded. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 Files: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sari

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

steakhal added a comment. I'd like to reach a wider consensus regarding this patch. @aaron.ballman @NoQ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal added a comment. In D102669#3395364 , @OikawaKirie wrote: > IMO, maybe we can just leave a FIXME or something else in the test case and > commit this patch to fix the original problem we want to fix. > (of course, re-submit to rerun the test ca

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Okay, let's give it another shot. Please monitor any buildbot failures and revert promptly if needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102669/new/ https://reviews.ll

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I'm liking it. We need to improve the diagnostics and the user-facing docs as well. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:545 +

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

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

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

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

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

steakhal updated this revision to Diff 417344. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 Files: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif clang/utils/TableGen/ClangSACheckersEmitter.cpp

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

steakhal marked an inline comment as done. steakhal added a comment. Now it should be much smaller. Sorry for not doing this in the first place. Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:90-91 + + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/chec

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:68-71 + if (ErrnoLoc) { +ErrnoRegion = ErrnoLoc->getAsRegion(); +assert(ErrnoRegion && "The 'errno' location should be a memory region."); + } NoQ wrote: > st

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

steakhal added a comment. > 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 regardless > of whether the value is of interest to the bug report. I think this is > debatable, the arguably better solu

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

steakhal added a comment. You have a single bool property, yet you allow to enable/disable this by each sub-checker. It feels like the last checker in the registration process will rule them all. That being said, in the fixit creation scope, you check for both this flag and the presence of the

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

steakhal added inline comments. Comment at: clang/docs/analyzer/checkers.rst:69-74 +Check for dereferences of null pointers. This checker specifically does +not report null pointer dereferences for x86 and x86-64 targets when the +address space is 256 (x86 GS Segment), 257 (x86 F

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

steakhal added reviewers: Szelethus, steakhal. steakhal added a comment. tldr; static-analyzer fixits are not completely implemented. We don't even have tests for it, which is a shame. When I passed the `apply-fixits`, it modified the input source file - as I expected. Then I tried the `-analyze

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. No major issues left. Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:331 + auto *Chk = mgr.registerChecker(); + // auto *Chk = mgr.getChecker(); +

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

steakhal added a comment. In D122243#3443109 , @ASDenysPetrov wrote: > LGTM Oh I completely forgot about this stack. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122243/new/ https://reviews.llvm

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

steakhal added a comment. What happens if the import fails? Or alternatively the `InitializeImportedDecl(FromD, ToD)` fails? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123685/new/ https://reviews.llvm.org/D123685 __

[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

steakhal added inline comments. Comment at: clang/include/clang/AST/ASTImporterSharedState.h:69-75 llvm::Optional getImportDeclErrorIfAny(Decl *ToD) const { auto Pos = ImportErrors.find(ToD); if (Pos != ImportErrors.end()) return Pos->second; else

[PATCH] D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/WorkList.cpp:204-207 // Compare by number of times the location was visited first (negated // to prefer less often visited locations), then by insertion time (prefer // expanding nodes inserted soo

[PATCH] D123464: [analyzer] Clean checker options from bool to DefaultBool (NFC)

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. > A recent review emphasized the preference to use DefaultBool instead of bool > for checker options. What I wanted to highlight is that we should aim for consistency. We should

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

steakhal added a subscriber: whisperity. steakhal added a comment. In D123352#3439649 , @MosheBerman wrote: > In D123352#3439390 , @steakhal > wrote: > >> tldr; static-analyzer fixits are not completely implemen

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG63c4ca9d14ba: [analyzer] Turn missing tablegen doc entry of a checker into fatal error (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

This revision was automatically updated to reflect the committed changes. Closed by commit rGb7c988811d50: [analyzer][NFC] Introduce the checker package separator character (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122243/

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

This revision was automatically updated to reflect the committed changes. steakhal marked an inline comment as done. Closed by commit rG744e2a3e2232: [analyzer] ClangSA should tablegen doc urls refering to the main doc page (authored by steakhal). Changed prior to commit: https://reviews.llvm.o

[PATCH] D121459: [analyzer] Remove HasAlphaDocumentation tablegen enum value

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG798418982630: [analyzer] Remove HasAlphaDocumentation tablegen enum value (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

steakhal added a comment. In D123155#3433715 , @tomrittervg wrote: > Just a note - I was able to test this finally and it works for what I was > trying to do. Do you have commit access? Or we shall commit this on your behalf? If the latter, please tell

[PATCH] D123155: [analyzer] Expose Taint.h to plugins

This revision was automatically updated to reflect the committed changes. Closed by commit rG82f3ed99045d: [analyzer] Expose Taint.h to plugins (authored by tomrittervg, committed by steakhal). Changed prior to commit: https://reviews.llvm.org/D123155?vs=420616&id=423622#toc Repository: rG L

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

steakhal updated this revision to Diff 423624. steakhal edited the summary of this revision. steakhal added a comment. Herald added a subscriber: manas. Herald added a project: All. rebased; I'm still interested in this. WDYT @martong BTW we have this downstream for about two years now without any

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

steakhal reopened this revision. steakhal added a comment. This revision is now accepted and ready to land. It broke some bots. I had to revert it in minutes it landed. Yet to be investigated how did it break something. Mind that I dont have any experience with objC. Repository: rG LLVM Githu

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

steakhal added a comment. 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 implies this anyway. That being said, digging out a callexpr from a CallEvent and calling

[PATCH] D119103: [NFC][Analyzer] Use range based for loop.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Hmm, I guess you must take it by mutable reference since you push it to `LocalTUDecls`. Thanks for the patch. Do you have push permissions? Otherwise, send me your `name ` to let me commit

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

steakhal added a comment. In D119004#3299971 , @NoQ wrote: > The original `lookup()` isn't exactly precise either, it's just slightly more > precise as it has better access to path-sensitive information such as current > values of function pointers, but

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

steakhal added a comment. I like it. nits here and there; Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h:102 + /// Returns true if the CallEvent is a call to a function that matches + /// the CallDescription. CallExpr

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

steakhal added a comment. Looks great! Thanks for your hard work on this topic @Szelethus. 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

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

steakhal updated this revision to Diff 406463. steakhal added a comment. - pin target triple to (possibly) resolve the build bot failure - add obj-c test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118690/new/ https://reviews.llvm.org/D118690 Files: clang/lib/StaticAnalyzer/Fr

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

steakhal resigned from this revision. steakhal added a comment. Herald added a subscriber: steakhal. I think I don't have much to add. I still haven't changed my mind, but let's go with what the majority of people want. To make the whole stack consistent, consider mocking the variadic free functi

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG841817b1ed26: [analyzer] Prevent misuses of -analyze-function (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

steakhal reopened this revision. steakhal added a comment. This revision is now accepted and ready to land. Ah, it seems like pinning the target triple was not enough. I'm stuck at this point. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118690/new

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

steakhal added a comment. In D118690#3305086 , @NoQ wrote: > Can you post a link to a specific buildbot failure so that we could look at > it together? Sure! I received these three mails: https://lab.llvm.org/buildbot#builders/67/builds/5824 https://la

[PATCH] D118439: [scan-build] Fix deadlock at failures in libears/ear.c

steakhal added a comment. We missed the release, let's backport it to 14.0.0. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118439/new/ https://reviews.llvm.org/D118439 ___ cfe-commits mailing list cfe-c

[PATCH] D119245: [analyzer][docs][NFC] Fix some broken links and some cosmetic changes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5e0be95fce26: [analyzer][docs][NFC] Fix some broken links and some cosmetic changes (authored by steakhal). Herald added a project: clang. Herald add

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

steakhal 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. + /// NoQ wrote: > ping ==

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

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I'm content. Thanks for the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118880/new/ https://reviews.llvm.org/D118880 ___ cfe-

[PATCH] D119462: [analyzer][NFCi] Use the correct BugType in CStringChecker.

steakhal added a comment. I would recommend not touching the unrelated lines, unless you have a clear motive. Other than that I think its fair to say that this must have been a bug. Ill check the report diff tomorrow, prior accepting this. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D119509: [analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

steakhal added a comment. I dont even see why should the no-state-changed-visitor attempt to do anything with that call. It should have disarm itself when it reaches the allocation. It its not the case, we should probably consider fixing that as well in a followup. Please consider rearranging th

[PATCH] D118388: [analyzer] Restrict CallDescription fuzzy builtin matching

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGabc873694ff7: [analyzer] Restrict CallDescription fuzzy builtin matching (authored by steakhal). Herald added a project: clang. Herald added a subscr

[PATCH] D118388: [analyzer] Restrict CallDescription fuzzy builtin matching

steakhal added a comment. I decided to land it as-is. I tried to refactor the rest of the `isCLibraryFunction()` to reuse the new detection logic, but I run into issues by doing that. BTW I'm not sure if we should support anything besides the builtins defined in `clang/include/clang/Basic/Builti

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

steakhal added a comment. In D116155#3303622 , @balazske wrote: > @martong Ping > We do not have possibility currently to check all LLDB tests. I think we can > commit this and observe the buildbots for failure. I agree. Let's see if it works out. I don

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

steakhal added a comment. Well, I've just recognized that the "Build Status" of this latest diff shows that it crashes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116155/new/ https://reviews.llvm.org/D116155 ___

[PATCH] D119462: [analyzer][NFCi] Use the correct BugType in CStringChecker.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I think yes, go ahead! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119462/new/ https://reviews.llvm.org/D119462 _

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG744745ae195f: [analyzer] Add failing test case demonstrating buggy taint propagation (authored by steakhal). Herald added a project: clang. Herald ad

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGb099e1e56255: [analyzer] Fix taint propagation by remembering to the location context (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM G

[PATCH] D119129: [analyzer] Fix taint rule of fgets and setproctitle_init

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf5963bf1967: [analyzer] Fix taint rule of fgets and setproctitle_init (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D119509: [analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119509/new/ https://reviews.llvm.org/D119509 ___ cfe-commits mailing list cfe-co

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

steakhal reopened this revision. steakhal added a subscriber: simoll. steakhal added a comment. This revision is now accepted and ready to land. It seems like the `clang-ve-ninja` doesn't really want to accept any patches from me :D I hope it's not personal. Let's be friends bot, please. I'm inv

[PATCH] D116924: [clang-extdef-mapping] Allow clang-extdef-mapping tool to output customized filename for each index entry

steakhal added a comment. I'm not against the idea. However, the implementation seems to be quite complicated, compared to what it's actually doing. Can't we spare the heap allocation somehow? It doesn't seem necessary to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal added a subscriber: NoQ. steakhal added a comment. Thank you @OikawaKirie for the thorough investigation and explanation, even annotations and examples. I really appreciate it. However, you already surpassed my knowledge regarding the frontend, cc1, and other driver magic transformation

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal added a reviewer: keith. steakhal added a subscriber: keith. steakhal added a comment. Hi @keith, I've seen you commented on a clang driver-related issue: "Why does march=native not work on Apple M1?" You mig

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

This revision was automatically updated to reflect the committed changes. Closed by commit rG4642c640f4ef: [analyzer] Remove breaking RUN line in the test case (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D118690?vs=406845&id=409175#toc Repository: rG LLVM Github

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

steakhal added a comment. I've decided to remove the offending RUN lines. It's less than ideal, but TBH I think it doesn't really matter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118690/new/ https://reviews.llvm.org/D118690 __

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