xazax.hun (Gábor Horváth)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 17 2012, 3:16 AM (309 w, 1 d)

Recent Activity

Wed, Aug 15

xazax.hun added a comment to D48027: [analyzer] Improve `CallDescription` to handle c++ method..

Generally looks good, I only wonder if this works well with inline namespaces. Could you test?

Wed, Aug 15, 12:51 PM

Thu, Aug 2

xazax.hun accepted D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker.
Thu, Aug 2, 10:54 AM
xazax.hun accepted D49811: [analyzer] Obtain a ReturnStmt from a CFGAutomaticObjDtor.

We could also print something about the ReturnStmt, like source location or pretty print of its expressions so we can check that we picked the right one in case we have multiple.
But consider this as an optional task if you have nothing better to do. I am ok with committing this as is.

Thu, Aug 2, 10:51 AM

Fri, Jul 27

xazax.hun accepted D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker.

LGTM!

Fri, Jul 27, 6:52 PM

Wed, Jul 25

xazax.hun added inline comments to D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker.
Wed, Jul 25, 9:17 AM
xazax.hun added a comment to D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker.

Small comments inline.

Wed, Jul 25, 8:42 AM

Mon, Jul 23

xazax.hun added a comment to D49693: [analyzer] Try to minimize the number of equivalent bug reports evaluated by the refutation manager.

What I'm suggesting is: first run all visitors, since no other visitor mark them as invalid, the evaluation of all 497 bugs will be fast and will return only on bug report. We run the refutation manager in this one bug report.

Mon, Jul 23, 4:00 PM
xazax.hun added inline comments to D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker.
Mon, Jul 23, 10:04 AM
xazax.hun requested changes to D49656: [analyzer] Add support for more pointer invalidating functions in InnerPointerChecker.

Some comments, mostly nits inline.

Mon, Jul 23, 9:01 AM

Jul 19 2018

xazax.hun added a comment to D49568: [analyzer][WIP] Scan the program state map in the visitor only once in DanglingInternalBufferChecker.

Yeah, I would rather have the cleanups and do extra work in the visitor. But lets wait what @NoQ thinks.

Jul 19 2018, 3:00 PM

Jul 11 2018

xazax.hun added a comment to D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager.

This may remove false-positives (like the example above), but we surely cannot find new errors where multiplicative operations are used.

Do you have examples of those? In my understanding, (almost) all of the imprecisions in the solver lead purely to false positives.
The only example where you lose bugs is when you stop exploring due to reaching a (false) fatal error.

Jul 11 2018, 12:19 PM

Jul 9 2018

xazax.hun added a comment to D49058: [analyzer] Move InnerPointerChecker out of alpha.

Don't you need to edit the tests as well?

Jul 9 2018, 9:46 PM

Jul 8 2018

xazax.hun added a comment to D49057: [analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker.

Thanks! The changes look good, I forgot to mark one double lookup though in my previous review.

Jul 8 2018, 10:59 AM
xazax.hun accepted D49057: [analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker.

Overall looks good, some nits inline. Let's run it on some projects to exercise this change.

Jul 8 2018, 10:21 AM

Jul 4 2018

xazax.hun added a comment to D48027: [analyzer] Improve `CallDescription` to handle c++ method..
In D48027#1142324, @MTC wrote:
  • There is possible match the wrong AST node, e.g. for the NamedDecl foo(), if the function body has the a::b::x, when we match a::b::X(), we will match foo() . Because I'm not familiar with ASTMatcher, my bad, I can't think of a perfect way to match only the specified nodes.
Jul 4 2018, 12:30 PM

Jun 26 2018

xazax.hun added a comment to D48565: [analyzer] Replace the vector of ConstraintSets by a single ConstraintSet and a function to merge ConstraintSets.

Ok, it looks like naively just dropping all the constraints at Z3 is not the most efficient way.

Jun 26 2018, 12:29 AM

Jun 25 2018

xazax.hun added inline comments to D48532: [analyzer] Add support for std::basic_string::data() in DanglingInternalBufferChecker.
Jun 25 2018, 8:47 AM
xazax.hun accepted D48532: [analyzer] Add support for std::basic_string::data() in DanglingInternalBufferChecker.

LG!

Jun 25 2018, 8:42 AM

Jun 24 2018

xazax.hun accepted D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker.
Jun 24 2018, 10:08 AM
xazax.hun accepted D48460: [analyzer] Fix invalidation on C++ const methods..

LGTM!

Jun 24 2018, 9:59 AM
xazax.hun accepted D48521: [analyzer] Highlight container object destruction in MallocChecker.

LGTM, given that the assert does not fire for the projects you tested on.

Jun 24 2018, 9:56 AM

Jun 23 2018

xazax.hun added a comment to D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker.

Looks better, thanks!

Jun 23 2018, 1:39 PM
xazax.hun added a comment to D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker.

Regarding the visitor:
Maybe rather than looking at the AST, we should check the states, when we started to track the returned symbol?

Jun 23 2018, 12:11 PM
xazax.hun added inline comments to D48521: [analyzer] Highlight container object destruction in MallocChecker.
Jun 23 2018, 12:05 PM
xazax.hun added inline comments to D48513: [analyzer] Correctly create a non-fatal error node for VA list checker..
Jun 23 2018, 1:29 AM

Jun 18 2018

xazax.hun added a comment to D48285: [analyzer][UninitializedObjectChecker] Added "NotesAsWarnings" flag.

I wonder if this could be done when plist is emitted generally, independent of any checks.

Jun 18 2018, 10:50 AM

Jun 13 2018

xazax.hun added inline comments to D48027: [analyzer] Improve `CallDescription` to handle c++ method..
Jun 13 2018, 9:32 AM

Jun 11 2018

xazax.hun added a comment to D48027: [analyzer] Improve `CallDescription` to handle c++ method..

Having C++ support would be awesome!
Thanks for working on this!

Jun 11 2018, 8:09 AM

Jun 10 2018

xazax.hun added a comment to D47671: [analyzer] Implement copy elision..

Just for the record, there is a common example where relying on copy elision might bite and google do not recommend relying on it for correctness: https://abseil.io/tips/120

Jun 10 2018, 3:58 AM

Jun 5 2018

xazax.hun added a comment to D47671: [analyzer] Implement copy elision..

So having an analyzer-config option would be useful for those codebases that are expected to be compiled with less advanced compilers that do not elide copies or not doing it aggressively enough.

I'm not sure it makes sense. From my understanding, that's just how c++17 is defined, and that's unrelated to what compiler implementation is used.

Jun 5 2018, 3:03 PM

Jun 4 2018

xazax.hun added a comment to D47671: [analyzer] Implement copy elision..

So having an analyzer-config option would be useful for those codebases that are expected to be compiled with less advanced compilers that do not elide copies or not doing it aggressively enough. Maybe it is worth to ask on the mailing list of the community wants to have an option for this? I am not sure though how often the end users end up fine tuning the analyzer. It might be nice to have a guide how to do that (and in what circumstances does each of the config values make sense).

Jun 4 2018, 1:29 PM

Jun 1 2018

xazax.hun added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Jun 1 2018, 8:37 AM
xazax.hun added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

Sorry for the limited activity. Unfortunately, I have very little time reviewing patches lately.
I think we need to answer the following questions:

  • Does this change affect the analysis of the constructors of global objects? If so, how?
  • Do we want to import non-const object's initializer expressions? The analyzer might not end up using the value anyways.
  • How big can the index get with this modification for large projects?
Jun 1 2018, 8:29 AM

May 30 2018

xazax.hun added a comment to D45517: [analyzer] False positive refutation with Z3.

I am not not sure that I got the idea what are you suggesting here. If we have the constraint of for example a symbol s > 10 and later on a path we discover s > 20, will we also deduplicate this that way?

No. But I thought in your optimization atoms inside the constraints would be the same?
Could you give an example where they are not?

May 30 2018, 1:44 PM
xazax.hun added a comment to D45517: [analyzer] False positive refutation with Z3.

@xazax.hun (I'll reply here to avoid scattering the conversation across many subtrees)

I was thinking about the optimization for not adding redundant constraints some more, and I've decided I'm still against it ---
we are creating a higher potential for bugs, and we are tightly coupling the visitor to an internal implementation detail (all formulas are eventually purged at purge locations),
which creates a more fragile code.

The proper way to do this would be to have a set of constraints, and then add all constraints there as we iterate through the states (and through constraints inside the state).
If we use the hashing function provided by Z3, the simple act of construction of a set would implicitly drop all redundant constraints.

May 30 2018, 1:03 PM
xazax.hun added inline comments to D45517: [analyzer] False positive refutation with Z3.
May 30 2018, 11:50 AM
xazax.hun added inline comments to D45517: [analyzer] False positive refutation with Z3.
May 30 2018, 12:08 AM

May 26 2018

xazax.hun accepted D47417: [analyzer] Add missing state transition in IteratorChecker.
May 26 2018, 12:17 PM
xazax.hun accepted D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker.

LG!

May 26 2018, 12:01 PM
xazax.hun accepted D47135: [analyzer] A checker for dangling internal buffer pointers in C++.

LGTM!

May 26 2018, 11:55 AM
xazax.hun added inline comments to D47135: [analyzer] A checker for dangling internal buffer pointers in C++.
May 26 2018, 11:32 AM
xazax.hun added inline comments to D47135: [analyzer] A checker for dangling internal buffer pointers in C++.
May 26 2018, 11:30 AM
xazax.hun added a comment to D47135: [analyzer] A checker for dangling internal buffer pointers in C++.

Looks good so far, some comments inline.

May 26 2018, 11:26 AM

May 14 2018

xazax.hun added inline comments to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.
May 14 2018, 11:04 PM
xazax.hun added a comment to D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer.

The analyzer can only reason about const variables this way, right? Maybe we should only import the initializers for such variables to have better performance? What do you think?

May 14 2018, 8:56 AM
xazax.hun accepted D46398: [ASTImporterTest] Fix potential use-after-free.
May 14 2018, 8:02 AM

May 5 2018

xazax.hun added inline comments to D46233: [ASTMatchers] Overload isConstexpr for ifStmts.
May 5 2018, 5:40 AM
xazax.hun updated the diff for D46233: [ASTMatchers] Overload isConstexpr for ifStmts.
  • Rerun script
May 5 2018, 5:38 AM
xazax.hun added inline comments to D45774: [analyzer] cover more cases where a Loc can be bound to constants.
May 5 2018, 1:01 AM

Apr 29 2018

xazax.hun added a comment to D46234: Mark if a null statement is the result of constexpr folding.

Can you say something about why you want to track this?

Apr 29 2018, 1:11 PM
xazax.hun created D46234: Mark if a null statement is the result of constexpr folding.
Apr 29 2018, 5:01 AM
xazax.hun created D46233: [ASTMatchers] Overload isConstexpr for ifStmts.
Apr 29 2018, 3:55 AM

Apr 25 2018

xazax.hun added a comment to D46066: [analyzer] Add checker for underflowing unsigned integers.

Isn't this case already covered by conversion checker? https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp

Apr 25 2018, 8:31 AM · Restricted Project
xazax.hun accepted D46019: [ASTImporter] Fix isa cast assert.

With a sufficiently detailed commit message, i.e.: what version of a project should be cheked out and how the analyzer needs to be ivoked to reproduce the problem I am ok with committing this without a test.

Apr 25 2018, 4:06 AM

Apr 24 2018

xazax.hun created D46027: [clang-tidy] Fix PR35824.
Apr 24 2018, 12:19 PM
xazax.hun created D46003: [clang-tidy] Fix PR35468.
Apr 24 2018, 4:54 AM

Apr 16 2018

xazax.hun added a comment to D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer..

I am in favour of this approach. This is what I suggested back than in https://reviews.llvm.org/D23014 but it was somehow overlooked.

Apr 16 2018, 2:09 PM

Apr 13 2018

xazax.hun accepted D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition.
Apr 13 2018, 5:35 AM
xazax.hun added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Apr 13 2018, 5:32 AM

Apr 12 2018

xazax.hun accepted D44079: [ASTImporter] Allow testing of import sequences; fix import of typedefs for anonymous decls.

Overall looks good, some minor nits inline. If those can be resolved trivially, I am ok with committing this without another round of reviews.

Apr 12 2018, 9:17 AM
xazax.hun added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Apr 12 2018, 8:50 AM
xazax.hun added a comment to D45458: MallocChecker refactoring of calls checkers.
In D45458#1062342, @NoQ wrote:

@xazax.hun: do you think the approach you took in the Valist checker is applicable here? Did you like how it ended up working? Cause i'd love to see CallDescription and initializer lists used for readability here. And i'd love to see branches replaced with map lookups. And i'm not sure if anybody has written code that has all the stuff that i'd love to see.

Apr 12 2018, 8:20 AM
xazax.hun added a comment to D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition.

We encountered the same problem but did not have time yet to submit the patch. We have literally the same fix internally, so it looks good to me. One minor style nit inline.

Apr 12 2018, 8:10 AM

Mar 30 2018

xazax.hun accepted D45081: [analyzer] Remove the unused method declaration in `ValistChecker.cpp`..

LG! Thanks!

Mar 30 2018, 5:09 AM

Mar 29 2018

xazax.hun added a comment to D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen.

You should always upload the whole diff, not just the last changes.

Mar 29 2018, 10:45 AM · Restricted Project

Mar 22 2018

xazax.hun accepted D43967: [ASTImporter] Add test helper Fixture.

Overall looks good, some minor comments inline.

Mar 22 2018, 8:26 AM

Mar 19 2018

xazax.hun added a comment to D44594: [analyzer] Fix the assertion failure when static globals are used in lambda by reference.

I was thinking about test/Analysis/lambdas.cpp as a possible candidate.

Mar 19 2018, 12:07 PM

Mar 17 2018

xazax.hun accepted D44594: [analyzer] Fix the assertion failure when static globals are used in lambda by reference.

I am only wondering what is the policy regarding the tests? When should we add a new file or when should we just extend an existing one?

Mar 17 2018, 2:32 AM

Mar 6 2018

xazax.hun closed D39454: [clang-tidy] Misc redundant expressions checker updated for confused operators.

Hmm, it looks like these changes are already upstreamed. Probably as part of other patch or maybe just someone (probably me) forgot to put the proper revision in the commit message? I close this revision now.

Mar 6 2018, 3:06 AM
xazax.hun added a comment to D43967: [ASTImporter] Add test helper Fixture.

I think having both kinds of tests might make sense.
Overall, this looks good to me. Some nits inline.

Mar 6 2018, 1:52 AM
xazax.hun added a comment to D32947: [ASTImporter] FriendDecl importing improvements.

Note that changes from https://reviews.llvm.org/D44100 might affect this.

Mar 6 2018, 1:12 AM
xazax.hun added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Aleksei!

Mar 6 2018, 1:11 AM

Mar 2 2018

xazax.hun abandoned D30691: [analyzer] Support for naive cross translational unit analysis.

Resubmitted in https://reviews.llvm.org/rL326439

Mar 2 2018, 6:30 AM

Mar 1 2018

xazax.hun accepted D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy.

LGTM!

Mar 1 2018, 5:48 AM
xazax.hun added a reviewer for D30691: [analyzer] Support for naive cross translational unit analysis: ilya-biryukov.

Could you please provide some more details where the cyclic dependency is? I cannot reproduce the problem and usually cmake fails when there is a cyclic dependency and you are building shared libraries.

Mar 1 2018, 4:59 AM

Feb 28 2018

whisperity awarded D30691: [analyzer] Support for naive cross translational unit analysis a Party Time token.
Feb 28 2018, 5:40 AM

Feb 19 2018

xazax.hun added a comment to D32947: [ASTImporter] FriendDecl importing improvements.

Could you add some context?

Feb 19 2018, 7:37 AM

Feb 15 2018

xazax.hun added a comment to D30691: [analyzer] Support for naive cross translational unit analysis.

Python code looks OK to me, I have one last request: could we have a small documentation how the whole thing is supposed work in integration, preferably on an available open-source project any reader could check out?
I am asking because I have actually tried and failed to launch this CTU patch on a project I was analyzing.

Feb 15 2018, 7:49 AM
xazax.hun updated the diff for D30691: [analyzer] Support for naive cross translational unit analysis.
  • Rebased to current ToT
  • Fixed a problem that the scan-build-py used an old version of the ctu configuration option
  • Added a short guide how to use CTU
Feb 15 2018, 7:48 AM

Feb 12 2018

xazax.hun updated subscribers of D38171: [clang-tidy] Implement clang-tidy check aliases.

@alexfh did you have any chance to think about this change?

Feb 12 2018, 6:24 AM

Feb 9 2018

xazax.hun accepted D43133: [analyzer] Introduce statistics for the total number of visited basic blocks.

I like the idea of having more statistics. Moreover, due to the fact that we output the percent of reachable basic blocks as an integer, it was really hard to get precise coverage data. This addition will help in this regard.

Feb 9 2018, 10:57 AM
xazax.hun added a comment to D43131: [analyzer] Serialize statistics to plist when serialize-stats=true is set.
In D43131#1003607, @NoQ wrote:

So are these present only on translation units that actually caused bug reports to appear?

Having a test for the plist output would be nice.

I suspect such test would be super fragile. Every time we change any modeling, many of the stats may change.

Feb 9 2018, 10:48 AM
xazax.hun added a comment to D43131: [analyzer] Serialize statistics to plist when serialize-stats=true is set.

Why not turn this on by default when we turn on reporting statistics?
Having a test for the plist output would be nice.

Feb 9 2018, 10:41 AM
xazax.hun accepted D5767: Template Instantiation Observer + a few other templight-related changes.

Thanks, this looks good to me! I will try this out soon and commit after that.

Feb 9 2018, 6:51 AM
xazax.hun accepted D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl.

Looks good to me. Only found a few nits.

Feb 9 2018, 2:52 AM
xazax.hun added inline comments to D5767: Template Instantiation Observer + a few other templight-related changes.
Feb 9 2018, 2:32 AM
xazax.hun added inline comments to D5767: Template Instantiation Observer + a few other templight-related changes.
Feb 9 2018, 2:16 AM

Feb 7 2018

xazax.hun added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.
  1. What do you mean by regression tests? We have run the clang-test target successfully on the patched code (which has the hook). Note that the hook this pull request provides is implemented as a ProgramAction. It is not intended to be used together with other program actions, I don't see how we could turn it on for other tests (if that is what you referred to).
Feb 7 2018, 3:16 AM

Feb 6 2018

xazax.hun added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.

Overall looks good. Was this tested on large software? I would also be grateful if you could run the regression tests with templight always being enabled to see if they uncover any assertions/crashes.

Feb 6 2018, 3:48 AM

Feb 4 2018

xazax.hun added inline comments to D42672: [CFG] [analyzer] Heavier CFGConstructor elements..
Feb 4 2018, 5:27 AM

Jan 25 2018

xazax.hun updated the diff for D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..
  • Address review comments.
Jan 25 2018, 2:56 AM
xazax.hun accepted D42301: [ASTImporter] Support LambdaExprs and improve template support.

Overall looks good! Thanks for working on this!

Jan 25 2018, 2:29 AM · Restricted Project

Jan 24 2018

xazax.hun added a comment to D42301: [ASTImporter] Support LambdaExprs and improve template support.

I'd rather create a separate patch - this one is already large enough. Is it OK?

Jan 24 2018, 1:18 PM · Restricted Project

Jan 23 2018

xazax.hun updated the diff for D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..
  • Added test for CXXTypeIdExpr import.
Jan 23 2018, 7:13 AM
xazax.hun updated the diff for D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..
  • Added import for CXXTypeidExpr. What is the best way to test this? <typeinfo> header is required for using the typeid operator, but relying on the presence of an STL library in tests is usually considered as a bad practice. Should we mock a typeinfo implementation just for this purpose?
Jan 23 2018, 6:48 AM
xazax.hun added a comment to D42301: [ASTImporter] Support LambdaExprs and improve template support.

I do not see a test for the following changes:

  • ASTImporter: don't add templated declarations into DeclContext

It's in ASTImporterTest. It checks that the templated decl cannot be found in the enclosing TU.

Jan 23 2018, 12:36 AM · Restricted Project
xazax.hun added a comment to D42301: [ASTImporter] Support LambdaExprs and improve template support.

I do not see a test for the following changes:

  • ASTImporter: don't add templated declarations into DeclContext
  • ASTImporter: proper set ParmVarDecls for imported FunctionProtoTypeLoc
Jan 23 2018, 12:22 AM · Restricted Project
xazax.hun added a comment to D42301: [ASTImporter] Support LambdaExprs and improve template support.

High level note: clang::TypeAliasDecl has the same issue as CXXRecordDecl, so templated versions should not be added to the DeclContext. Could you add that just for the sake of completeness?

Jan 23 2018, 12:01 AM · Restricted Project

Jan 22 2018

xazax.hun added a comment to D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..

Hello Peter,

Thank you for the patch! It is almost LGTM, just a few minor questions inline.
Am I understand correctly that it is partially based on https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp?
Also, FYI: you can use ASTMerge and clang-import-test facilities for testing. ASTMatchers are completely fine, but usage of imported AST samples as Sema lookup source can uncover some subtle issues in the built AST.

Jan 22 2018, 2:27 PM
xazax.hun updated the diff for D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..
  • Address review comments.
Jan 22 2018, 2:25 PM