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 (300 w, 3 d)

Recent Activity

Mon, Jun 18

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.

Mon, Jun 18, 10:50 AM

Wed, Jun 13

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

Mon, Jun 11

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!

Mon, Jun 11, 8:09 AM

Sun, Jun 10

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

Sun, Jun 10, 3:58 AM

Tue, Jun 5

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.

Tue, Jun 5, 3:03 PM

Mon, Jun 4

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).

Mon, Jun 4, 1:29 PM

Fri, Jun 1

xazax.hun added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Fri, Jun 1, 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?
Fri, Jun 1, 8:29 AM

Wed, May 30

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?

Wed, May 30, 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.

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

Sat, May 26

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

LG!

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

LGTM!

Sat, May 26, 11:55 AM
xazax.hun added inline comments to D47135: [analyzer] A checker for dangling internal buffer pointers in C++.
Sat, May 26, 11:32 AM
xazax.hun added inline comments to D47135: [analyzer] A checker for dangling internal buffer pointers in C++.
Sat, May 26, 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.

Sat, May 26, 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
xazax.hun updated the summary of D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..
Jan 22 2018, 1:48 PM

Jan 20 2018

xazax.hun created D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing..
Jan 20 2018, 9:25 AM
xazax.hun added a comment to D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr.

This should be rebased to latest master.

Jan 20 2018, 8:14 AM

Jan 11 2018

xazax.hun updated the diff for D30691: [analyzer] Support for naive cross translational unit analysis.
  • Fixed review comments
Jan 11 2018, 1:33 PM
xazax.hun added inline comments to D30691: [analyzer] Support for naive cross translational unit analysis.
Jan 11 2018, 1:33 PM
xazax.hun added inline comments to D30691: [analyzer] Support for naive cross translational unit analysis.
Jan 11 2018, 1:15 PM

Jan 9 2018

xazax.hun added a comment to D41848: [analyzer] mark returns of functions where the region passed as parameter was not initialized.

I like this patch, only found a few nits after the other's review.

Jan 9 2018, 11:52 AM
xazax.hun added a comment to D41797: [analyzer] Suppress escape of this-pointer during construction..

I am fine with suppressing the escape globally.
I did see some code in the wild where the constructors registered the objects with a (global) map.
But I think it is still easier to annotate code that does something unconventional than the other way around.

Jan 9 2018, 6:42 AM
xazax.hun added a comment to D41799: [analyzer] PtrArithChecker: Update to use check::NewAllocator.

(@xazax.hun - this is an alpha checker last touched by you, do you still have plans for it?)

Jan 9 2018, 6:36 AM
xazax.hun accepted D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr.
Jan 9 2018, 6:23 AM
xazax.hun added a comment to D41406: [analyzer] Add a new checker callback, check::NewAllocator..

Do you have a plan for the new false negatives when c++-allocator-inlining is on? Should the user mark allocation functions with attributes?

Jan 9 2018, 6:14 AM
xazax.hun accepted D41408: [analyzer] NFC: Fix nothrow operator new definition in a test..
Jan 9 2018, 5:52 AM
xazax.hun accepted D41409: [analyzer] Fix intermediate diagnostics on paths that go through operator new()..
Jan 9 2018, 5:50 AM
xazax.hun accepted D41800: [analyzer] Use a custom program point for the check::NewAllocator callback..

LG!

Jan 9 2018, 5:49 AM
xazax.hun accepted D6550: ASTImporter: Fix missing SourceLoc imports.

Great! Thanks!

Jan 9 2018, 3:05 AM
xazax.hun added a comment to D41816: [analyzer] Model and check unrepresentable left shifts.

Overall looks good to me, one comment inline. I think it is good to have these checks to prevent the analyzer executing undefined behavior. Maybe this would make it more feasible to run the analyzer with ubsan :)
In the future, it would be great to also look for these cases symbolically, but I believe it is perfectly fine to have that in a separate patch.

Jan 9 2018, 3:01 AM

Jan 8 2018

xazax.hun added a comment to D41815: [clang-tidy] implement check for goto.

A high level comment: while it is very easy to get all the gotos using grep, it is not so easy to get all the labels. So for this check to be complete, I think it would be useful to also find labels (possibly having a configuration option for that).

Jan 8 2018, 2:05 AM

Jan 6 2018

xazax.hun added a comment to D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.

It missed the 6.0 branching. Will you try to get it on this branch?
Thanks

Jan 6 2018, 4:00 AM
xazax.hun updated subscribers of rC321933: [analyzer] Fix some check's output plist not containing the check name.

Hi Hans!

Jan 6 2018, 3:03 AM
xazax.hun added a comment to D37437: [analyzer] Fix some checker's output plist not containing the checker name.

https://reviews.llvm.org/D41538 is superior and committed.

Jan 6 2018, 2:52 AM
xazax.hun abandoned D37437: [analyzer] Fix some checker's output plist not containing the checker name.
Jan 6 2018, 2:52 AM

Jan 4 2018

xazax.hun added inline comments to D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
Jan 4 2018, 10:24 AM
xazax.hun updated the diff for D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
  • Address some review comments.
Jan 4 2018, 10:24 AM

Dec 27 2017

xazax.hun added inline comments to D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
Dec 27 2017, 1:47 PM
xazax.hun updated the diff for D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
  • Address review comments.
Dec 27 2017, 1:46 PM