[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

martong created this revision. martong added reviewers: riccibruno, aaron.ballman, shafik, teemperor, rsmith. Herald added subscribers: gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. martong requested review of this revision. Herald added a project: clang. Herald added

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

martong added a comment. F15719554: valgrind-output.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97849/new/ https://reviews.llvm.org/D97849 ___ cfe-commits maili

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

martong added a comment. In D97849#2600220 , @steakhal wrote: > In D97849#2600201 , @aaron.ballman > wrote: > >> This looks reasonable to me (good catch!), but is there a way for us to add >> a regression test for

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

martong added a comment. In D97849#2600300 , @teemperor wrote: > We can (and do) run LIT tests under valgrind, so if you have a test that > triggers this valgrind error then that seems like the way to go. I don't > think the test has to deterministically

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

martong updated this revision to Diff 328149. martong added a comment. Herald added a reviewer: Szelethus. Add a test case which fails if lit --vg is used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97849/new/ https://reviews.llvm.org/D97849 Fi

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2e90fc2c407b: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member (authored by martong). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

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

[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

martong accepted this revision. martong added a comment. Thanks for the test! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96586/new/ https://reviews.llvm.org/D96586 ___ cfe-commits mailing list cfe-com

[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

martong added a comment. > This causes a crash if the function is called in such case. Is the crash caused by the below assertion? QualType Expr::findBoundMemberType(const Expr *expr) { assert(expr->hasPlaceholderType(BuiltinType::BoundMember)); Comment at: clang/lib/AS

[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

martong added inline comments. Comment at: clang/lib/AST/Expr.cpp:1406 + } else if (CalleeType->isDependentType() || + CalleeType->isSpecificPlaceholderType(BuiltinType::Overload)) { +return CreateDependentType(); Can't we create a built in place

[PATCH] D98244: [analyzer] Fix StdLibraryFunctionsChecker performance issue

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Good catch! Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98244/new/ https://reviews.llvm.org/D98244 _

[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

martong added reviewers: aaron.ballman, riccibruno, dblaikie. martong added subscribers: aaron.ballman, dblaikie. martong added a comment. Hi @aaron.ballman @riccibruno @dblaikie, I am adding you as a reviewer because it seems you have great experience with Clang's AST. Could you please take a lo

[PATCH] D104550: [analyzer] Implement getType for SVal

martong added a comment. In D104550#3139239 , @stevewan wrote: > In D104550#2849582 , @vsavchenko > wrote: > >> In D104550#2849561 , >> @DavidSpickett wrote: >> >>> @vsa

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

martong added a comment. Do we have a comprehensive list of non-inclusive terms and their inclusive correspondent somewhere available? I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` -> `validation`, ... I'd assume that we go through that list, and that could give me a clue

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

martong added a comment. Thanks for the update, I am okay with the `.cpp` file, now I continue the review with the tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99797/new/ https://reviews.llvm.org/D99797 ___ cfe-commits mailing list c

[PATCH] D114006: [analyzer][NFC] Enable access to CodeGenOptions from analyzer's instances

martong accepted this revision. martong added a comment. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114006/new/ https://reviews.llvm.org/D114006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks @balazske for updating the tests, I think this addresses @shafik 's questions. LGTM! Ping @shafik Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

martong added a comment. @whisperity Thank you for pinging! This seems still feasible. Besides, this is valuable for us, as it could pass some SEI CERT test cases, notably from INT31

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

martong added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:102 } - } else if (isa(Parent)) { + } else if (isa(Parent) || isa(Parent)) { +if (!Cast->IgnoreParenImpCasts()->isEvaluatable(C.getASTContext())) { CHANGES

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

martong added a comment. In D113754#3127245 , @steakhal wrote: > Great stuff. Have you checked the coverage? Thanks for the review! So, here are the coverage results for the new test file: 1186 2 : SVal VisitIntSymExpr(const IntSymExpr *

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rGffc32efd1cd6: [Analyzer][Core] Simplify IntSym in SValBuilder (authored by martong). Changed prior to commit: https://reviews.llvm.org/D113754?vs=386800&id=388927#toc Repository: rG LLVM Github Monor

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong marked 2 inline comments as done. martong added a comment. I'm attaching the coverage of the new test file for the related change: 375 : // Constraints may have changed since the creation of a bound SVal. Check if 376 : // the values can be simplified base

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong updated this revision to Diff 389186. martong marked an inline comment as done. martong added a comment. - Add new test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113753/new/ https://reviews.llvm.org/D113753 Files: clang/lib/Stat

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 389198. martong marked 6 inline comments as done. martong added a comment. - Return explicitly with UndefinedVal - Unify test cases (return 0 -> return) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1144 + : (SVal)SVB.makeIntVal(*Const); + return SVal(); +} steakhal wrote: > Let's be explicit about it. Good poin

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added a comment. In D103317#3127318 , @steakhal wrote: > To me at least, the patch looks good. > Please post some comparative measurements to demonstrate it won't introduce > runtime regression. Sure! F20586670: stats.html

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added inline comments. Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:32 + clang_analyzer_eval(x + y * z == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(y * z == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(x == 0);

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG12887a202404: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN (authored by martong). Repository: rG LLVM Github Monorepo

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong added a comment. I'm attaching the coverage of the new test file for the related change: 375 : // Constraints may have changed since the creation of a bound SVal. Check if 376 : // the values can be simplified based on those new constraints. 377

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:6081 + // FunctionTemplateDecl objects are created, but in different order. In this + // way DeclContext of these template parameters may change relative to the + // "from" context. Because these DeclCont

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

martong accepted this revision. martong added a comment. Still LGTM! Let's land it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113118/new/ https://reviews.llvm.org/D113118 ___ cfe-commits mailing list

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1105 - // FIXME: Add support for SymExprs. return nullptr; steakhal wrote: > Where did you address this FIXME? I didn't,

[PATCH] D114256: [clang-tidy] Fix crashing altera-struct-pack-align on invalid RecordDecls

martong added a comment. What happens if this checker runs on a forward declared class? struct Foo; I'd expect the pack/alignment info is missing also in that case. Comment at: clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp:54 + // Ignore invalid decls to p

[PATCH] D114454: [NFC][AIX]Disable unstable CSA tests failing on AIX

martong edited reviewers, added: steakhal; removed: vsavchenko. martong added a comment. vsavchenko is inactive, presumably he is no longer working with CSA Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114454/new/ https://reviews.llvm.org/D114454

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

martong added a comment. > Probably this is a bug in the AST creation but currently it works this way. I don't think this is a bug, deduction guides have a convoluted implementation. See the related DeclContext issue with them when local typdefs are involved: https://reviews.llvm.org/D92209 =

[PATCH] D114256: [clang-tidy] Fix crashing altera-struct-pack-align on invalid RecordDecls

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114256/new/ https://reviews.llvm.org/D114256 __

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:6082-6085 + // "from" context. Because these DeclContext values look already not stable + // and unimportant this change looks acceptable. + // For these reasons the old DeclContext must be saved to chang

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 389935. martong marked an inline comment as done. martong added a comment. - Add new test case (for no-crash) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/ https://reviews.llvm.org/D103317 Files:

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 389936. martong added a comment. - Remove superfluous param (int a) from the new test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/ https://reviews.llvm.org/D103317 Files: clang/lib/StaticAn

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added a comment. I've added a new test case that triggers an assertion. That problem is getting fixed in a new parent patch, which I am about to add to the stack very soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/ https://

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong created this revision. martong added a reviewer: 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. martong requested review of

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong added a comment. > Please repeat the measurement for `openssl`. There must have been some > interference in the memory consumption. Aside from that the results look > great. I did. Average memory usage is unaffected. F20722167: svalbuilder_improvements_openssl.png

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong updated this revision to Diff 390343. martong marked an inline comment as done. martong added a comment. - Update a test case - Rebase to the parent patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103317/new/ https://reviews.llvm.org/D1

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

martong marked an inline comment as done. martong added inline comments. Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:75 + if (b * b == 1) +; // no-crash (assertion should not be violated) + } steakhal wrote: > I guess, th

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong updated this revision to Diff 390349. martong marked 4 inline comments as done. martong added a comment. - Update a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114619/new/ https://reviews.llvm.org/D114619 Files: clang/lib/Stati

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2226 + // + // Empirical measurements show that if we relax assumption G then the + // runtime does not grow noticeably. This is most probably because the --

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. In D114619#3155363 , @steakhal wrote: > I see. Simplification is always good. Let's measure and compare the runtime > characteristics before moving forward. Here they are. Seems like the runtime and memory consumption are pretty

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

This revision was automatically updated to reflect the committed changes. Closed by commit rGf02c5f347831: [Analyzer][solver] Do not remove the simplified symbol from the eq class (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

This revision was automatically updated to reflect the committed changes. Closed by commit rG0a17896fe6fd: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in… (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong created this revision. martong added a reviewer: 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. martong requested review of

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2218 +LLVM_NODISCARD +static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State, + const SymbolRef Sym) { Perhaps

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. In D114619#3164898 , @thakis wrote: > The test sometimes fails flakily: http://45.33.8.238/macm1/22813/step_7.txt Thanks @thakis for the report, I am aware of the issue and addressing that in https://reviews.llvm.org/D114887 Re

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. @thakis, if the issue is really disturbing and cannot wait until the review of D114887 finishes then please do a revert of this patch. But then the dependent child patch D103317 needs to be reverted a

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong added a comment. I am attaching the performance measurement results: F20834663: removemember_revert_with_another_fixpoint.png F20834673: stats.html There is no any noticeable difference. I am going to commit thi

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20f8733d4b8d: [Analyzer][solver] Simplification: Do a fixpoint ite

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

martong added a comment. I've had the confidence to commit D114887 , the test should not be flaky anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114619/new/ https://reviews.llvm.org/D114619 __

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

martong added a comment. In D114887#3165397 , @steakhal wrote: > Also, please provide the coverage of the new test case (only excercising the > new test case not the whole test file) I really want to make sure that it > will cover the loop as it suppose

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong created this revision. martong added a reviewer: 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. martong requested review of

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong added a comment. Measurement results are attached, I think there is no relevant difference. F20847935: another_fixpoint_before_merge_2.png F20847949: stats.html Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong added a comment. Here is a smaller reproducer: void bar(short k) { ++k; // k1 = k0 + 1 assert(k == 1); // k1 == 1 --> k0 == 0 (long)k << 16; // k0 + 1 << 16 } And the explanation is the following. With this patch, when the analyzer evaluates the `(long)k

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong updated this revision to Diff 391352. martong marked 2 inline comments as done. martong added a comment. - Rename simplifySValImpl to simplifySValOnce - Remove a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114938/new/ https://revi

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

martong accepted this revision. martong added a comment. Nice, assiduous work! The tests are awesome! LGTM, with minor revisions. Please check out my suggestions about the tests' formatting and there are those disturbing (LHS, RHS) swaps in the comments. I am going to continue with the next patc

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:372 QualType resultTy) { NonLoc InputLHS = lhs; NonLoc InputRHS = rhs; steakhal wrote: > @martong, you simplified the operands

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:51 + // sub-trees and if a value is a constant we do the constant folding. + SVal simplifySValImpl(ProgramStateRef State, SVal V); + steakhal wrote: > What about call

[PATCH] D114418: [clang][ASTImporter] Update lookup table correctly at deduction guides.

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

[PATCH] D135136: [analyzer] Make directly bounded LazyCompoundVal as lazily copied

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2843 -// subregions of the `LCS.getRegion()` also lazily copied. -if (const MemRegion *R = LCS->getRe

[PATCH] D136162: [analyzer] Fix assertion failure in RegionStore within bindArray()

martong added a comment. Hmm, seems like the conflicting prototype (i.e. the obsolescent use of zero parameters) is needed to reproduce the assertion failure. That makes me wonder, how does the redecl chain of `b` looks like? Is `void b()` chained with `void b(int*)`, or are they represented in

[PATCH] D136162: [analyzer] Fix assertion failure in RegionStore within bindArray()

martong added a comment. > A similar situation could happen if we reinterpret cast pointers, etc. so the > situation is not limited to conflicting function prototypes. Please provide tests for those cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

martong added a comment. In D135247#3867445 , @balazske wrote: > If `StdCLibraryFunctionsChecker` is added as dependency to `StreamChecker` > and no other thing is changed these tests fail, and I could not find out the > reason: > Errors in test **strea

[PATCH] D133698: [clang][dataflow] SignAnalysis, edgeTransfer, branchTransfer

martong updated this revision to Diff 469625. martong marked 8 inline comments as done. martong added a comment. - Add comments - Assumption -> ConditionValue - Use CRTP - branchTransfer -> transferBranch - Make just one simple test case for transferBranch Repository: rG LLVM Github Monorepo

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong updated this revision to Diff 469636. martong edited the summary of this revision. martong added a comment. - Rebase to latest llvm/main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133698/new/ https://reviews.llvm.org/D133698 Files: cl

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:127 + // Default implementation is a Noop. + virtual void branchTransfer(bool Branch, const Stmt *S, Lattice &L, + Environment &Env) {}

[PATCH] D136162: [analyzer] Fix assertion failure with conflicting prototype calls

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks for the update. Nice Work! Comment at: clang/test/Analysis/region-store.c:66 + // expected-warning@+1 {{passing arguments to 'b' without a prototype is deprecated

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

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

[PATCH] D136668: [clang][dataflow] Add initial sign analysis

martong created this revision. martong added reviewers: xazax.hun, gribozavr2, sgatev, ymandel. Herald added subscribers: steakhal, rnkovacs. Herald added a reviewer: Szelethus. Herald added a project: All. martong requested review of this revision. Herald added a project: clang. Herald added a sub

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. In D135375#3882491 , @Szelethus wrote: > Seems like the issues mentioned above are real, but orthogonal to this patch. > Would it be okay to address

[PATCH] D136603: [analyzer] Model cast after LValueToRValueBitCasts

martong added a comment. I am not sure, if the `ExprEngine::VisitCast` is the proper place to add these new modifications and call SValBuilder's evalCast. I think it might be better positioned in `RegionStoreManager::getBinding`. Considering, we already do a cast evaluation for certain kind of

[PATCH] D136603: [analyzer] Model cast after LValueToRValueBitCasts

martong added a comment. > PS: I'm not sure how/when we should get rid of the LocAsInteger and > represent this by a SymbolCast. > Maybe @ASDenysPetrov or @martong could help me review this. Whenever this https://reviews.llvm.org/D117229 gets accepted and when we emit all symbolCasts for all int

[PATCH] D136684: [clang][ASTImporter] Remove use of ParentMapContext.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. It is very good that we can get rid of the `ParentMapContext` because it was sub-optimal to build that up for **//all//** declaration contexts of the tra

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TransferBranchTest.cpp:9-11 +// This file defines a simplistic version of Sign Analysis as an example +// of a forward, monotonic dataflow analysis. The analysis tracks all +// variables in the s

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong marked 9 inline comments as done. martong added a comment. Thanks for the assiduous review @ymandel ! Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:78-79 public: - TerminatorVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env, + //

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong updated this revision to Diff 470748. martong marked an inline comment as done. martong added a comment. - Address ymandel's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133698/new/ https://reviews.llvm.org/D133698 Files: clang

[PATCH] D136668: [clang][dataflow] Add initial sign analysis

martong marked 13 inline comments as done. martong added a comment. In D136668#3883241 , @ymandel wrote: > Nice!! Just nits. > > At a high level, I'm a little concerned about this as a demo, since I > wouldn't recommend this implementation in practice

[PATCH] D136668: [clang][dataflow] Add initial sign analysis

martong updated this revision to Diff 470796. martong marked 2 inline comments as done. martong added a comment. - Address ymandel's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136668/new/ https://reviews.llvm.org/D136668 Files: clang

[PATCH] D136668: [clang][dataflow] Add initial sign analysis

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG93ce23adb548: [clang][dataflow] Add initial sign analysis (authored by martong). Changed prior to commit: https://reviews.llvm.org/D136668?vs=4707

[PATCH] D133698: [clang][dataflow] Implement transferBranch

This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rGbb72d0dde29e: [clang][dataflow] Implement transferBranch (authored by martong). Changed prior to commit: https://reviews.llvm.org/D133698?vs=470748&id=470802#to

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TransferBranchTest.cpp:73 + .getName())}), + /*VerifyResults=*/Match), + llvm::Succeeded()); ymandel wrote: > nit: maybe just ren

[PATCH] D136684: [clang][ASTImporter] Remove use of ParentMapContext.

martong added a comment. In D136684#3884983 , @balazske wrote: > This is really a NFC-like change but not NFC because it has visible effects > of removing some crashes. I could not produce a test that provokes the wrong > case (when `getParents` returns

[PATCH] D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

martong added a comment. In D135360#3885494 , @balazske wrote: > If we look only at the C standard we can not tell much about if the functions > should set `errno`. It seems that setting `errno` is totally > implementation-dependent, except for a few fu

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rG82a50812f7e5: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg (authored by martong). R

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

martong added a comment. In D101526#3883871 , @NoQ wrote: > Looks great to me, thanks!! Thanks for the review! Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp:32-33 __buf_size_arg_constraint_con

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:165 + TypeErasedDataflowAnalysis &Analysis; const StmtToEnvMap &StmtToEnv; Actually, this member is not used, I am not sure why was this added. Anyway

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:165 + TypeErasedDataflowAnalysis &Analysis; const StmtToEnvMap &StmtToEnv; martong wrote: > Actually, this member is not used, I am not sure why was t

[PATCH] D133698: [clang][dataflow] Implement transferBranch

martong added a comment. In D133698#3886329 , @aaron.ballman wrote: > FWIW, it looks like this change broke the AIX bot: > https://lab.llvm.org/buildbot/#/builders/214/builds/3932 Yes, I've received many build bot failures complaining about the unused

[PATCH] D136603: [analyzer] Model cast after LValueToRValueBitCasts

martong added a comment. In D136603#3888065 , @steakhal wrote: > Previously, even in the `RegionStoreManager::getBinding()` even if `T` was > non-null, we replaced it with `TVR->getValueType()` in case the `MR` was > `TypedValueRegion`. Yeah, that mean

[PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM, Thanks! Comment at: clang/lib/AST/DeclBase.cpp:1781 if (Name && !hasLazyLocalLexicalLookups() && !hasLazyExternalLexicalLookups()) { if (StoredDeclsMap

[PATCH] D133698: [clang][dataflow] SignAnalysis, edgeTransfer, branchTransfer

martong planned changes to this revision. martong added a comment. Aligned with the RFC, I am going to dissect this patch into two patches: 1. This patch will remain a simple infrastructural change that introduces transferBranch. This could be useful for complex lattices (e.g integer value rang

[PATCH] D134303: [AST] Preserve more structure in UsingEnumDecl node.

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:4866 auto ToUsingLoc = importChecked(Err, D->getUsingLoc()); - auto ToEnumLoc = importChecked(Err, D->getEnumLoc()); + auto ToEnumLoc = importChecked(Err, D->getEnumType()); auto ToEnumDecl = importC

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3707 ToFunction->setRangeEnd(ToEndLoc); + ToFunction->setDefaultLoc(D->getDefaultLoc()); This should be imported very similarly to `EndLoc`. Repository: rG LLVM Github Monorepo CH

<    12   13   14   15   16   17   18   19   20   21   >