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 (278 w, 1 d)

Recent Activity

Thu, Jan 11

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

Tue, Jan 9

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.

Tue, Jan 9, 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.

Tue, Jan 9, 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?)

Tue, Jan 9, 6:36 AM
xazax.hun accepted D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr.
Tue, Jan 9, 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?

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

LG!

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

Great! Thanks!

Tue, Jan 9, 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.

Tue, Jan 9, 3:01 AM

Mon, Jan 8

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

Mon, Jan 8, 2:05 AM

Sat, Jan 6

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

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

Hi Hans!

Sat, Jan 6, 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.

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

Thu, Jan 4

xazax.hun added inline comments to D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
Thu, Jan 4, 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.
Thu, Jan 4, 10:24 AM

Wed, Dec 27

xazax.hun added inline comments to D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
Wed, Dec 27, 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.
Wed, Dec 27, 1:46 PM

Fri, Dec 22

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

In case you do not like this solution, I uploaded an alternative approach: https://reviews.llvm.org/D41538

Fri, Dec 22, 3:14 AM
xazax.hun created D41538: [analyzer] Fix some checker's output plist not containing the checker name #2.
Fri, Dec 22, 3:14 AM

Thu, Dec 21

xazax.hun accepted D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory.

LGTM!

Thu, Dec 21, 4:44 AM

Wed, Dec 20

xazax.hun closed D41451: Make DiagnosticIDs::getAllDiagnostics use std::vector.

Committed in https://reviews.llvm.org/rL321190

Wed, Dec 20, 8:57 AM
xazax.hun added a comment to D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory.

Also, I still think we should rather not apply template-related patches until this testing is implemented. Gabor, Peter, do you agree?

Wed, Dec 20, 8:13 AM
xazax.hun accepted D41451: Make DiagnosticIDs::getAllDiagnostics use std::vector.

LGTM!

Wed, Dec 20, 7:56 AM
xazax.hun added a comment to D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory.

Is it possible that this will hide other problems? Wouldn't it be better to run the tests twice once with this argument and once without it?

I don't think so. In fact, without instantiation, we are not even able to check semantic code correctness inside templates. So, we are solving this problem as well.

Wed, Dec 20, 6:50 AM
xazax.hun added a comment to D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory.

Is it possible that this will hide other problems? Wouldn't it be better to run the tests twice once with this argument and once without it?

Wed, Dec 20, 6:31 AM
xazax.hun added a comment to D41406: [analyzer] Add a new checker callback, check::NewAllocator..

Maybe debug.AnalysisOrder could be used to test the callback order explicitly. This way the test could also serve as a documentation for the callback order.

Wed, Dec 20, 6:28 AM
xazax.hun reopened D38692: [ASTImporter] Support importing FunctionTemplateDecl and CXXDependentScopeMemberExpr.

It was reverted due to tests failures on windows build bots.

Wed, Dec 20, 5:54 AM
xazax.hun added reviewers for D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy: NoQ, george.karpenkov.

In the tests there are multiple variants of the strcpy function guarded by macros. Maybe we should run the tests multiple times to test all variants with different defines?

Wed, Dec 20, 5:39 AM
xazax.hun added a comment to D39285: [clang-tidy] Misc redundant expression checker updated for ineffective bitwise operator expressions.

There was two accepts and the comments found by Alex were resolved, so I went ahead committing this. In case there are further comments, we can address them separately.

Wed, Dec 20, 4:24 AM

Tue, Dec 19

xazax.hun added inline comments to D30691: [analyzer] Support for naive cross translational unit analysis.
Tue, Dec 19, 7:11 AM
xazax.hun updated the diff for D30691: [analyzer] Support for naive cross translational unit analysis.
  • Address some review comments
  • Rebased on ToT
Tue, Dec 19, 7:10 AM
xazax.hun added inline comments to D30691: [analyzer] Support for naive cross translational unit analysis.
Tue, Dec 19, 5:29 AM

Dec 16 2017

xazax.hun added a comment to D40937: [clang-tidy] Infinite loop checker.

I think, while the analyzer is more suitable for certain kinds of checks that require deep analysis, it is still useful to have quicker syntactic checks that can easily identify problems that are the results of typos or incorrectly modified copy and pasted code. I think this check is in that category. Also, the original warning Peter mentioned does something similar but has some shortcomings.

Dec 16 2017, 10:52 AM · Restricted Project
xazax.hun added a comment to D41151: [analyzer] Adding LoopContext and improve loop modeling.

Just to be sure, this is just a refactoring to make this cleaner or you expect this to have other effects as well, like better performance?

Dec 16 2017, 10:33 AM
xazax.hun added a comment to D40560: [analyzer] Get construction into `operator new` running in simple cases..
In D40560#947514, @NoQ wrote:

Replaced the live expression hack with a slightly better approach. It doesn't update the live variables analysis to take CFGNewAllocator into account, but at least tests now pass.

In order to keep the return value produced by the operator new() call around until CXXNewExpr is evaluated, i added a program state trait, CXXNewAllocatorValueStack:

  1. Upon evaluating CFGNewAllocator, the return SVal of the evaluated allocator call is put here;
  2. Upon evaluating CXXConstructExpr, that return value is retrieved from here;
  3. Upon evaluating CXXNewExpr, the return value is retrieved from here again and then wiped.

    In order to support nested allocator calls, this state trait is organized as a stack/FIFO, with push in CFGNewAllocator and pop in CXXNewExpr. The llvm::ImmutableList thing offers some asserts for warning us when we popped more times than we pushed; we might want to add more asserts here to detect other mismatches, but i don't see a need for implementing this as an environment-like map from (Expr, LocationContext) to SVal.
Dec 16 2017, 10:29 AM
xazax.hun accepted D41250: [analyzer] Model implied cast around operator new()..

The code looks good to me. But the best way to verify these kinds of changes to see how the results change on large projects after applying the patch.

Dec 16 2017, 10:14 AM
xazax.hun accepted D41266: [analyzer] With c++-allocator-inlining, fix memory space for operator new pointers..

In the future, we might want to model the standard placement new and return a symbol with the correct SpaceRegion (i.e.: the space region of the argument).

Dec 16 2017, 9:51 AM

Dec 12 2017

xazax.hun updated the diff for D30691: [analyzer] Support for naive cross translational unit analysis.
  • Further improvements to python script part.
Dec 12 2017, 6:31 AM

Dec 8 2017

xazax.hun added inline comments to D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Dec 8 2017, 6:30 AM · Restricted Project

Dec 7 2017

xazax.hun updated the diff for D30691: [analyzer] Support for naive cross translational unit analysis.
  • @gerazo addressed some review comments in scan-build-py.
Dec 7 2017, 9:54 AM
xazax.hun added a comment to D40939: [analyzer] NFC: Avoid element regions of void type..
In D40939#948252, @NoQ wrote:

Like, it's not the situation when we couldn't figure out the type - it would have been null in that case. Here we know exactly that the type is void.

Dec 7 2017, 8:21 AM
xazax.hun added inline comments to D40937: [clang-tidy] Infinite loop checker.
Dec 7 2017, 8:15 AM · Restricted Project
xazax.hun added a comment to D40937: [clang-tidy] Infinite loop checker.

This does not support memberExprs as condition variables right now.

Dec 7 2017, 8:08 AM · Restricted Project
xazax.hun added a comment to D40939: [analyzer] NFC: Avoid element regions of void type..

I like the idea of adding those assertions but a bit worried about the other changes. Basically (if I get this right), we are masking the issues here and I am a bit afraid that they will get forgotten. I think it would be nice to at least add a FIXME that this path should never be triggered.

Dec 7 2017, 7:50 AM

Dec 4 2017

xazax.hun added inline comments to D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Dec 4 2017, 8:22 AM · Restricted Project
xazax.hun added reviewers for D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions: aaron.ballman, hokein.
Dec 4 2017, 8:20 AM · Restricted Project
xazax.hun added inline comments to D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr.
Dec 4 2017, 6:16 AM

Nov 29 2017

xazax.hun added inline comments to D40463: [analyzer] Fix false negative on post-increment of uninitialized variable..
Nov 29 2017, 10:46 AM · Restricted Project
xazax.hun added inline comments to D40463: [analyzer] Fix false negative on post-increment of uninitialized variable..
Nov 29 2017, 7:14 AM · Restricted Project

Nov 28 2017

xazax.hun accepted D40507: [clang-tidy] Move more checks from misc- to performance-.

Found one possible problem. Once fixed, LG!

Nov 28 2017, 5:40 AM
xazax.hun added inline comments to D38171: [clang-tidy] Implement clang-tidy check aliases.
Nov 28 2017, 4:47 AM
xazax.hun added a comment to D38171: [clang-tidy] Implement clang-tidy check aliases.

And, btw, sorry for the long delay. I've been on travelling / on vacation for the last few weeks.

Nov 28 2017, 4:45 AM
xazax.hun accepted D5767: Template Instantiation Observer + a few other templight-related changes.

I found some nit, otherwise LG!

Nov 28 2017, 4:24 AM

Nov 27 2017

xazax.hun added a comment to D39285: [clang-tidy] Misc redundant expression checker updated for ineffective bitwise operator expressions.

:( My initial intention was to simplify the review process by splitting the change-set into small patches, but it seems that this idea fell flat...

Nov 27 2017, 7:11 AM

Nov 26 2017

xazax.hun updated subscribers of D40463: [analyzer] Fix false negative on post-increment of uninitialized variable..
Nov 26 2017, 3:57 AM · Restricted Project

Nov 24 2017

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

@dcoughlin do you have some input on this?

Nov 24 2017, 3:04 AM

Nov 23 2017

xazax.hun added a comment to D39722: [ASTImporter] Support TypeTraitExpr Importing.

Hello Takafumi,

This is almost OK to me but there is an inline comment we need to resolve in order to avoid Windows buildbot failures.
In addition, as Gabor pointed, when we add a new matcher, we need to update matcher documentation as well. To update the docs, you should perform just three steps.

  1. Rebase your patch onto latest master
  2. Launch script docs/tools/dump_ast_matchers.py
  3. Add changes made by this script to your commit.

I'm a bit confused -- I don't see any ASTMatcher changes as part of this patch aside from what's in the test file. Are you suggesting the matcher there should be hoisted as a separate patch?

Nov 23 2017, 8:05 AM
xazax.hun accepted D40388: [clang-tidy] rename_check.py misc-string-constructor bugprone-string-constructor.

LG!

Nov 23 2017, 4:16 AM

Nov 21 2017

xazax.hun added a comment to D39285: [clang-tidy] Misc redundant expression checker updated for ineffective bitwise operator expressions.

This patch needs to be rebased off trunk before it can be applied cleanly.

Nov 21 2017, 11:40 AM
xazax.hun requested changes to D39722: [ASTImporter] Support TypeTraitExpr Importing.

The checker documentation should be updated as well.

Nov 21 2017, 6:42 AM
xazax.hun added a comment to D30876: [ASTImporter] Unnamed structs import.

Ok, https://reviews.llvm.org/D39886 is an independent fix for the same issue. It might be useful to add the test cases from that patch to this one as Aleksei pointed out.
The current status of this one, the windows build bot failed, but we could not reproduce it (even on windows). Is there someone who could reproduce the fail? Should we just recommit this hoping that the fail was unrelated to this patch?

Nov 21 2017, 6:39 AM
xazax.hun requested changes to D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2.
> So what are the arguments that are passed to getSimplifiedOffset() in that case? 0? That does not seem to be correct.

yes.

Nov 21 2017, 4:14 AM
xazax.hun accepted D39243: [clang-tidy] Misc redundant expressions checker updated for overloaded operators.

Other than two nits it looks good to me. Wait one more reviewer just in case.

Nov 21 2017, 3:38 AM

Nov 17 2017

xazax.hun accepted D38921: [analyzer] LoopUnrolling: update the matched assignment operators.

A nit, otherwise LG!

Nov 17 2017, 9:31 AM
xazax.hun added a comment to D39438: [analyzer] Diagnose stack leaks via block captures.

I found some nits, but overall I think this is getting close.

Nov 17 2017, 6:57 AM
xazax.hun added inline comments to D34812: [StaticAnalyzer] LoopUnrolling: Handling conditions with known value variables.
Nov 17 2017, 6:43 AM

Nov 16 2017

xazax.hun added a comment to D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2.

Could you do a similar analysis that I did above to check why does this not work for the multidimensional case? (I.e.: checking what constraints are generated and what the analyzer does with them.)

Expressions:
rawOffsetVal => 0

extentBegin => 0

For getSimplifiedOffset() , the offset is not a SymIntExpr it will just return 0.

Nov 16 2017, 2:18 AM
xazax.hun added a comment to D39965: [Analyzer] Split Critical Sections.

There are valid cases to use short but different critical sections within the same function for the same mutex.
For instance, one implementation of a readers-writer lock uses two short critical sections in the read side (https://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock#Using_two_mutexes)

Nov 16 2017, 2:00 AM
xazax.hun added a comment to D35109: [Analyzer] SValBuilder Comparison Rearrangement.

So still the options are to fix it in the checker or fix it in the engine (the max/4 or the type extension solution), but leaving it unfixed is not an option. I am open to any solution, but only full solutions and no partial solutions, because they will not help us enough.

Nov 16 2017, 1:09 AM

Nov 15 2017

xazax.hun added inline comments to D35068: [analyzer] Detect usages of unsafe I/O functions.
Nov 15 2017, 5:39 AM
xazax.hun added a comment to D38171: [clang-tidy] Implement clang-tidy check aliases.

One problem to think about when we add all clang-diagnostic as "first or second" class citizen, checkes=* might now enable all the warnings which make no sense and might be surprising to the users. What do you think?

This is a good point. Should I insert ",-clang-diagnostic*" after any (positive) * ?

Nov 15 2017, 3:16 AM
xazax.hun added inline comments to D5767: Template Instantiation Observer + a few other templight-related changes.
Nov 15 2017, 1:47 AM

Nov 14 2017

xazax.hun added a comment to D39965: [Analyzer] Split Critical Sections.

Do you expect false positives with this check? Are there cases where the user deliberately wants to break a long critical section to read an updated value (possibly by other thread) in the next one?

Nov 14 2017, 7:49 AM
xazax.hun accepted D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true.

LGTM! But wait for @dcoughlin, @zaks.anna , or @NoQ before commit.

Nov 14 2017, 7:17 AM
xazax.hun added a comment to D39965: [Analyzer] Split Critical Sections.

Just some quick comments I only checked the first few lines.

Nov 14 2017, 4:44 AM
xazax.hun added a comment to D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2.
In D39049#910482, @NoQ wrote:
// TODO: once the constraint manager is smart enough to handle non simplified
// symbolic expressions remove this function. Note that this can not be used in
// the constraint manager as is, since this does not handle overflows. It is
// safe to assume, however, that memory offsets will not overflow.

Wasn't safe enough, i guess.

Nov 14 2017, 4:24 AM

Nov 13 2017

xazax.hun accepted D39247: [ASTImporter] TypeAliasTemplate and PackExpansion importing capability.

LGTM too!

Nov 13 2017, 9:13 AM · Restricted Project
xazax.hun added inline comments to D39247: [ASTImporter] TypeAliasTemplate and PackExpansion importing capability.
Nov 13 2017, 6:21 AM · Restricted Project
xazax.hun accepted D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures..

LGTM!

Nov 13 2017, 3:17 AM
xazax.hun accepted D39803: [analyzer] pr34766: Fix a crash on explicit construction of std::initializer_list..

LGTM!

Nov 13 2017, 3:15 AM
xazax.hun added a comment to D38921: [analyzer] LoopUnrolling: update the matched assignment operators.

I agree it might be useful to expose this matcher to everybody.

Nov 13 2017, 3:10 AM
xazax.hun added inline comments to D39247: [ASTImporter] TypeAliasTemplate and PackExpansion importing capability.
Nov 13 2017, 3:10 AM · Restricted Project
xazax.hun accepted D39372: Make DiagnosticIDs::getAllDiagnostics static..

LGTM!

Nov 13 2017, 3:07 AM
xazax.hun accepted D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast.

LGTM!

Nov 13 2017, 3:05 AM
xazax.hun added a reviewer for D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures: doug.gregor.

Doug added anonymous structure handling, added as a reviewer in case he wants to have a look.

Nov 13 2017, 1:55 AM

Nov 8 2017

xazax.hun added inline comments to D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast.
Nov 8 2017, 10:37 PM

Nov 7 2017

xazax.hun added a comment to D33722: [clang-tidy] Add checker for undelegated copy of base classes.

Also, bugprone might be a better module to put this?

I don't have strong opinions on misc vs bugprone (they're both effectively catch-alls for tidy checks, as best I can tell).

Nov 7 2017, 6:29 AM · Restricted Project
xazax.hun updated the diff for D33722: [clang-tidy] Add checker for undelegated copy of base classes.
  • Fix doc comments that I overlooked earlier
Nov 7 2017, 6:29 AM · Restricted Project
xazax.hun updated the diff for D33722: [clang-tidy] Add checker for undelegated copy of base classes.
  • Fix review comments
Nov 7 2017, 6:26 AM · Restricted Project
xazax.hun added inline comments to D39243: [clang-tidy] Misc redundant expressions checker updated for overloaded operators.
Nov 7 2017, 5:07 AM
xazax.hun accepted D39285: [clang-tidy] Misc redundant expression checker updated for ineffective bitwise operator expressions.

LGTM! But wait one more reviewer to be sure :)

Nov 7 2017, 5:00 AM
xazax.hun added a comment to D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast.

@xazax.hun Would you be willing to take a look?

Nov 7 2017, 4:58 AM
xazax.hun accepted D39707: [analyzer] assume bitwise arithmetic axioms.

This looks like a great addition! Apart from some nits, LGTM.

Nov 7 2017, 4:45 AM

Nov 6 2017

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

Otherwise, from a nit that is probably an artifact from a rebase it looks good to me. But please wait for one more reviewer to accept this just to be sure.

Nov 6 2017, 6:19 AM
xazax.hun added inline comments to D39285: [clang-tidy] Misc redundant expression checker updated for ineffective bitwise operator expressions.
Nov 6 2017, 6:15 AM