Page MenuHomePhabricator

martong (Gabor Marton)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 10 2017, 8:01 AM (241 w, 4 d)

Recent Activity

Yesterday

martong requested review of D126560: [analyzer][NFC] SimpleSValBuilder simplification: Remove superfluous workaround code.
Fri, May 27, 11:55 AM · Restricted Project, Restricted Project
martong updated the diff for D126481: [analyzer] Handle SymbolCast in SValBuilder.
  • Hoist S->getOperand, rework the test file
Fri, May 27, 9:34 AM · Restricted Project, Restricted Project
martong added inline comments to D126481: [analyzer] Handle SymbolCast in SValBuilder.
Fri, May 27, 9:34 AM · Restricted Project, Restricted Project
martong retitled D126534: [analyzer] Deadstore static analysis: Fix false positive on C++17 assignments from Deadstore static analysis: Fix false positive on C++17 assignments to [analyzer] Deadstore static analysis: Fix false positive on C++17 assignments.
Fri, May 27, 7:09 AM · Restricted Project, Restricted Project
martong accepted D126215: [analyzer] Deprecate `-analyzer-store region` flag.

Still looks good.

Fri, May 27, 5:43 AM · Restricted Project, Restricted Project
martong committed rG6ab69efe61f2: [analyzer][NFC] Rename GREngine->CoreEngine, GRExprEngine->ExprEngine in… (authored by martong).
[analyzer][NFC] Rename GREngine->CoreEngine, GRExprEngine->ExprEngine in…
Fri, May 27, 2:05 AM · Restricted Project, Restricted Project

Thu, May 26

martong added a comment to D103096: [analyzer] Implement cast for ranges of symbolic integers.

Denys, I've created a very simple patch that makes the SValBuilder to be able to look up and use a constraint for an operand of a SymbolCast. That change passes 2 of your test cases, thus I made that a parent patch.

Thu, May 26, 8:48 AM · Restricted Project, Restricted Project
martong requested review of D126481: [analyzer] Handle SymbolCast in SValBuilder.
Thu, May 26, 8:44 AM · Restricted Project, Restricted Project
martong added a comment to D125400: [clang][Analyzer] Add errno state to standard functions modeling..

Nice work, looks promising! Once this https://reviews.llvm.org/D125400?vs=431909&id=431924#inline-1206369 is addressed I will accept.

Thu, May 26, 6:33 AM · Restricted Project, Restricted Project
martong accepted D125986: [clang][ASTImporter] Add support for import of UsingPackDecl..

LGTM! Thanks! And sorry for the delay in the review, please ping me next time.

Thu, May 26, 6:04 AM · Restricted Project, Restricted Project
martong added inline comments to D125318: [analyzer] Add UnarySymExpr.
Thu, May 26, 5:18 AM · Restricted Project, Restricted Project
martong committed rGcd5783d3e82b: [analyzer][solver] Handle UnarySymExpr in SMTConv (authored by martong).
[analyzer][solver] Handle UnarySymExpr in SMTConv
Thu, May 26, 5:17 AM · Restricted Project, Restricted Project
martong committed rG88abc50398eb: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver (authored by martong).
[analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver
Thu, May 26, 5:17 AM · Restricted Project, Restricted Project
martong committed rGb5b2aec1ff51: [analyzer] Add UnarySymExpr (authored by martong).
[analyzer] Add UnarySymExpr
Thu, May 26, 5:17 AM · Restricted Project, Restricted Project
martong closed D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
Thu, May 26, 5:17 AM · Restricted Project, Restricted Project
martong closed D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver.
Thu, May 26, 5:17 AM · Restricted Project, Restricted Project
martong closed D125318: [analyzer] Add UnarySymExpr.
Thu, May 26, 5:17 AM · Restricted Project, Restricted Project
martong committed rGca3d962548b9: [analyzer] Return from reAssume if State is posteriorly overconstrained (authored by martong).
[analyzer] Return from reAssume if State is posteriorly overconstrained
Thu, May 26, 4:51 AM · Restricted Project, Restricted Project
martong added inline comments to D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained.
Thu, May 26, 4:51 AM · Restricted Project, Restricted Project
martong closed D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained.
Thu, May 26, 4:51 AM · Restricted Project, Restricted Project
martong added inline comments to D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained.
Thu, May 26, 2:49 AM · Restricted Project, Restricted Project
martong updated the diff for D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained.
  • Make reAssume friend, pin the target in the test
Thu, May 26, 2:49 AM · Restricted Project, Restricted Project

Wed, May 25

martong added inline comments to D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained.
Wed, May 25, 12:41 PM · Restricted Project, Restricted Project
martong requested review of D126406: [analyzer] Return from reAssume if State is posteriorly overconstrained.
Wed, May 25, 12:30 PM · Restricted Project, Restricted Project
martong added a comment to D124758: [analyzer] Implement assume in terms of assumeDual.

This change in itself reduced the run-time of the analysis to 16 seconds, on my machine. However, the repetition of States should still be addressed. I am going to upload the upper patch for a starter.

Wed, May 25, 12:08 PM · Restricted Project, Restricted Project
martong added a comment to D124758: [analyzer] Implement assume in terms of assumeDual.

Thanks Balazs for the report.

Wed, May 25, 11:49 AM · Restricted Project, Restricted Project
martong updated the diff for D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
  • Compare the size of the types instead of the type pointers
Wed, May 25, 9:17 AM · Restricted Project, Restricted Project
martong added inline comments to D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
Wed, May 25, 9:14 AM · Restricted Project, Restricted Project
martong updated the diff for D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
  • Fix Assertion `*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same sort!"' failed.
Wed, May 25, 6:53 AM · Restricted Project, Restricted Project
martong committed rGf75bc5bfc8f8: [analyzer] Fix symbol simplification assertion failure (authored by martong).
[analyzer] Fix symbol simplification assertion failure
Wed, May 25, 2:03 AM · Restricted Project, Restricted Project
martong closed D126281: [analyzer] Fix symbol simplification assertion failure.
Wed, May 25, 2:02 AM · Restricted Project, Restricted Project

Tue, May 24

martong accepted D126123: [analyzer][NFC] Use MemRegion::getRegion()'s return value unconditionally.
Tue, May 24, 3:40 AM · Restricted Project, Restricted Project
martong added inline comments to D126281: [analyzer] Fix symbol simplification assertion failure.
Tue, May 24, 3:39 AM · Restricted Project, Restricted Project
martong updated the diff for D126281: [analyzer] Fix symbol simplification assertion failure.
  • Add clang_analyzer_warnIfReached() to the test and remove unused debug function decls
Tue, May 24, 3:39 AM · Restricted Project, Restricted Project
martong added a comment to D126123: [analyzer][NFC] Use MemRegion::getRegion()'s return value unconditionally.

Is it documented with getRegion? Could we decorate that with returns-nonnull https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?

Ah, I thought one of my changes addressing this was already on review.
It turns out it was not the case. Now I uploaded it for review: D126198

Tue, May 24, 3:28 AM · Restricted Project, Restricted Project
martong added a comment to D126281: [analyzer] Fix symbol simplification assertion failure.

Point 2) above has negligible performance impact, empirical measurements do not show any noticeable difference in the run-time.

Tue, May 24, 3:06 AM · Restricted Project, Restricted Project
martong accepted D126215: [analyzer] Deprecate `-analyzer-store region` flag.

LGTM.

Tue, May 24, 3:02 AM · Restricted Project, Restricted Project
martong added inline comments to D126281: [analyzer] Fix symbol simplification assertion failure.
Tue, May 24, 2:48 AM · Restricted Project, Restricted Project
martong requested review of D126281: [analyzer] Fix symbol simplification assertion failure.
Tue, May 24, 2:47 AM · Restricted Project, Restricted Project

Mon, May 23

martong added a comment to D126215: [analyzer] Deprecate `-analyzer-store region` flag.

We should support deprecated analyzer flags for at least one release. In this case I'm planning to drop this flag in clang-17

Mon, May 23, 9:36 AM · Restricted Project, Restricted Project
martong accepted D126216: [analyzer][NFC] Remove unused RegionStoreFeatures.

LGTM

Mon, May 23, 9:35 AM · Restricted Project, Restricted Project
martong accepted D126130: [analyzer][NFC] Remove unused SVal::hasConjuredSymbol.
Mon, May 23, 5:32 AM · Restricted Project, Restricted Project
martong accepted D126198: [analyzer][NFCi] Annotate major nonnull returning functions.

Looks good, with minor revisions.

Mon, May 23, 5:30 AM · Restricted Project, Restricted Project
martong added a comment to D126123: [analyzer][NFC] Use MemRegion::getRegion()'s return value unconditionally.

Is it documented with getRegion? Could we decorate that with returns-nonnull https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?

Mon, May 23, 5:03 AM · Restricted Project, Restricted Project
martong added a comment to D126130: [analyzer][NFC] Remove unused SVal::hasConjuredSymbol.

Remove unused SVal::hasConjuredSymbol

The title seems to be off with the changes.

Mon, May 23, 5:00 AM · Restricted Project, Restricted Project
martong accepted D126129: [analyzer][NFC] Remove unused nonloc::ConcreteInt::evalBinOp.
Mon, May 23, 4:58 AM · Restricted Project, Restricted Project
martong accepted D126128: [analyzer][NFC] Inline loc::ConcreteInt::evalBinOp.

Looks, good, but it was a struggle to follow if you did the inlining right or not. TBH, someone else should also check it.

Mon, May 23, 4:57 AM · Restricted Project, Restricted Project
martong added a comment to D126127: [analyzer][NFC] Relocate unary transfer functions.

This is an initial step of removing the SimpleSValBuilder abstraction. The SValBuilder alone should be enough.

Mon, May 23, 4:49 AM · Restricted Project, Restricted Project
martong added inline comments to D126126: [analyzer][NFC] Inline and simplify nonloc::ConcreteInt functions.
Mon, May 23, 4:40 AM · Restricted Project, Restricted Project
martong accepted D126125: [analyzer][NFC] Inline ExprEngine::evalMinus.
Mon, May 23, 4:32 AM · Restricted Project, Restricted Project
martong accepted D126124: [analyzer][NFC] Inline ExprEngine::evalComplement.
Mon, May 23, 4:31 AM · Restricted Project, Restricted Project
martong committed rG96fba640cf58: [analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and… (authored by martong).
[analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and…
Mon, May 23, 12:47 AM · Restricted Project, Restricted Project
martong committed rG32f189b0d9a8: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual (authored by martong).
[analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual
Mon, May 23, 12:47 AM · Restricted Project, Restricted Project
martong closed D125954: [analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and assumeInclusiveRangeDual.
Mon, May 23, 12:47 AM · Restricted Project, Restricted Project
martong closed D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.
Mon, May 23, 12:47 AM · Restricted Project, Restricted Project

Fri, May 20

martong added a comment to D126067: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag.

Uhh, ohh, don't forget to reflect this change in the ReleaseNotes.rst, so urers will be notified!

Fri, May 20, 7:23 AM · Restricted Project, Restricted Project, Restricted Project
martong accepted D126067: [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag.

However, this arises a couple of burning questions.

Should the cc1 frontend still accept this flag - to keep tools/users passing this flag directly to cc1 (which is unsupported, unadvertised) working.
If we should remain backward compatible, how long?
How can we get rid of deprecated and obsolete flags at some point?

The answer might be similar to what we do in case of a checker rename (or when we move out from alpha). Such a rename have the same problems, but we have not made much efforts to combat these problem. Users had to comply.

Fri, May 20, 7:20 AM · Restricted Project, Restricted Project, Restricted Project
martong added inline comments to D125986: [clang][ASTImporter] Add support for import of UsingPackDecl..
Fri, May 20, 1:19 AM · Restricted Project, Restricted Project

Thu, May 19

martong added a comment to D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.

There are Z3 refutation related assertions on open-source projects once the patch stack is applied. Interestingly it happens in fromBinOp.

clang-14: ../../git/llvm-project/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:96: static const llvm::SMTExpr* clang::ento::SMTConv::fromBinOp(llvm::SMTSolverRef&, const llvm::SMTExpr* const&, clang::BinaryOperator::Opcode, const llvm::SMTExpr* const&, bool): Assertion `*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same sort!"' failed.
Thu, May 19, 4:20 AM · Restricted Project, Restricted Project
martong updated the summary of D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.
Thu, May 19, 3:56 AM · Restricted Project, Restricted Project
martong added a comment to D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.

Now I see, the summary confused me.

This includes the refactoring of the common assumle*Dual logic into the
function template assumeDualImpl.

I felt like this is a refactoring change, but it was not. It's about fixing the behavior by using the cloneAsPosteriorlyOverconstrained().
Please reword the summary.

Thu, May 19, 3:55 AM · Restricted Project, Restricted Project
martong updated the diff for D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.
  • Move SimpleConstraintManager's assume*Internal functions to be protected
Thu, May 19, 3:55 AM · Restricted Project, Restricted Project
martong updated the summary of D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.
Thu, May 19, 3:49 AM · Restricted Project, Restricted Project
martong accepted D125920: [analyzer][NFC] Remove the unused LocAsInteger::getPersistentLoc().

LGTM. Are there any other getPersistentLoc function definitions in other SVals? Might they be also unused?

Thu, May 19, 3:47 AM · Restricted Project, Restricted Project
martong requested review of D125954: [analyzer][NFC] Factor out the copy-paste code repetition of assumeDual and assumeInclusiveRangeDual.
Thu, May 19, 2:26 AM · Restricted Project, Restricted Project
martong updated the diff for D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.
  • Split into two patches, this first patch will be just the no-brainer copy-paste from assumeDual implementaton.
Thu, May 19, 2:24 AM · Restricted Project, Restricted Project
martong added a comment to D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.

Okay, it took me a while to get what's going on.
Do you have anything in mind to express it by slightly less indirection, references, templates and well, virtual functions xD

Thu, May 19, 2:23 AM · Restricted Project, Restricted Project

Wed, May 18

martong added a comment to D125871: [analyzer] Delete alpha.deadcode.UnreachableCode checker.

Could you please give a few examples of these FPs for the record?

Wed, May 18, 8:25 AM · Restricted Project, Restricted Project
martong requested review of D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual.
Wed, May 18, 8:02 AM · Restricted Project, Restricted Project
martong added inline comments to D125871: [analyzer] Delete alpha.deadcode.UnreachableCode checker.
Wed, May 18, 3:52 AM · Restricted Project, Restricted Project
martong requested review of D125871: [analyzer] Delete alpha.deadcode.UnreachableCode checker.
Wed, May 18, 3:51 AM · Restricted Project, Restricted Project
martong committed rG56b9b97c1ef5: [clang][analyzer][ctu] Make CTU a two phase analysis (authored by martong).
[clang][analyzer][ctu] Make CTU a two phase analysis
Wed, May 18, 1:37 AM · Restricted Project, Restricted Project
martong committed rG25ac078a961d: [clang][ASTImporter] Add isNewDecl (authored by martong).
[clang][ASTImporter] Add isNewDecl
Wed, May 18, 1:37 AM · Restricted Project, Restricted Project
martong closed D123773: [clang][analyzer][ctu] Make CTU a two phase analysis.
Wed, May 18, 1:37 AM · Restricted Project, Restricted Project
martong closed D123685: [clang][ASTImporter] Add isNewDecl.
Wed, May 18, 1:37 AM · Restricted Project, Restricted Project
martong added inline comments to D123773: [clang][analyzer][ctu] Make CTU a two phase analysis.
Wed, May 18, 1:25 AM · Restricted Project, Restricted Project

Tue, May 17

martong updated the diff for D123773: [clang][analyzer][ctu] Make CTU a two phase analysis.
  • Update ReleaseNotes.rst
Tue, May 17, 7:04 AM · Restricted Project, Restricted Project
martong added inline comments to D125771: [clang-tidy] Add a useful note about -std=c++11-or-later.
Tue, May 17, 6:32 AM · Restricted Project, Restricted Project
martong abandoned D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order.

Abandoning because by using a bool in ctuBifurcate, the CTUWorklist will not have more than one elements during the first phase. Details: https://reviews.llvm.org/D123773?id=426037#inline-1206493

Tue, May 17, 6:29 AM · Restricted Project, Restricted Project
martong updated the diff for D123773: [clang][analyzer][ctu] Make CTU a two phase analysis.
  • Change ctuBifurcate to use bool in GDM
Tue, May 17, 6:24 AM · Restricted Project, Restricted Project
martong added inline comments to D123773: [clang][analyzer][ctu] Make CTU a two phase analysis.
Tue, May 17, 6:15 AM · Restricted Project, Restricted Project
martong accepted D125708: [analyzer][NFC] Remove unused default SVal constructors.

LGTM

Tue, May 17, 1:53 AM · Restricted Project, Restricted Project
martong accepted D125707: [analyzer][NFC] Remove unused friend SVal declarations.

LGTM

Tue, May 17, 1:53 AM · Restricted Project, Restricted Project
martong accepted D125706: [analyzer][NFC] Use idiomatic classof instead of isKind.

LGTM

Tue, May 17, 1:53 AM · Restricted Project, Restricted Project
martong added a comment to D125709: [analyzer][Casting] Support isa, cast, dyn_cast of SVals.

I'm not sure, shall I add tests?

Tue, May 17, 1:51 AM · Restricted Project, Restricted Project
martong added inline comments to D125400: [clang][Analyzer] Add errno state to standard functions modeling..
Tue, May 17, 1:36 AM · Restricted Project, Restricted Project
martong added a comment to D125400: [clang][Analyzer] Add errno state to standard functions modeling..

@martong Do you mean that a "describe" function is needed for the return value constraint of the function and for the errno constraint type? Then a note tag can contain what the function is assumed to return on success and what is allowed (or not) to do with errno. For example: "Assuming that 'mkdir' is successful, it returns zero in this case and value of 'errno' is unspecified after the call".

Tue, May 17, 1:31 AM · Restricted Project, Restricted Project
martong added a comment to D125400: [clang][Analyzer] Add errno state to standard functions modeling..

Function mkdir is modeled incorrectly by the checker. According to the man page it can return 0 or -1 only (-1 is error) but the checker allows non-negative value at success. So the shown bug report is incorrect (it can be only -1 if not 0 and then check of errno is allowed). Anyway the note tags should be added to every function.

Tue, May 17, 1:28 AM · Restricted Project, Restricted Project

Mon, May 16

martong added a comment to D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.

Something is messed up with the diff. You refer to fromUnOp() but it's not defined anywhere.

Mon, May 16, 2:35 AM · Restricted Project, Restricted Project
martong updated the diff for D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
  • Use existing fromUnOp
  • pass nullptr as FromTy
Mon, May 16, 12:51 AM · Restricted Project, Restricted Project
martong added inline comments to D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
Mon, May 16, 12:51 AM · Restricted Project, Restricted Project

Fri, May 13

martong added inline comments to D125524: [BoundV2] ArrayBoundV2 checks if the extent is tainted.
Fri, May 13, 7:20 AM · Restricted Project, Restricted Project
martong requested review of D125547: [analyzer][solver] Handle UnarySymExpr in SMTConv.
Fri, May 13, 7:07 AM · Restricted Project, Restricted Project
martong updated the diff for D123685: [clang][ASTImporter] Add isNewDecl.
  • setNewDecl -> markAsNewDecl
Fri, May 13, 6:01 AM · Restricted Project, Restricted Project
martong added inline comments to D125524: [BoundV2] ArrayBoundV2 checks if the extent is tainted.
Fri, May 13, 5:51 AM · Restricted Project, Restricted Project
martong accepted D125532: [analyzer] Introduce clang_analyzer_dumpSvalType introspection function.
Fri, May 13, 5:44 AM · Restricted Project, Restricted Project
martong added inline comments to D125318: [analyzer] Add UnarySymExpr.
Fri, May 13, 5:31 AM · Restricted Project, Restricted Project
martong updated the diff for D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver.
  • Add a test case for casts related crash
Fri, May 13, 5:24 AM · Restricted Project, Restricted Project
martong updated the diff for D125318: [analyzer] Add UnarySymExpr.
  • Revert "Add type parameter to evalMinus and evalComplement"
Fri, May 13, 5:20 AM · Restricted Project, Restricted Project
martong added inline comments to D125318: [analyzer] Add UnarySymExpr.
Fri, May 13, 5:19 AM · Restricted Project, Restricted Project