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

2020-06-30 Thread Gabor Marton via Phabricator via cfe-commits
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] D82882: [ASTImporter] Fix AST import crash for a friend decl

2020-06-30 Thread Gabor Marton via Phabricator via cfe-commits
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.

2020-06-30 Thread Gabor Marton via Phabricator via cfe-commits
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] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

2020-07-01 Thread Gabor Marton via Phabricator via cfe-commits
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] D81787: [clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr::getBody

2020-07-01 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-07-01 Thread Gabor Marton via Phabricator via cfe-commits
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] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

2020-04-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:6821 + E->getFPFeatures(Importer.getFromContext()), + importChecked(Err, ToComputationLHSType), + importChecked(Err, ToComputationResultType)); This introduced an assertion fa

[PATCH] D76594: [clang][AST] Support AST files larger than 512M

2020-04-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. I am not sure, but maybe this patch causes an undefined behavior? http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/40472/steps/check-clang%20ubsan/logs/stdio /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Serialization/ASTReader.cpp:632

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-20 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D78457#1991288 , @xazax.hun wrote: > > As turned out we don't even need a BugReporterVisitor for doing the > > crosscheck. > > We should simply use the constraints of the ErrorNode since that has all > > the necessary informa

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

2020-04-20 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189 + if (const MemRegion *MRegion = PlaceVal.getAsRegion()) { +if (const ElementRegion *TheElementRegion = +MRegion->getAs()) { f00kat wrote: > f0

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

2020-04-20 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Thanks for the test Shafik, that is pretty self explanatory! Comment at: clang/lib/AST/ASTImporter.cpp:8161 if (auto *ToRecord = dyn_cast(ToDC)) { auto *FromRecord = cast(FromDC); if (ToRecord->isCompleteDefinition()) { Wha

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

2020-04-20 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks Endre for the docs! I checked the CI job also, seemed okay, so, I think we are ready! Nice work! Let's do the commit! Comment at: clang/docs/analyzer/user-docs/Cros

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

2020-04-21 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:56 + load_threshold_reached, + ambiguous_compile_commands_database }; xazax.hun wrote: > The two enum values refer to compilation database and compile command > data

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

2020-04-22 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. LGTM! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78000/new/ https://reviews.llvm.org/D78000 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

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

2020-04-22 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227 +/// Identifier. +virtual LoadResultTy load(StringRef Identifier) = 0; +virtual ~ASTLoader() = default; xazax.hun wrote: > martong wrote: > > xazax.hun w

[PATCH] D78638: [analyzer] Consider array subscripts to be interesting lvalues

2020-04-22 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/test/Analysis/PR53280338.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text -verify %s + AFAIK rdar is not accessible outside Apple. So, for the rest of the open source developers any

[PATCH] D77641: [analyzer] StdLibraryFunctionsChecker: Associate summaries to FunctionDecls

2020-04-22 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77641/new/ https://reviews.llvm.org/D77641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. I like it! Nice work! I have some minor comments. Comment at: clang/unittests/StaticAnalyzer/CheckerRegistration.h:81 +template +bool runCheckerOnCodeWithArgs(const std::string &Code, std::string &Diags, + const std::vector

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:55 + + bool reachedWithNoContradiction(const CallEvent &, CheckerContext &C, + ProgramStateRef State) const { It

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:65 + + bool reportIfCanBeZero(const CallEvent &Call, CheckerContext &C, + ProgramStateRef State) const { xazax.hun wrote:

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2822 PathSensitiveBugReport &BR) { // Collect new constraints + addConstraints(EndPathNode, /*OnlyForNewSymbols=*/false); Probably you wanted to use `hasCont

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2871 + // Overwrite the associated constraint of the Symbol. + Constraints = CF.remove(Constraints, Sym); Constraints = CF.add(Constraints, Sym, C.second); --

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

2020-04-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:261 + + if (const VarRegion *TheVarRegion = BaseRegion->getAs()) { +const VarDecl *TheVarDecl = TheVarRegion->getDecl(); Perhaps you could call instead `checkV

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-24 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. Other than Whispy's nits, LGTM! Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:24 + do \ +if (!LLVM_WIT

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-24 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2869 + Constraints = CF.add(Constraints, Sym, C.second); +} else if (!OnlyForNewSymbols) { + // Overwrite the associated constraint of the Symbol. If th

[PATCH] D78457: [analyzer][Z3-refutation] Fix a refutation BugReporterVisitor bug

2020-04-27 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. LGTM! Thanks for addressing the comments! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/ https://reviews.llvm.org/D78457 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D77641: [analyzer] StdLibraryFunctionsChecker: Associate summaries to FunctionDecls

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG62e747f61729: [analyzer] StdLibraryFunctionsChecker: Associate summaries to FunctionDecls (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D77641: [analyzer] StdLibraryFunctionsChecker: Associate summaries to FunctionDecls

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D77641#2004273 , @Szelethus wrote: > I think you can go ahead and commit. You seem to have a firm grasp on this > project, I believe your experience with ASTImporter has more then prepared > you for digging functions out of th

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 260647. martong added a comment. - Rebase to master - Add the option to the lit test analyzer-config.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78118/new/ https://reviews.llvm.org/D78118 Files: clang/in

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 260651. martong added a comment. - Better description for the option in Checkers.td Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78118/new/ https://reviews.llvm.org/D78118 Files: clang/include/clang/StaticA

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D78118#1981592 , @Szelethus wrote: > This is a great idea, but the tests just seem to, well, test the new > functionality? > > On a different issue, take a look at how certain help related frontend flags > (not `-analyzer-conf

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong marked an inline comment as done. martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:299 + "DisplayLoadedSummaries", + "If set to true, the checker displays all loaded summaries.", +

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D78118#1988464 , @balazske wrote: > The new option will simply list the (found) functions that have a summary > based check enabled. (The term "Loaded" may be misleading in the current > implementation, somebody can think if i

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 260687. martong added a comment. - Handle and test the display of signatures Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78118/new/ https://reviews.llvm.org/D78118 Files: clang/include/clang/StaticAnalyzer

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D78118#2007943 , @martong wrote: > In D78118#1988464 , @balazske wrote: > > > The new option will simply list the (found) functions that have a summary > > based check enabled. (The term

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

2020-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. > This is a great idea, but the tests just seem to, well, test the new > functionality? I updated the patch so now we have a new RUN line that checks with FileCheck. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78118/new/

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

2020-04-29 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 4 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:651 +// Get the declaration of a function proto as written in the source file. +StringRef ToString(const FunctionDecl *FD)

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

2020-04-29 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 260930. martong marked 2 inline comments as done. martong added a comment. - Use Decl::print and 'for: ' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78118/new/ https://reviews.llvm.org/D78118 Files: clang/

[PATCH] D76287: [analysis][analyzer] Introduce the skeleton of a reaching definitions calculator

2020-04-30 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/Analysis/ReachingDefinitions.cpp:269 + for (const CFGBlock *Succ : N->succs()) +Worklist.push_back(Succ); +} I understand that this is the worklist algorithm uplifted from Wikipedia. But how d

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

2020-05-04 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. > ... This draws a pattern that we should recursively descend down to the top > most base region. I.e. the different check*RegionAlign methods should call > into each other until we reach the top level base region. > The observation her

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

2020-05-04 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/test/Analysis/vla.c:107 + if (x == BIGINDEX) { +size_t s = sizeof(int[x][x][x][x]); // expected-warning{{Declared variable-length array (VLA) has too large size}} +return s; I think we could make the arit

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision. martong added reviewers: NoQ, Szelethus, balazske, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity, mgorny. H

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 283254. martong added a comment. - Remove dummy test file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85319/new/ https://reviews.llvm.org/D85319 Files: clang/include/clang/CodeGen/CodeGenMangling.h clang

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/IRContext.h:39-41 + // Get the LLVM code for a function. We use the complete versions of the + // constructors and desctructors in this overload. Use the

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done. martong added a comment. In D85319#2196648 , @whisperity wrote: > What will happen with the ability to analyse a translation unit whose target > was not part of `LLVM_TARGETS_TO_BUILD` of the current `clang` binar

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D85319#2197104 , @rjmccall wrote: > This seems a huge architectural change that we need to talk about. Yes, sure. I am open to and welcome any discussions and suggestions. This patch is just a prototype to demonstrate the usab

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:720 + F->getAttributes().hasFnAttribute(llvm::Attribute::ReadOnly)) +return; + NoQ wrote: > Before i forget: you still need to conjure th

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

2020-08-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:30 + // markers which are used by some LLVM analysis (e.g. AliasAnalysis). + CGO.OptimizationLevel = 2; // -O2 + TODO overwrite ALL optimization related config. Ar

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

2020-08-11 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 284738. martong added a comment. - Use custom, CSA specific pipeline - Rename runOptimizerPipeline to runPipeline - Add unittest for IRContext - Add BuildCodeGen to CSA's FrontendAction.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

2020-08-11 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:30 + // markers which are used by some LLVM analysis (e.g. AliasAnalysis). + CGO.OptimizationLevel = 2; // -O2 + xazax.hun

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

2020-08-11 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 284760. martong added a comment. - Use /// comments in IRContext.h - Conjure the return value even if the funciton is pure - Set all CodeGen options to the default (except the opt level) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D85093: [analyzer] StdLibraryFunctionsChecker: Add support for new functions

2020-08-12 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Thanks for your work! Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:190 void test_notnull_symbolic2(FILE *fp, int *buf) { - if (!buf) //

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

2020-08-12 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D72705#2210255 , @balazske wrote: > More results in CodeChecker: emacs_errorreturn >

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

2020-08-12 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D72705#2199333 , @balazske wrote: > Test results for tmux are available here >

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

2020-08-12 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:270-272 +default: + llvm_unreachable("Invalid template argument for isa<> or " + "isa_and_nonnull<>"); baloghadamsoftware wrote: > NoQ

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

2020-08-12 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

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

2020-08-13 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 285289. martong marked 2 inline comments as done. martong added a comment. - Handle minimum buffer sizes - Fix copy paste error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84248/new/ https://reviews.llvm.org/

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

2020-08-13 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:2040 + +if (ConstStructTimevalPtrTy && StructTimespecPtrTy) + // int nanosleep(const struct timespec *rqtp, struct timespec *rmtp); balazske wrot

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

2020-08-13 Thread Gabor Marton via Phabricator via cfe-commits
martong added reviewers: NoQ, vsavchenko. martong added subscribers: vsavchenko, NoQ. martong added a comment. @NoQ, @vsavchenko My apologies, somehow I forgot to add you as reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https:

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

2020-08-13 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. @NoQ, @vsavchenko My apologies, somehow I forgot to add you as reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84248/new/ https://reviews.llvm.org/D84248 ___ cfe-commits

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

2020-08-14 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D84415#2215780 , @vsavchenko wrote: > Off-topic: I really think that we should have some sort of DSL for that kind > of stuff. In a sense the API provided in the Checker itself is an (embedded) DSL. Or do you think about bein

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

2020-08-14 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 285626. martong marked an inline comment as done. martong added a comment. - Use the shorter 'getPointerTy' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https://reviews.llvm.org/D84415 Files: cla

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

2020-08-14 Thread Gabor Marton via Phabricator via cfe-commits
martong marked an inline comment as done. martong added a comment. In D84415#2218084 , @balazske wrote: > Even some macro-like construct can improve readability. The current way of > adding functions is source of copy-paste errors because more things are

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

2020-08-14 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 285682. martong added a comment. - Use Optionals through the Signature Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https://reviews.llvm.org/D84415 Files: clang/lib/StaticAnalyzer/Checkers/StdLib

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

2020-08-14 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Now I've update the patch to remove all the `if`s. And the optional types are now implicitly handled, so no such mistakes can be done that I had done previously. However, the patch is getting to affect the core of this Checker, so, perhaps I should split this into two p

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

2020-05-04 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 7 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:234-236 + // The signature of a function we want to describe with a summary. This is a + // concessive signature, meaning there

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

2020-05-04 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 261855. martong marked 2 inline comments as done. martong added a comment. - Better encapsulate the data in a 'Summary' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77658/new/ https://reviews.llvm.org/D77658

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ https://reviews.llvm.org/D77066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 262095. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ https://reviews.llvm.org/D77066 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Dynam

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 262098. martong marked 2 inline comments as done. martong added a comment. - Rebase to latest parent patch - Create BufSize constraints in a more descriptive way Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D771

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:229-234 BufferSizeConstraint(ArgNo BufArgN, ArgNo SizeArgN) : ValueConstraint(BufArgN), SizeArgN(SizeArgN) {} +

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-05 Thread Gabor Marton via Phabricator via cfe-commits
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

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

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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] D79420: [analyzer] Make NonNullParamChecker as dependency for StdCLibraryFunctionsChecker

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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] D79430: [analyzer] StdLibraryFunctionsChecker: Add LazyRanges to support type dependent Max

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-05-06 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-05-07 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-05-07 Thread Gabor Marton via Phabricator via cfe-commits
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

2020-05-07 Thread Gabor Marton via Phabricator via cfe-commits
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] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-07 Thread Gabor Marton via Phabricator via cfe-commits
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] D79072: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

2020-05-07 Thread Gabor Marton via Phabricator via cfe-commits
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] D79072: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

2020-05-13 Thread Gabor Marton via Phabricator via cfe-commits
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] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Gabor Marton via Phabricator via cfe-commits
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] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Gabor Marton via Phabricator via cfe-commits
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] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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] D76996: [analyzer] Improve PlacementNewChecker

2020-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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] D79420: [analyzer] Make NonNullParamChecker as dependency for StdCLibraryFunctionsChecker

2020-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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] D78118: [analyzer] StdLibraryFunctionsChecker: Add option to display loaded summaries

2020-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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] D76996: [analyzer] Improve PlacementNewChecker

2020-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

2020-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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

<    4   5   6   7   8   9   10   11   12   13   >