[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

martong added a comment. Thanks for you patience Denys! Finally I had some time for the review. Nice work! Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:242 +/// +/// This function optimized for each of the six cast cases:

[PATCH] D107636: [analyzer][solver] Compute adjustment for unsupported symbols as well

martong requested changes to this revision. martong added a comment. This revision now requires changes to proceed. I am not sure if this patch makes sense at all because the adjustment handling logic is restricted to handle SymInt expressions only. Comment at: clang/lib/Stati

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

martong created this revision. martong added reviewers: balazske, steakhal. Herald added subscribers: gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. martong requested review of this revision. Herald added a p

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:167 + // Let CTU run as many steps we had in the single TU run. + // However, we need at least some minimal value to pass those lit tests that + // report a bug only in the CTU mode. --

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

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks, LGTM! With minor revisions. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:749 +// E.g. char{1,3,5,127} -> uint{1,3,5,127} +// Interrupt the f

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added a subscriber: vabridgers. martong added a comment. First of all, thanks Denys for working on this, nice work! Here are my concerns and remarks. I think this fixed set of types in `NominalTypeList` is too rigid. I don't like the fact that we have to iterate over all four types when

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

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

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

martong added a comment. I'd like to have a guarantee that if `Opts.ShouldSupportSymbolicIntegerCasts` is set to `true` then the `SymboCast` is produced both for the scoped and the unscoped enums. Could you please have an additional lit test for that? At some point we'd like to make `ShouldSupp

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2280-2290 +/// Return a symbol which is the best canidate to save it in the constraint +/// map. We should correct symbol because in case of truncation cast we can +/// only reason

[PATCH] D124436: [analyzer] Fix Static Analyzer g_memdup false-positive

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Ok. LGTM. (Just a note: it looks kind of interesting that we support the GTK related Glib functions explicitly here. I suppose they are far less frequently used than the `libc` or POSIX fun

[PATCH] D124442: [analyzer] Allow exploded graph dumps in release builds

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM, thanks! For the record, I am inserting the url of the issue: https://github.com/llvm/llvm-project/issues/53873 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D124443: [analyzer] Allow CFG dumps in release builds

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 425782. martong added a comment. - Add an option to config the inlining mode during the first phase - Change the `ctu-main.c[pp]` tests to have a RUN line with this new config option that mimics the old ctu implementation's behavor. - Change the on-demand-par

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 425784. martong added a comment. - Chosing the correct baseline this time Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123773/new/ https://reviews.llvm.org/D123773 Files: clang/include/clang/CrossTU/CrossTr

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong added inline comments. Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:23 +// FIXME On-demand ctu should be tested in the very same file that we have for +// the PCH version, but with a different a different verify prefix (e.g. +// -verfiy=on-demanc-ctu) -

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added a comment. Thanks Denys for the update! This is getting really good. I have some concerns though about the `CastMap = Map`. I think we should have `CastMap = Map` instead, and we could get the `RangeSet` from the existing `ConstraintRange` mapping. By storing directly the `RangeS

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added a comment. Giving it some more thought, the `SymCastMap = Map` should be keyed as well with an equivalence class : `SymCastMap = Map`. This is the only way to use the equivalence info correctly when we process the casts. Comment at: clang/lib/StaticAnalyzer/Cor

[PATCH] D124659: [analyzer][docs] Document alpha.security.cert.pos.34c limitations

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1289-1291 + auto It = llvm::find_if(*CM, [MinBitWidth](CastMap::value_type &Item) { +return Item.first >= MinBitWidth; + }); There might be a pro

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

martong added a comment. > This avoid producing very large RHS in case when the symbol is cased to > unsigned number, and as consequence makes the value more robust in presence > of cast. Could you please elaborate on this? I understand that you'd like to simplify certain binary operations by

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 426037. martong added a comment. - Add more explanatory comments to the values of ctu-phase1-inlining - Fix typo in comments in ctu-on-demand-parsing tests - Remove stale comment, config options are desribed elsewhere Repository: rG LLVM Github Monorepo C

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

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

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

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

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:96-97 - -// If StTrue is infeasible, asserting the falseness of Cond is unnecessary -// because the existing constraints already establish this. -if (

[PATCH] D109467: [analyzer] check for std::__addressof for inner pointer checker

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

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

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

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong added a comment. Although there is a visible slowdown, the new run-times seem promising. My guess is that it is usually not more than 1-2%. F22955239: image.png Full report: F22955251: stats.html Repository: r

[PATCH] D124761: [analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

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

[PATCH] D124761: [analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

martong updated this revision to Diff 426382. martong added a comment. - Set parent revision (in the commit message b/c the phab ui 'edit related revisions' is not operational) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124761/new/ https://revi

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong updated this revision to Diff 426635. martong marked 6 inline comments as done. martong added a comment. - Update according to review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://reviews.llvm.org/D124674 Files

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:51 +ProgramStateRef StFalse = assume(State, Cond, false); +if (!StFalse) { // both infeasible + ProgramStateRef Infeasible = State->cloneAsInfeasible(); st

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added a comment. In D124674#3482765 , @steakhal wrote: > Finally, we have this! Thanks for the review. > Can we have this https://godbolt.org/z/oferc6P5Y in addition to the one you > proposed? > I find it more readable. Sure, I find it also mo

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong updated this revision to Diff 426715. martong added a comment. - Add a simpler test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://reviews.llvm.org/D124674 Files: clang/include/clang/StaticAnalyzer/Core/PathSensit

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:51 +ProgramStateRef StFalse = assume(State, Cond, false); +if (!StFalse) { // both infeasible + ProgramStateRef Infeasible = State->cloneAsInfeasible(); ma

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong marked an inline comment as done. martong added inline comments. Comment at: clang/test/Analysis/sink-infeasible.c:37-48 + /* The BASELINE passes these checks ('wrning' is used to avoid lit to match) + // The parent state is already infeasible, look at this contradictio

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:51 +ProgramStateRef StFalse = assume(State, Cond, false); +if (!StFalse) { // both infeasible + ProgramStateRef Infeasible = State

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:51 +ProgramStateRef StFalse = assume(State, Cond, false); +if (!StFalse) { // both infeasible + ProgramStateRef Infeasible = State->cloneAsInfeasible(); ma

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong updated this revision to Diff 426785. martong added a comment. - Add LLVM_UNLIKELY Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://reviews.llvm.org/D124674 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Co

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

martong added a comment. Can we have a test for this, got idea from here (https://stackoverflow.com/questions/4129961/how-is-the-size-of-a-struct-with-bit-fields-determined-measured) typedef struct { unsigned int a:1; unsigned int x:31; unsigned int c:1;

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added a comment. @NoQ Could you please take a look? This change effects the very core of the analyzer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://reviews.llvm.org/D124674

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

martong accepted this revision. martong added a comment. Herald added a subscriber: rnkovacs. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124708/new/ https://reviews.llvm.org/D124708 ___ cfe-commi

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

martong accepted this revision. martong added a comment. LGTM! Thanks Vince! Comment at: clang/test/Analysis/array-punned-region.c:38 + ff.b[0] = 3; + clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}} // Or should this be `pff + 3` ??? +}

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

martong added a comment. Very good, this is a great initiative! Also, the quality of this patch is high, docs are great, even the release notes are updated. I would accept if I were that confident in Clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D124845: StaticAnalyzer should inline overridden delete operator the same way as overridden new operator

martong added a comment. Looks promising, I am close to accept this, thanks! Comment at: clang/include/clang/Analysis/ConstructionContext.h:122-124 assert(isa(E) || isa(E) || - isa(E) || isa(E)); + isa(E) || isa(E) || + isa(E)); --

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:106-126 +// FIXME: Move ASTMatcher library. +// Note: Taken from UnusedUsingDeclsCheck. +AST_POLYMORPHIC_MATCHER_P( +forEachTemplateArgument, +AST_POLYMORPHIC_SUPPO

[PATCH] D124448: [clang-tidy] Add project-level analysis support to misc-discarded-return-value

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:132 + + auto IDIt = FunctionIDs.find(FD); + if (IDIt == FunctionIDs.end()) { Please spell out the type instead of `auto`. Comment at:

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

martong added a comment. > The patch adds two new checkers. I don't see any technical dependencies between the two, so, please split it into two independent patches. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124244/new/ https://reviews.llvm.org/D124244 ___

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

martong added a comment. Also, could you please update the `Static Analyzer` section of `clang/docs/ReleaseNotes.rst` with the new checkers and their description? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124244/new/ https://reviews.llvm.org/D124244 ___

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

martong added a comment. Maybe it is not too late to update the clang/docs/ReleaseNotes.rst? A new checker is certainly important for the users. Many thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120489/new/ https://reviews.llvm.org/D12048

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

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

[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! And please accept my apologies @v.g.vassilev for taking it months to review this small change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119675/new

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

martong added a comment. In D124774#3493868 , @balazske wrote: > The code looks correct but the diff is not OK. I think that you have separate > commits for every change (instead of changing the first with `git commit > --amend` when a new change is mad

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

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

[PATCH] D124845: StaticAnalyzer should inline overridden delete operator the same way as overridden new operator

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Thanks for the update, LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124845/new/ https://reviews.llvm.org/D124845 ___

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong marked an inline comment as done. martong added a comment. In D124674#3491710 , @NoQ wrote: > Yes, we've discussed this before, and I'm very much in favor of this change. > This is assertion removal, and the assertion has been really useful back

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong updated this revision to Diff 427594. martong marked an inline comment as done. martong added a comment. - Add explanatory comment, rename to PosteriorlyOverconstrained Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://revie

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:99 + // recognise that a parent is infeasible only *after* a new and more specific + // constraint and its negation is evaluated. + // Yeah, thx, I

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

martong marked 3 inline comments as done. martong added a comment. Herald added a project: All. Thanks for the review Denys, and sorry for the long delay with the update. I hope that this patch is going to complement nicely the rest of the cast patches. Comment at: clang/lib/S

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

martong updated this revision to Diff 427621. martong marked 2 inline comments as done. martong added a comment. - add new assert for canonical type - remove superflous assert about makeSymbolVal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117229/

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

martong updated this revision to Diff 427625. martong marked 2 inline comments as done. martong added a comment. - Fix typos in essay Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://reviews.llvm.org/D124674 Files: clang/include

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong updated this revision to Diff 427635. martong added a comment. - Rebase to parent revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124758/new/ https://reviews.llvm.org/D124758 Files: clang/include/clang/StaticAnalyzer/Core/PathSens

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong updated this revision to Diff 427636. martong marked an inline comment as done. martong added a comment. - Remove ExprInspection checker from the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124758/new/ https://reviews.llvm.org/D12475

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong added inline comments. Comment at: clang/test/Analysis/infeasible-crash.c:3 +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.unix.cstring.OutOfBounds,alpha.unix.cstring.UninitializedRead \ +// RUN: -analyzer-checker=debug.ExprInspection \ ---

[PATCH] D124761: [analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

martong updated this revision to Diff 427637. martong added a comment. - Rebase to parent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124761/new/ https://reviews.llvm.org/D124761 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Pro

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

martong marked 4 inline comments as done. martong added a comment. @xazax.hun Hey Gabor, thanks a lot for your time and effort for reviewing this patch set! (And of course thank you for you too @steakhal, but you've already seen most of it.) In D123685#3458329

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong marked 4 inline comments as done. martong added a comment. Thanks for the review Gabor, I'll update soon and hope I could answer your questions. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116 + const bool Foreign = false; // From

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

martong added a comment. In D123784#3496981 , @xazax.hun wrote: > This approach fixes the worklist for the second phase. Would it be possible > to create a wrapper that reverses the order of any worklist instead of > committing to one and hardcode that?

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

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/WorkList.cpp:216-218 // Number of inserted nodes, used to emulate DFS ordering in the priority // queue when insertions are equal. + long Counter = 0; ---

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM (given that you remove the vague and disturbing FIXME comments). Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2014-2017 + // FIXME: This is a hack, and d

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

martong added a comment. Thanks Vince for the patch! I think the implementation is correct. But I am worried that it might do sub-optimal computations on the string literals. I mean, getting the length would require O(N) steps each time, and that could be problematic with long literals, especia

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

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

[PATCH] D129737: [analyzer] Fixing SVal::getType returns Null Type for NonLoc::ConcreteInt in boolean type

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Thanks! LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129737/new/ https://reviews.llvm.org/D129737

[PATCH] D129640: [clang][ASTImporter] Improved handling of functions with auto return type.

martong added a comment. Thanks, nice work! Comment at: clang/lib/AST/ASTImporter.cpp:3237 + ParentMapContext &ParentC = DC->getParentASTContext().getParentMapContext(); + DynTypedNodeList P = ParentC.getParents(*S); + while (!P.empty()) { ===

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

martong added a comment. Very good! In D129498#3650595 , @NoQ wrote: > In D129498#3647348 , @ASDenysPetrov > wrote: > >> In D129498#3644222 , @NoQ wrote: >> >>> Maybe `c

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Took a while to accept that this is indeed an NFC, but looks good! And indeed the code is better structured this way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D104647: [analyzer] Support SVal::getType for pointer-to-member values

martong added a comment. In D104647#3643060 , @ASDenysPetrov wrote: > @vsavchenko Is this alive? I think, we could commit this and set Valery as the author (he is no longer working on CSA). Unless you find something wrong with the patch; in that case w

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

martong accepted this revision. martong added a comment. In D129678#3652859 , @ASDenysPetrov wrote: > Fixed a typo that caused `constraint_manager_negate.c` and `unary-sym-expr.c` > tests crashes. Good. Still LGTM. CHANGES SINCE LAST ACTION https:/

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added a comment. Thanks Denys for your continued work on this. These are very good questions that must be answered, we need exactly such thinking to implement this properly. I believe we can advance gradually. In D103096#3642681 , @ASDenysPetrov

[PATCH] D129280: [analyzer] PlacementNewChecker, properly handle array overhead (cookie)

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

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

martong added inline comments. Comment at: clang/test/Analysis/print-ranges.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s +// REQUIRES: no-z3 ASDenysPetrov wrote: > martong wrote: >

[PATCH] D130029: [analyzer][NFC] Use `SValVisitor` instead of explicit helper functions

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. In D112621#3660256 , @manas wrote: > Considering @ASDenysPetrov 's example of `LHS = [1, 2] U [8, 9]` and `RHS = > [5, 6]`, I constructed a test case as following: > > `(((u1 >= 1 && u1 <= 2) || (u1 >= 8 && u1 <= 9)) && u2 >= 5 &

[PATCH] D129640: [clang][ASTImporter] Improved handling of functions with auto return type.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3269-3273 +// Note: It is possible that T can be get as both a RecordType and a +// TemplateSpecializationType. + } + if (const auto *TST = T->getAs()) { +return llvm::count_if(TST->template_ar

[PATCH] D130091: [clang][analyzer] Added partial wide character support to CStringChecker

martong added a comment. Nice improvement and the tests are meaningful! > clang/test/Analysis/cstring.c Hadn't we have already a test file for this checker? What about `string.c` and `bstring.c`? You might have added redundant test cases in the new test file. Comment at: cla

[PATCH] D130091: [clang][analyzer] Added partial wide character support to CStringChecker

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Okay, thanks for the update. LGTM! Comment at: clang/test/Analysis/wstring.c:385 + wchar_t a[32]; + // FIXME: This should work with '

[PATCH] D129640: [clang][ASTImporter] Improved handling of functions with auto return type.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/ASTImporter.cpp:3237 + ParentMapContext &ParentC = DC->getParentASTContext().getParentMapContext(); + DynTypedNodeList P = ParentC.getParen

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong updated this revision to Diff 428055. martong marked 8 inline comments as done. martong added a comment. Herald added a reviewer: shafik. - Add explanatory comment to RuntimeDefinition::Foreign field - Changed the return value from bool to void in ctuBifurcate and in inlineCall - Add isCTU

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

martong added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116 + const bool Foreign = false; // From CTU. + martong wrote: > xazax.hun wrote: > > I feel that we use different terms for the imported declaration

[PATCH] D125225: [WIP][analyzer] Taint Notes enhancements

martong added a comment. I've checked the `StdLibraryFunctionsChecker` related changes and they are promising. Comment at: clang/test/Analysis/std-c-library-functions-taint.c:88 +clang_analyzer_dump(n + 1); // expected-warning {{(conj_$}} expected-note {{(conj_$}} +

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

This revision was automatically updated to reflect the committed changes. Closed by commit rGc4fa05f5f778: [analyzer] Indicate if a parent state is infeasible (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124674/new/ https://r

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1c1c1e25f94f: [analyzer] Implement assume in terms of assumeDual (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D124761: [analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG34ac048aef29: [analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual (authored by martong). Changed prior to commit: https://reviews

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong added a comment. `infeasible-crash.c` fails both on arm and windows. The reasion is the incompatible `memmove` declaration. I am to fix this ASAP. Armv7 error: 'warning' diagnostics seen but not expected: File /home/tcwg-buildbot/worker/clang-armv7-quick/llvm/clang/test/Analysis

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong added a comment. Hopefully this commit fixes the failure: https://github.com/llvm/llvm-project/commit/21feafaeb85aad2847db44aa2208999b166ba4a9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124758/new/ https://reviews.llvm.org/D124758 _

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

martong added a comment. In D124758#3502913 , @martong wrote: > Hopefully this commit fixes the failure: > https://github.com/llvm/llvm-project/commit/21feafaeb85aad2847db44aa2208999b166ba4a9 Yes, it fixed the test case both at clang-armv7-quick and at

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

martong added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125318: [analyzer] Add UnarySymExpr

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

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