[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

martong added inline comments. Comment at: clang/docs/ReleaseNotes.rst:269 + +- New frontend flag: ``-analyzer-werror`` to turn analyzer warnings into errors. Could you please also add: - Numerous`` ASTImporter`` related fixes and improvements which increase t

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

martong marked 4 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3452 << FoundField->getType(); - - return make_error(ImportError::NameConflict); + ConflictingDecls.push_back(FoundField); }

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

martong updated this revision to Diff 217330. martong marked 2 inline comments as done. martong added a comment. - Revert changes in VisitFieldDecl and in VisitIndirectFieldDecl - Remove test suite ConflictingDeclsWithConservativeStrategy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

martong updated this revision to Diff 217337. martong added a comment. - Apply clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692/new/ https://reviews.llvm.org/D59692 Files: clang/include/clang/AST/ASTImporter.h clang/lib/AST/AST

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

This revision was automatically updated to reflect the committed changes. Closed by commit rGf035b75d8f07: [ASTImporter] Fix name conflict handling with different strategies (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692/n

[PATCH] D59692: [ASTImporter] Fix name conflict handling with different strategies

martong added a comment. Jenkins is not green http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/743/ However, the newly failing test is `TestTargetCommand.py`, which seems to be unrelated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692

[PATCH] D66866: [ASTImporter] At import of records re-order indirect fields too.

martong added a comment. Looks good, I just have a comment about the matcher. Comment at: clang/unittests/AST/ASTImporterTest.cpp:1434 +AST_MATCHER_P(RecordDecl, hasFieldIndirectFieldOrder, std::vector, + Order) { This name sounds strange for me,

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added a comment. In D63640#1648700 , @Tyker wrote: > Sorry for the long wait. > > Changes: > > - Rebased on current master > - Duplicated test file so that it runs for both importing Thanks for addressing the issues. The ASTImporter related chang

[PATCH] D66866: [ASTImporter] At import of records re-order indirect fields too.

martong accepted this revision. martong 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/D66866/new/ https://reviews.llvm.org/D66866

[PATCH] D66933: [ASTImporter] Propagate errors during import of overridden methods.

martong accepted this revision. martong added a reviewer: a_sidorin. martong added a comment. This revision is now accepted and ready to land. LGTM, other than a few comments. Comment at: clang/lib/AST/ASTImporter.cpp:7809 -void ASTNodeImporter::ImportOverrides(CXXMethodDecl

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong created this revision. martong added reviewers: shafik, a_sidorin, balazske. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs, mgorny. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong updated this revision to Diff 217899. martong added

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong updated this revision to Diff 217899. martong added a comment. - Fix copy error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66951/new/ https://reviews.llvm.org/D66951 Files: clang/unittests/AST/ASTImporterODRStrategiesTest.cpp clang/

[PATCH] D66348: [ASTImporter] Do not look up lambda classes

This revision was automatically updated to reflect the committed changes. Closed by commit rL370461: [ASTImporter] Do not look up lambda classes (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

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

[PATCH] D66336: [ASTImporter] Add development internals docs

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

[PATCH] D66999: [ASTImporter][NFC] Add comments to code where we cannot use HandleNameConflict

martong created this revision. martong added reviewers: shafik, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. This is the continuation of https://reviews.llvm.org/D59692 where we started

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong updated this revision to Diff 218625. martong added a comment. - Fix wrong file comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66951/new/ https://reviews.llvm.org/D66951 Files: clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong marked 6 inline comments as done. martong added inline comments. Comment at: clang/unittests/AST/ASTImporterODRStrategiesTest.cpp:36 + static constexpr auto *Definition = "void X(int a) {}"; + static constexpr auto *ConflictingDefinition = "void X(double a) {}"; + Bind

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

martong marked 3 inline comments as done. martong added a comment. In D66951#1660886 , @balazske wrote: > OK > Probably the `ClassTemplateSpec` can not be handled in liberal way because > the AST data structure for template specializations do not allow m

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91 + SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State); + const auto SizeOfTargetCI = SizeOfTarget.getAs(); + if (!SizeOfTargetCI) xazax.hun wrote: > Here

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong updated this revision to Diff 237059. martong marked 11 inline comments as done. martong added a comment. - Add documentation to checkers.rst - Pass CheckerContext as last param - Use BugType and default member initializer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong added a comment. In D71612#1790045 , @NoQ wrote: > I wonder if this checker will find any misuses of placement into > `llvm::TrailingObjects`. I've evaluated this checker on LLVM/Clang source and it did not find any results, though there are ma

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong updated this revision to Diff 237290. martong added a comment. - Declare ElementCountNL in if stmt - Use makeArrayIndex instead makeIntVal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71612/new/ https://reviews.llvm.org/D71612 Files: cl

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong marked 2 inline comments as done. martong added a comment. > Can we enable the check by default then? What pieces are missing? Like, the > symbolic extent/offset case is not a must. I think we have everything except > maybe some deeper testing. I'd rather spend some time on the above exe

[PATCH] D71612: [analyzer] Add PlacementNewChecker

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

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong added a comment. In D71612#1814225 , @NoQ wrote: > In D71612#1813937 , @martong wrote: > > > I am going to execute the analysis on LLVM/Clang as you suggested, so then > > we'll see what the experiment will

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong added a comment. Related commit that moves to alpha: https://github.com/llvm/llvm-project/commit/13ec473b9d4bd4f7a558272932b7c0806171c666 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71612/new/ https://reviews.llvm.org/D71612 __

[PATCH] D71612: [analyzer] Add PlacementNewChecker

martong added a comment. In D71612#1812476 , @NoQ wrote: > In D71612#1812036 , @martong wrote: > > > In D71612#1790045 , @NoQ wrote: > > > > > I wonder if this checker will f

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8043 + + Error VisitFunctionTypeLoc(FunctionTypeLoc From) { +auto To = ToL.castAs(); balazske wrote: > martong wrote: > > a_sidorin wrote: > >

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

martong updated this revision to Diff 238267. martong marked an inline comment as done. martong added a comment. - Move the setting of parameters of FunctionProtoTypeLoc to TypeLocImporter - Add tests for TypeLoc import in case of functions Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. > Unfortunately template specializations do not catch the descendants of the > class for which the template is specialized. Therefore it does not work > correcly for the descendants of Funct

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

martong added a comment. > This problem only exists if we traverse a CFGBlock in order. And Liveness in > fact does it reverse order. So a distinct pass is indeed unnecessary, we can > note the appearance of the assignment by the time we reach the variable. Should this patch depend on https://r

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

martong added inline comments. Comment at: clang/include/clang/Analysis/CFG.h:1311 + + llvm::iterator_range nodes() { return *this; } + llvm::iterator_range const_nodes() const { return *this; } martong wrote: > Do we convert `this` to `CFGBlock *Entry` here? I

[PATCH] D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order

martong added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:522 - // FIXME: we should enqueue using post order. - for (const CFGBlock *B : cfg->nodes()) { + for (const CFGBlock *B : *AC.getAnalysis()) { worklist.enqueueBlock(B); xaza

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong updated this revision to Diff 291606. martong added a comment. - Add tests to verify compatibility of the two checkers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87081/new/ https://reviews.llvm.org/D87081 Files: clang/lib/StaticAnalyz

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:352 HelpText<"Improve modeling of the C standard library functions">, - Dependencies<[CallAndMessageModeling]>, CheckerOptions<[ Umm, we should not have remo

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong added a comment. There is a blatant regression we introduced in D87240 . Actually, the test added here could have catched that regression. See https://reviews.llvm.org/D87240#inline-812062 I am putting back the modeling dependency with this patch. Repos

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

This revision was automatically updated to reflect the committed changes. Closed by commit rGa012bc4c42e4: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite (authored by martong). Changed prior to commit: https://reviews.llvm.org/D87081?vs=291606&id=291919#toc Re

[PATCH] D87683: [clang-tidy] Crash fix for bugprone-misplaced-pointer-arithmetic-in-alloc

martong added a comment. In D87683#2274663 , @baloghadamsoftware wrote: > In D87683#2274569 , @njames93 wrote: > >> Please fix the test case first, can't call `operator new(unsigned long, >> void*)` with an argume

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

martong added a comment. > Actually, the test added here could have catched that regression. Correction: It is the new BufferSize argument constraint in `fread`'s summary and the existing test `test_fread_uninitialized` in `std-c-library-functions.c` that could have catched that regression. (No

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

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

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:694 // execution continues on a code whose behaviour is undefined. assert(SuccessSt); NewState = SuccessSt; This is where we crashed b

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. In D86660#2278025 , @shafik wrote: > We are going to move forward with this approach (after dealing with the > multi-dimensional array case) temporar

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

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

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong updated this revision to Diff 292550. martong added a comment. - apply -> assume, Add isApplicable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87785/new/ https://reviews.llvm.org/D87785 Files: clang/lib/StaticAnalyzer/Checkers/StdLibra

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong added a comment. Alright, I refactored the change a bit. A Constraint::assume works similarly to an engine DualAssume. Plus I added `isApplicable` to check if it is meaningful to call the assume at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

martong added a subscriber: steakhal. martong added a comment. Hi Valery, Together with @steakhal we have found a serious issue. The below code crashes if you compile with `-DEXPENSIVE_CHECKS`. The analyzer goes on an unfeasible path, the State has a contradiction. We think that `getRange(Sym("a

[PATCH] D87785: [analyzer][StdLibraryFunctionsChecker] Fix a BufferSize constraint crash

martong abandoned this revision. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:694 // execution continues on a code whose behaviour is undefined. assert(SuccessSt); NewState = SuccessSt; --

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

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

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

martong added a comment. We came up with a quick fix, let us know what you think: https://reviews.llvm.org/D88019 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82445/new/ https://reviews.llvm.org/D82445

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong added a comment. @steakhal, thank you for your time and huge effort in debugging this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88019/new/ https://reviews.llvm.org/D88019 ___ cfe-commits mai

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong added a comment. Thanks for the quick review! I updated according to your suggestion, so we avoid the same pattern. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1350-1371 ProgramStateRef trackEQ(ProgramStateRef State, SymbolRef Sym,

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

martong updated this revision to Diff 293168. martong marked an inline comment as done. martong added a comment. - Avoid same pattern when checking whether the assumption is infeasible Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88019/new/ https:

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0c4f91f84b2e: [analyzer][solver] Fix issue with symbol non-equality tracking (authored by martong). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

martong added a reviewer: vsavchenko. martong added a comment. Adding Valeriy as a reviewer. He's been working with the solver recently, so his insights might be really useful here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77792/new/ https://reviews.llvm.org/D77792 __

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

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

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

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

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

martong added a comment. Thanks for the review! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88100/new/ https://reviews.llvm.org/D88100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

martong added a comment. In D88092#2289312 , @Szelethus wrote: > A joy of reviewing C++ code is that you get to marvel in all the great things > the language has, without having to pull fistfuls of hair out to get get to > that point. These patches are a

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

This revision was automatically updated to reflect the committed changes. Closed by commit rGd63a945a1304: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D880

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG11d2e63ab006: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries (authored by martong). Repository: rG LLVM Github M

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

martong added a comment. Beware, Phabricator ruins the visual experience of this nice analysis. E.g `//char ***//` is visible as an italic `char *`. > I think we should have a symbolic cast back to the static type before doing > anything with the SVal (iff the BaseKind differs). > If we do this

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

martong added a comment. In D86295#2232005 , @steakhal wrote: > In D86295#2231760 , @NoQ wrote: > >> I mean, like, you can measure the entire process with `time` or something >> like that. I believe @vsavchenko's d

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

martong added a comment. In D86295#2233068 , @martong wrote: > In D86295#2232005 , @steakhal wrote: > >> In D86295#2231760 , @NoQ wrote: >> >>> I mean, like, you can measure

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:198 /// SymbolMetadata - Represents path-dependent metadata about a specific region. /// Metadata symbols remain live as long as they are marked as in use before

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

martong added a comment. I can feel your pain. > The fundamental problem is, we simply can't ask Preprocessor what a macro > expands into without hacking really hard. Can you summarize what is the exact problem (or give a link to a discussion, etc)? Is it an architectural problem in Clang itse

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

martong updated this revision to Diff 287409. martong marked 4 inline comments as done. martong added a comment. - Remove private ctor of Signature - Assert a valid signature in Signature::matces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/n

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

martong added a comment. Thanks @balazske for your comments, you always make an assiduous review! Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:350 + } else { +*this = Signature(Args, *RetTy); } balazske wrote:

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

martong planned changes to this revision. martong added a comment. Planning to rebase this on a dependent patch that uses an API with Optionals. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84415/new/ https://reviews.llvm.org/D84415 _

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

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882 + const QualType SizePtrTy = getPointerTy(SizeTy); + const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy); balazske wrote: > martong wrote:

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker] Use Optionals throughout the summary API

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

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

martong updated this revision to Diff 287640. martong added a comment. - Rebase to parent patch 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/StdLibraryFunctions

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

martong added inline comments. Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:213 compile said TU are given in YAML format referred to as `invocation list`, and must be passed as an analyer-config argument. The index, which maps function USR names to sourc

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

martong added inline comments. Comment at: clang/docs/ReleaseNotes.rst:471 + +- Improve the pre- and post condition modeling of several hundred more standard + C functions. Umm, this is still an alpha command line option, plus we improved only the pre-condition

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker] Use Optionals throughout the summary API

martong added inline comments. Comment at: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c:14 +// declarations in a way that the summary is not added to the map. We expect no +// crashes (i.e. no optionals should be 'dereferenced') and no output. + Pro

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

martong added inline comments. Comment at: clang/docs/ReleaseNotes.rst:490-491 + +- Added :ref:`on-demand parsing ` capability to cross translation + unit analysis. + whisperity wrote: > What's the proper way of naming this feature, @martong @dkrupp @xazax.hun?

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong updated this revision to Diff 287680. martong added a comment. - Use FileCheck in the lit test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86531/new/ https://reviews.llvm.org/D86531 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFu

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:851 + } getPointerTy(ACtx); + class { + public: balazske wrote: > Why has this class different layout than `Ge

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

martong added a comment. In D86465#2238379 , @vsavchenko wrote: > Here are some benchmarking results. > In docker: > F12777445: tiny.png > F12777444: small.png > Natively on my Mac

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! (At least those changes that involved me, perhaps it is better to wait for more LGs.) Comment at: clang/docs/ReleaseNotes.rst:471 + +- Improve the pre- and post cond

[PATCH] D86691: [analyzer] Fix wrong parameter name in printFormattedEntry

martong accepted this revision. martong added a comment. In D86691#2241604 , @nullptr.cpp wrote: > In D86691#2241401 , @Szelethus wrote: > >> Nice, thank you! Did you stumble across this, or found it with a tool? >

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1737 // If we are in the process of ImportDefinition(...) for a RecordDecl we // want to make sure that we are also completing each FieldDecl. There `ImportDefinition(...)` here

[PATCH] D86691: [analyzer] Fix wrong parameter name in printFormattedEntry

martong added a comment. In D86691#2242004 , @nullptr.cpp wrote: > @Szelethus @martong > Sorry to interrupt, I don't have commit access, can you help commit this and > D86334 ? > Yang Fan > Thanks! Yep, no problem, I'll

[PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRecordDecl(); + ToRecordDecl = To

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:896-898 +RawLexer.reset(new Lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, + MB->getBufferStart(), MacroNameTokenPos, +

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

martong added a comment. In D86135#2235473 , @Szelethus wrote: > In D86135#2233611 , @martong wrote: > >>> The fundamental problem is, we simply can't ask Preprocessor what a macro >>> expands into without hacking

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

martong added a comment. > For any other loops, in order to know whether we should analyze another > iteration, among other things, we evaluate it's condition. Which is a problem > for ObjCForCollectionStmt, because it simply doesn't have one Shouldn't we try to fix the ObjCForCollectionStmt in

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

martong added a comment. > Another solution for the problem is if the system calls are modeled in a way > that there is always a state split between error end non-error (we will have > a path where it is known that the specific variable can be only (for example) > NULL and this can be detected

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

martong added a comment. In D72705#2238487 , @Szelethus wrote: > I debated this >

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong marked 3 inline comments as done. martong added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86531/new/ https://reviews.llvm.org/D86531 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

martong added a comment. In D86736#2247035 , @Szelethus wrote: > In D86736#2244365 , @martong wrote: > >>> For any other loops, in order to know whether we should analyze another >>> iteration, among other things,

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1006 +return IntRangeVector{std::pair{b, *e}}; + return IntRangeVector{}; +} balazske wrote: > This return of empty vector and possibilit

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong updated this revision to Diff 288946. martong added a comment. - Add assert in getRanges Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86531/new/ https://reviews.llvm.org/D86531 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunction

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong added a comment. Just realized. There maybe cases when we'd like to give XMax to an arg constraint, but the type of the arg is Y (X may be a looked up type). One example for such is getchar, where the return type is Int, but we have a range constraint {0, UCharRangeMax}. Consequently, i

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong updated this revision to Diff 288957. martong added a comment. - Revert "Add assert in getRanges" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86531/new/ https://reviews.llvm.org/D86531 Files: clang/lib/StaticAnalyzer/Checkers/StdLibrar

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong updated this revision to Diff 288958. martong added a comment. - Rebase to correct base Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86531/new/ https://reviews.llvm.org/D86531 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctions

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

martong updated this revision to Diff 289099. martong added a comment. - Support empty ranges Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86531/new/ https://reviews.llvm.org/D86531 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsCh

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

martong added a comment. > Note that we don't deal with wrapping here. Wrapping? Please elaborate. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:226 + // No unsigned symbolic value can be less then a negative constant. + if (const auto SymbolicRoot =

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

martong added inline comments. Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34 + +void symbolic_uint_and_int0(unsigned len) { + (void)a[len + 1]; // no-warning Hmm, this seems to be quite redundant with the `size_t` tests. Why is it not enough

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:52 + /// Offset: SymIntExpr{conj{n, int}, +, 12, long long} + class RawOffsetCalculator final + : public MemRegionVisitor { Since you are already deep in

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