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

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

[PATCH] D75740: [ASTImporter] Corrected import of repeated friend declarations.

martong added a comment. Ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75740/new/ https://reviews.llvm.org/D75740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D83006: [ASTImporter] Add unittest case for friend decl import

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks, looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83006/new/ https://reviews.llvm.org/D83006 __

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGdb4d5f7048a2: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

[PATCH] D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. In D83009#2127106 , @vabridgers wrote: > I cherry picked this change and retried the case that was failing. This > change addresses the issue I was s

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

martong abandoned this revision. martong added a comment. Abandoning for a better fix in D83009 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82940/new/ https://reviews.llvm.org/D82940 __

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

martong added a comment. In D82940#2125183 , @riccibruno wrote: > > I'm not certain I understand, but I'm removing my LG while we figure it > > out. Can you expound a bit further? > > Take a look at the AST dump of a small modification of the reduced test

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

martong added a comment. In case of the reproducer the problem is that we clear the switch ids in GetExternalDeclStmt. But we read the SwitchCase with the same ID twice from the same call of `GetExternalDeclStmt` (**//bold-italic//** in the below call stacks): - 1) clang::ASTReader::RecordSwi

[PATCH] D81787: [clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr::getBody

martong added a comment. This patch causes a regression (crash) during the deserialization of certain functions. Please see the details and review the supposed fix here: https://reviews.llvm.org/D82940 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

martong created this revision. martong added reviewers: vabridgers, riccibruno, aaron.ballman. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a project: clang. https://github.com/llvm/llvm-project/commit/05843dc6ab97a00cbde7aa4f08bf3692eb83109d introduc

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

martong accepted this revision. martong added a comment. Ok, I checked and it seems the parent map context is used only by the AST matchers, and this is okay because it looks like there is not any storing or caching of the parent map objects (the type is never copied). Repository: rG LLVM Gi

[PATCH] D82882: [ASTImporter] Fix AST import crash for a friend decl

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks Vince, looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82882/new/ https://reviews.llvm.org/D82882 __

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

martong added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:723 + // Parent map is invalidated after changing the AST. + ToDecl->getASTContext().getParentMapContext().clear(); + > ... the TU is modified by getCrossTUDefinition the pare

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

martong accepted this revision. martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:324 "for import when inlining functions during CTU analysis. " "Lowering this threshold can alleviate the memor

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

martong added a comment. > We use NoEvalCall in all functions in this patch I just now realized that it is actually not true, sorry about that. So, seems like `fnmatch`, `strcasecmp`, `strncasecmp` are `EvalCallAsPure`. I could skip them from this patch, if you guys consider this as dangerous a

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

martong added a comment. Thanks for the review guys! I understand there are concerns about `evalCall`, so I am trying to dissolve those concerns below. :) In D82288#2111206 , @Szelethus wrote: > I see a lot of `NoEvalCall`, but I wonder whether modifying

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

martong updated this revision to Diff 273018. martong marked 4 inline comments as done. martong added a comment. - Add 'Hide' and 'InAlpha', move Off_tMax inside the brace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82288/new/ https://reviews.llv

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

martong marked 8 inline comments as done. martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:352-357 CmdLineOption, Szelethus wrote: > D78118#inline-723679 Please mark this `Hidden`. Ok, it missed my attention

[PATCH] D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1123-1124 + "abs", Summary(ArgTypes{IntTy}, RetType{IntTy}, EvalCallAsPure) + .Case({ArgumentCondition(0, Wi

[PATCH] D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc

martong added a comment. In D79432#2022207 , @xazax.hun wrote: > I think testing summaries this way can be really hard to manage in the future. > I see two possible ways forward to make this easier: > a) Make something like https://reviews.llvm.org/D7811

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

martong abandoned this revision. martong marked an inline comment as done. martong added a comment. At the current state we add a summary individually for each name, that makes it easier to modify a summary for each function without effecting other functions. I'll add this functionality in the f

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

martong marked 2 inline comments as done. martong added a comment. In D79425#2022245 , @xazax.hun wrote: > I'd prefer to have this functionality committed together its first actual use > with tests. Okay, at the current state we add a summary individual

[PATCH] D79430: [analyzer] StdLibraryFunctionsChecker: Add LazyRanges to support type dependent Max

martong abandoned this revision. martong added a comment. Since D80016 we are able to look up types in the TU, so we can get the maximum value from the found type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7943

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

martong abandoned this revision. martong added a comment. Since D80016 we are able to look up types in the TU, so this patch is no longer needed. We'd like to be as precise with the function signatures as possible. Repository: rG LLVM Github Monorepo CHANGE

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

martong abandoned this revision. martong added a comment. This patch is no longer needed because I am going to upstream the POSIX functions in groups, otherwise the patch would be hideously large. The groups are: file handling, networking, pthread_, mq_, dbm_, regex related, sched_, time, signa

[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

martong created this revision. martong added reviewers: Szelethus, balazske, NoQ, xazax.hun, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald ad

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

martong added a comment. In D77658#2065174 , @MaskRay wrote: > `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` > `Subscribers:` `Tags:` and the text `Summary:` with the following script: > > arcfilter () { > arc amend >

[PATCH] D81745: [analyzer][MallocChecker] PR46253: Correctly recognize standard realloc

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1071-1072 bool ShouldFreeOnFail) const { + // HACK: CallDescription currently recognizes non-standard realloc functions + // as standard because it d

[PATCH] D81916: [analyzer] Fix StdLibraryFunctionsChecker crash on macOS

martong added a comment. In D81916#2095106 , @NoQ wrote: > Thanks! > > @martong, thoughts on this? :) > > Also i guess in this test we're unable to obtain the value for `EOF`, what > would be a good fixme-test to demonstrate that? I accept this as a ver

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

martong added a comment. In D72705#2041779 , @balazske wrote: > I can make comparisons if more functions are added to the checker, `fputs` > only is not sufficient. Yes, you cannot measure just with fputs, similar thing happened to me with StdCLibraryF

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thank you Kristof for working on this. Looks good to me as it is! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80905/new/ https://reviews.llv

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong added a comment. In D80016#2063234 , @xazax.hun wrote: > A high level comment. > > Trying to match function signatures is not a unique problem! In fact, almost > all of the checks the analyzer have is trying to solve the very some problem. > One

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rG634258b80606: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:739 + Optional FileTy = lookupType("FILE", ACtx); + Optional FilePtrTy, FilePtrRestrictTy; balazske wrote: >

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG16506d789084: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints (authored by martong). Changed prior to commit: https://reviews.llvm.org/D77658?vs=267177&id=267243#toc Repository

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

This revision was automatically updated to reflect the committed changes. Closed by commit rG41928c97b6a1: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved (authored by martong). Changed prior to commit: https://reviews.llvm.org/D77148?vs=267141&id=267232#toc Rep

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

This revision was automatically updated to reflect the committed changes. Closed by commit rGbd03ef19beb8: [analyzer] ApiModeling: Add buffer size arg constraint (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ https:/

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:237 + SVal BufDynSize = + getDynamicSizeWithOffset(State, BufV, C.getSValBuilder()); + Szelethus wrote: > You can retrieve `SValBuilder` from

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

martong updated this revision to Diff 267216. martong marked 2 inline comments as done. martong added a comment. - Remove SValBuilder param Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ https://reviews.llvm.org/D77066 Files: clang/inc

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

martong added a comment. Thanks! LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80786/new/ https://reviews.llvm.org/D80786 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

martong added a comment. Just out of curiosity. In what way do you prepare to share these test? For which component are you planning to reuse this test infrastructure? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80786/new/ https://reviews.llvm.o

[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

martong added a comment. Thank you for working on this. > moving into a clang::test namespace instead of prefixing the names +1 for this. We could go even further: `clang::test::ast` or `clang::unittest::ast`. Comment at: clang/unittests/AST/Language.h:23 + +enum UnittestLan

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

martong updated this revision to Diff 267177. martong marked 8 inline comments as done. martong added a comment. - Add comments, remove auto from 'for' loops, matches -> matchesAndSet Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77658/new/ https:/

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

martong accepted this revision. martong added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75665/new/ https://reviews.llvm.org/D75665 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

martong updated this revision to Diff 267141. martong marked an inline comment as done. martong added a comment. - Rebase to D77066 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77148/new/ https://reviews.llvm.org/

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

martong updated this revision to Diff 266918. martong marked 4 inline comments as done. martong added a comment. - Rename: validate -> checkValidity, sanityCheck -> checkSpecificValidity - Rename: Summary::checkValidity -> validateByConstraints - Add more docs to the Summary class - Rename matches

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

martong marked 7 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:697 if (auto *FD = dyn_cast(D)) { - if (S.matchesSignature(FD)) { + if (S.Sign.matches(FD) && S.validat

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112 +Optional ExprEngine::retrieveFromConstructionContext( +ProgramStateRef State, const LocationContext *LCtx, baloghadamsoftware wrote: > NoQ wrote: > > balazske wr

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

martong updated this revision to Diff 266879. martong marked an inline comment as done. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77658/new/ https://reviews.llvm.org/D77658 Files: clang/lib/StaticA

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:747 + : *FilePtrTy) +: None; + balazske wrote: > martong wrote: > > balazske wrote: > > > The `Optional` can be le

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong updated this revision to Diff 266858. martong marked 2 inline comments as done. martong added a comment. - Add `if (FileTy)` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80016/new/ https://reviews.llvm.org/D80016 Files: clang/lib/Static

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong updated this revision to Diff 266834. martong marked an inline comment as done. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80016/new/ https://reviews.llvm.org/D80016 Files: clang/lib/StaticA

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added a comment. In D80366#2059765 , @martong wrote: > In this version of the patch you are using `handleConstructionContext` to > "retrieve" or read an SVal for the construction. However, it seems like that > function was designed to store value

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added a comment. In this version of the patch you are using `handleConstructionContext` to "retrieve" or read an SVal for the construction. However, it seems like that function was designed to store values in the GDM for individual construction context items. To mix a read-only function

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:28 + +Optional RetVal = Call.getReturnValueUnderConstruction(0); +assert(RetVal); How do we know that `0` is the proper block count? CHANGES

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:24 +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const { +if (!Call.getOriginExpr()) I assume this tests this call expre

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:249-250 + // cannot apply the constraint. Actually, other checkers like + // CallAndMessage should catch this situation earlier, because we call a + // func

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

martong updated this revision to Diff 266287. martong marked 4 inline comments as done. martong added a comment. - Add assert - Create the BufferSize arg constraints in a more readable way Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ ht

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy, SizeTy, Si

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy, SizeTy, Si

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:747 + : *FilePtrTy) +: None; + balazske wrote: > The `Optional` can be l

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:435 + /// If the call returns a C++ record type then the region of its return value + // can be retrieved from its construction context. + Optional getReturnValueUnder

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LG! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78099/new/ https://reviews.llvm.org/D78099 ___ cfe-commits mailing list cfe-commits@

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Looks good. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2024 + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) { +C.a

[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2132 + + assert(!isDependency(Registry, bt.getCheckerName()) && + "Some checkers depend on this one

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

martong requested changes to this revision. martong added a comment. This revision now requires changes to proceed. Found a small mistake that should be corrected, then will be good to me. Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:

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

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:194 + const ProgramPointTag *Tag = nullptr) { +// Say this 3 times fast. +State = State ? State : getState(); I like the joke,

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp:22 +const ParmVarDecl *PVD) { +for (const auto *D2: PVD->redecls()) { + const auto *PVD2 = cast(D2); I am concerned about the re

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

martong added a comment. Thanks for addressing my comments! Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:309 + /// Get the lvalue for a parameter. + Loc getLValue(const Expr *Call, unsigned Index, +const LocationContext *LC

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

martong added a comment. Hi Endre, just checked the latest update. Overall looks good to me, but found some nits. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:260 +/// that produce the AST used for analysis. +StringRef OnDemandParsingDatabase; + -

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

martong added a comment. Seems like many changes could be simplified or absolutely not needed in this patch if ParamRegion (or with a better name ParamVarRegion) was derived from VarRegion. Is there any difficulties in that derivation? Comment at: clang/include/clang/StaticA

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

martong added a comment. > Retrieving the parameter location of functions was disabled because it may > causes crashes due to the fact that functions may have multiple declarations > and without definition it is difficult to ensure that always the same > declration is used. Now parameters are s

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

martong added a comment. Wow, this is some nice work and huge effort from all the participants, it was pretty informative to read through. Thanks @baloghadamsoftware and @NoQ for working on this! > This means that we always found a Decl. However, this Decl is not stored but > retrieved always

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

martong added a comment. In D72705#2036066 , @balazske wrote: > It is a way of find some of the cases with the mentioned way of tracking what > operations are allowed after what operations with or without error check, on > a specific stream. Instead of a

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

martong created this revision. martong added reviewers: xazax.hun, NoQ, Szelethus, balazske. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a projec

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

martong planned changes to this revision. martong added a comment. With @xazax.hun we agreed that we should try our best to provide signatures for all functions. This includes two major changes: 1. Add signatures to each summary here, possibly with `Irrelevant` types. 2. Add the ability to looku

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

martong planned changes to this revision. martong added a comment. We had a separate discussion with @xazax.hun. Let me summarize the outcome. We agreed that we should try our best to provide signatures for all functions (this affects child patches, particularly the POSIX related D79433

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG7c3768495e8c: [analyzer] Improve PlacementNewChecker (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.or

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGff4492c89feb: [analyzer] StdLibraryFunctionsChecker: Add option to display loaded summaries (authored by martong). Changed prior to commit: https://reviews.llvm.org/D78118?vs=260930&id=263992#toc Repos

[PATCH] D79420: [analyzer] Make NonNullParamChecker as dependency for StdCLibraryFunctionsChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rGd061685a8304: [analyzer] Make NonNullParamChecker as dependency for… (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79420/new/ https:/

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong added a comment. Thanks, just committed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinf

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

martong added a comment. I think this is a useful concept and it can nicely complement the ongoing summary based work in `StdLibraryFunctionsChecker`. Still, in my opinion, there are more discussions and work to be done here before we get to the final implementation. Our analyzer does not reas

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

martong accepted this revision. martong added a comment. In D79330#2033990 , @Szelethus wrote: > > Variable-length array (VLA) should have a size that fits into a size_t > > value. At least if the size is queried with sizeof, but it is better (and > > mo

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

martong added a comment. In D79330#2026564 , @balazske wrote: > I do not know if it would me **much** cleaner. > > - First part: Move calculation of `ArraySize` into `checkVLA` and rename > `checkVLASize` to `checkVLAIndexSize`. > - Second part: Add the c

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

martong accepted this revision. martong added a comment. Herald added a subscriber: rnkovacs. In D79072#2026553 , @balazske wrote: > This change is relatively small and the refactoring like part (introduction > of `checkVLA` if I think correct?) is connec

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

martong added a comment. I'd split this patch into two as well. 1. [NFC] Refactoring parts 2. The actual extra additions about sizeof and typedef. WDYT? Other than that looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79072/new/ https:/

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

martong added a comment. Would it be possible to split this patch into two? 1. The refactoring part where you move code out from checkPreStmt to checkVLA. This should be an NFC. 2. Handling of the overflow. Would be much cleaner I guess. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

martong added a comment. In D79423#2023150 , @balazske wrote: > Probably something like `IsCStdLibraryFunction` can be included in the > signature, and empty signature is allowed only for functions that are in a C > standard library. Yeah, we could do

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

martong added a comment. In D79423#2022920 , @xazax.hun wrote: > In D79423#2022812 , @martong wrote: > > > I don't think that could be a concern. > > Actually, redefinition of a reserved name either in the C or in

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

martong updated this revision to Diff 262620. martong added a comment. - Rename 'empty' -> 'irrelevant' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79423/new/ https://reviews.llvm.org/D79423 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryF

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

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:263 bool matches(const FunctionDecl *FD) const; +bool empty() const { return RetTy.isNull() && ArgTys.size() == 0; } TODO rename to isIrrelevant

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

martong added a comment. In D79423#2022251 , @xazax.hun wrote: > I am a bit unsure about this change. C libraries might be consumed in C++ > projects and C++ projects might be free to overload those functions. I don't think that could be a concern. Actu

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

martong added a comment. In D79423#2022167 , @balazske wrote: > An empty signature means really that the whole signature is "irrelevant"? It > is like all types and number of arguments is irrelevant. Probably that would > be a better name, instead of `em

[PATCH] D79430: [analyzer] StdLibraryFunctionsChecker: Add LazyRanges to support type dependent Max

martong added a comment. In D79430#202 , @xazax.hun wrote: > I am a bit unsure what the purpose of these `Max` summaries are? As far as I > understand the `Max` represents the largest value for the type of the formal > parameter. Yes, that's right.

[PATCH] D79420: [analyzer] Make NonNullParamChecker as dependency for StdCLibraryFunctionsChecker

martong added a comment. In D79420#2022237 , @xazax.hun wrote: > > If a given parameter in a FunctionDecl has a nonull attribute then the > > NonNull constraint in StdCLibraryFunctionsChecker has the same effect as > > NonNullParamChecker. > > Wait, wher

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

martong added a comment. In D79433#2022353 , @balazske wrote: > The checker or at least the source file should be split somehow, adding of > summaries should be separated from the other parts, otherwise the file will > be too long. Yes, I've been think

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

martong added a comment. In D79433#2022244 , @xazax.hun wrote: > This is cool! Hi Gabor, thanks for the reviews! :) > Some questions: > > 1. Have you reported those bugs to CppCheck devs? It might be useful for us > as well, as they can also double-ch

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

martong updated this revision to Diff 262366. martong added a comment. - Fix file descriptor params to have proper range value - usleep 100 -> 99 - Fix fstat family Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79433/new/ https://reviews.ll

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

martong created this revision. martong added reviewers: Szelethus, NoQ, baloghadamsoftware, balazske, steakhal. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a projec

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