NoQ (Artem Dergachev)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 3 2015, 9:16 AM (110 w, 5 d)

Recent Activity

Mon, Oct 16

NoQ added a comment to D38921: [analyzer] LoopUnrolling: update the matched assignment operators.

Neat! Tests?

Mon, Oct 16, 9:02 AM

Fri, Oct 13

NoQ abandoned D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block..
Fri, Oct 13, 2:44 AM
NoQ created D38877: [analyzer] RetainCount: Accept "safe" CFRetain wrappers..
Fri, Oct 13, 1:57 AM

Thu, Oct 12

NoQ added a comment to D38728: [analyzer] Use the signature of the primary template for issue hash calculation.

The other way round, i guess. I like the test change, it's easier to understand, so it's better to have it before starting to understand :)

Thu, Oct 12, 2:01 AM
NoQ added a comment to D38728: [analyzer] Use the signature of the primary template for issue hash calculation.

Ideas behind both hashing change and new testing mechanism look great to me.

Thu, Oct 12, 1:56 AM

Wed, Oct 11

NoQ updated the diff for D23963: [analyzer] pr28449 - Move literal rvalue construction away from RegionStore..

Because i didn't get back to this in a while, and similar crashes keep coming, i decided to leave this refactoring as a FIXME.

Wed, Oct 11, 8:25 AM
NoQ created D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type..
Wed, Oct 11, 7:42 AM
NoQ added inline comments to D38797: [analyzer] CStringChecker: pr34460: Admit that some casts are hard to model..
Wed, Oct 11, 6:29 AM
NoQ created D38797: [analyzer] CStringChecker: pr34460: Admit that some casts are hard to model..
Wed, Oct 11, 6:20 AM

Tue, Oct 10

NoQ added inline comments to D38673: [analyzer] MisusedMovedObjectChecker: Fix false positive on state-resetting, handling method calls on base-class sub-objects.
Tue, Oct 10, 6:21 AM
NoQ added a comment to D38675: [analyzer] MisusedMovedObjectChecker: Moving the checker out of alpha state.

Last time i was running on WebKit; i already lost my results, so i'd try to reproduce the results on the fixed checker and follow up. Apart from D31538, i've seen a few cases where a method was safe to be called on a moved-from object (which led me to believe that we'd need to be safer here), and a few weird cases where a moved-from object was accidentally copied implicitly, which seemed to be a non-issue.

Tue, Oct 10, 2:51 AM

Wed, Oct 4

NoQ added a comment to D35216: [analyzer] Escape symbols when creating std::initializer_list..

This is precisely how the rest of the compiler handles CXXStdInitializerListExpr

Wow. Cool. I'd see what I can do. Yeah, it seems that this is a great case for us to pattern-match the implementations as well (the problems are still there for other STL stuff).

Wed, Oct 4, 11:00 AM
NoQ added inline comments to D35216: [analyzer] Escape symbols when creating std::initializer_list..
Wed, Oct 4, 10:59 AM
NoQ updated the diff for D35216: [analyzer] Escape symbols when creating std::initializer_list..

Escape into array and dictionary literals, add relevant tests. Fix the null statement check.

Wed, Oct 4, 10:58 AM
NoQ added a comment to D35216: [analyzer] Escape symbols when creating std::initializer_list..

This is precisely how the rest of the compiler handles CXXStdInitializerListExpr

Wow. Cool. I'd see what I can do. Yeah, it seems that this is a great case for us to pattern-match the implementations as well (the problems are still there for other STL stuff).

Wed, Oct 4, 10:56 AM
NoQ added inline comments to D38358: [analyzer] Fix autodetection of getSVal()'s type argument..
Wed, Oct 4, 10:48 AM
NoQ added a comment to D38358: [analyzer] Fix autodetection of getSVal()'s type argument..

Whoops forgot to submit inline comments.

Wed, Oct 4, 8:59 AM

Tue, Oct 3

NoQ accepted D38487: [Analyzer] More granular special casing in RetainCountChecker.

Yep, looks good.

Tue, Oct 3, 8:44 AM

Mon, Oct 2

NoQ updated the diff for D38358: [analyzer] Fix autodetection of getSVal()'s type argument..

Yeah, nice catch. So we need to either always tell the checkers to specify their CharTy when they are dealing with void pointers, or to do our substitution consistently, not only for SymbolicRegion but also for AllocaRegion (did that in this diff).

Mon, Oct 2, 8:58 AM
NoQ added inline comments to D35216: [analyzer] Escape symbols when creating std::initializer_list..
Mon, Oct 2, 8:02 AM

Thu, Sep 28

NoQ updated the diff for D38358: [analyzer] Fix autodetection of getSVal()'s type argument..

Add @alexfh's small reproducer test case. It was so small i never noticed it until now!

Thu, Sep 28, 8:03 AM
NoQ updated the diff for D38358: [analyzer] Fix autodetection of getSVal()'s type argument..

Add a forgotten comment.

Thu, Sep 28, 7:21 AM
NoQ created D38358: [analyzer] Fix autodetection of getSVal()'s type argument..
Thu, Sep 28, 7:21 AM

Wed, Sep 27

NoQ abandoned D37255: [analyzer] Fix bugreporter::getDerefExpr() again - a smaller targeted fix..
Wed, Sep 27, 2:39 AM

Tue, Sep 26

NoQ updated the diff for D37023: [analyzer] Fix bugreporter::getDerefExpr() again..

Add no-crash test cases from https://bugs.llvm.org/show_bug.cgi?id=34373 and https://bugs.llvm.org/show_bug.cgi?id=34731 .

Tue, Sep 26, 3:30 PM

Mon, Sep 25

NoQ updated the diff for D37023: [analyzer] Fix bugreporter::getDerefExpr() again..

@dcoughlin: You're right, my reasoning and understanding was not correct, and your explanation is much more clear. My code still makes sense to me though, so i updated the comments to match. And moved the unusual logic for the lvalue-to-rvalue cast unwrap to the bottom of the function.

Mon, Sep 25, 6:24 AM
NoQ updated the diff for D36737: [analyzer] Store design discussions in docs/analyzer for future use..

Update to use .rst formatting.

Mon, Sep 25, 5:03 AM
NoQ updated the diff for D35216: [analyzer] Escape symbols when creating std::initializer_list..

Fix some comments in tests.

Mon, Sep 25, 5:03 AM
NoQ added a comment to D38214: [analyzer] Fix crash on modeling of pointer arithmetic.

Looks good!

Mon, Sep 25, 5:03 AM
NoQ accepted D38214: [analyzer] Fix crash on modeling of pointer arithmetic.
Mon, Sep 25, 5:03 AM

Tue, Sep 19

NoQ added a comment to D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written.

The overall idea makes sense to me. I'd like you to join the effort with Peter who during his work on loop widening came up with a matcher-based procedure for finding out if a variable is changed anywhere; it currently lives in LoopUnrolling.cpp and we need only once implementation of that.

Tue, Sep 19, 11:07 AM
NoQ accepted D37840: [Analyzer] Synthesize function body for call_once.

Yeah, sounds good to me as well.

Tue, Sep 19, 10:57 AM
NoQ accepted D37910: [Analyzer] Log when auto-synthesized body is used.

*approves more active use of DEBUG()*

Tue, Sep 19, 9:01 AM

Sep 17 2017

NoQ created D37963: [analyzer] PthreadLock: Don't track dead regions..
Sep 17 2017, 1:39 PM
NoQ updated the diff for D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC..

Remove the changes in tests for now. I guess they'd need more cleanup anyway.

Sep 17 2017, 1:02 PM
NoQ added inline comments to D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions..
Sep 17 2017, 12:49 PM

Sep 16 2017

NoQ added a comment to D37908: [Analyzer] Check function name size before performing indexing.

Neat! I've seen this code multiple times but never noticed this bug.

Sep 16 2017, 1:19 AM
NoQ added a comment to D37935: [Analyzer] Use the same filename for the header and the implementation of BugReporterVisitor.

The header declares and the cpp file defines multiple visitors. However, unless you are the BugReporter, you only care about the possibility of defining their own visitor, and don't care about re-using other visitors. I guess that's how these names came to be.

Sep 16 2017, 1:16 AM

Sep 14 2017

NoQ added inline comments to D37840: [Analyzer] Synthesize function body for call_once.
Sep 14 2017, 8:58 AM

Sep 13 2017

NoQ created D37812: [analyzer] PthreadLock: Escape the pointers..
Sep 13 2017, 9:16 AM
NoQ added a dependent revision for D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.: D37812: [analyzer] PthreadLock: Escape the pointers..
Sep 13 2017, 9:16 AM
NoQ added a dependency for D37812: [analyzer] PthreadLock: Escape the pointers.: D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC..
Sep 13 2017, 9:16 AM
NoQ added a dependent revision for D37805: [analyzer] PthreadLock: add printState().: D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions..
Sep 13 2017, 7:20 AM
NoQ added a dependency for D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.: D37805: [analyzer] PthreadLock: add printState()..
Sep 13 2017, 7:20 AM
NoQ added a dependency for D37807: [analyzer] PthreadLock: Add the other XNU rwlock unlock functions.: D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions..
Sep 13 2017, 7:20 AM
NoQ added a dependent revision for D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.: D37807: [analyzer] PthreadLock: Add the other XNU rwlock unlock functions..
Sep 13 2017, 7:20 AM
NoQ added a dependency for D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC.: D37807: [analyzer] PthreadLock: Add the other XNU rwlock unlock functions..
Sep 13 2017, 7:19 AM
NoQ added a dependent revision for D37807: [analyzer] PthreadLock: Add the other XNU rwlock unlock functions.: D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC..
Sep 13 2017, 7:19 AM
NoQ updated the diff for D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC..

Don't forget to check that the function is a global C function in post-call.

Sep 13 2017, 7:19 AM
NoQ created D37809: [analyzer] PthreadLock: Refactor, use PostCall API. NFC..
Sep 13 2017, 7:04 AM
NoQ created D37807: [analyzer] PthreadLock: Add the other XNU rwlock unlock functions..
Sep 13 2017, 6:44 AM
NoQ created D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions..
Sep 13 2017, 6:39 AM
NoQ created D37805: [analyzer] PthreadLock: add printState()..
Sep 13 2017, 6:35 AM

Sep 8 2017

NoQ added a comment to D37596: [Analyzer] Document a gotcha: for C++ -analyze-function requires parameters in function name.

Or maybe just add an example with blocks below your text?:

Sep 8 2017, 1:57 AM
NoQ added a comment to D37596: [Analyzer] Document a gotcha: for C++ -analyze-function requires parameters in function name.

Yeah, in D22856 i intended -analyze-function to work in pair with -analyzer-display-progress - you can always copy-paste the fully qualified name from there, even if it's an ObjC method or even an anonymous block. But i guess i failed to explain this in D22874.

Sep 8 2017, 1:41 AM

Sep 6 2017

NoQ added a comment to D37478: [analyzer] Implement pointer arithmetic on constants.

I've seen this recently, and while i agree that the fix is correct, i'm not entirely sure that the test cases are correct. As weird as this may sound, null dereference is not an attempt to read from or write to memory address 0. Instead, it is about using a null pointer as if it was pointing to an actual object in memory, even if accessing it by a non-zero offset. For example, in

struct S {
  int x, y;
};
Sep 6 2017, 4:55 AM
NoQ added a comment to D37500: [CSA] Move AnalysisContext.h to AnalysisDeclContext.h.

Heh, i never noticed. Thanks.

Sep 6 2017, 2:08 AM

Sep 4 2017

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

In the future probably it would be better to alter the signature of the checkers' constructor to set the name in the constructor so it is possible to create the BugType eagerly.

Sep 4 2017, 7:42 AM

Sep 1 2017

NoQ planned changes to D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block..

This is all wrong. While RetainCountChecker is more function-local than, say, MallocChecker, we still can't say for sure that it is the bottom frame's function (or block) that should be owning the object in this case. Ideally it should, but that's not the pattern that the checker is de facto trying to find. The checker is usually fine seeing a pointer allocated in a top function and released in a sub-block. If the bottom frame is over-releasing, we'd warn, but it wouldn't be simply because the bottom frame is over-releasing, so mentioning the particular stack frame may end up being misleading.

Sep 1 2017, 6:51 AM

Aug 30 2017

NoQ added a comment to D35216: [analyzer] Escape symbols when creating std::initializer_list..

The CXXStdInitializerListExpr node has pretty simple evaluation semantics: it takes a glvalue array expression, and constructs a std::initializer_list<T> from it as if by filling in the two members with a pointer to the array and either the size of the array or a pointer to its end. Can we not model those semantics directly here?

Aug 30 2017, 3:22 AM

Aug 29 2017

NoQ updated the diff for D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block..

Avoid creating a new RefVal kind.

Aug 29 2017, 9:12 AM
NoQ updated the diff for D35216: [analyzer] Escape symbols when creating std::initializer_list..

Fix a bit of ObjCBoxedExpr behavior.

Aug 29 2017, 8:54 AM
NoQ added inline comments to D35216: [analyzer] Escape symbols when creating std::initializer_list..
Aug 29 2017, 8:49 AM
NoQ created D37255: [analyzer] Fix bugreporter::getDerefExpr() again - a smaller targeted fix..
Aug 29 2017, 2:22 AM

Aug 28 2017

NoQ added inline comments to D36708: [analyzer] Fix Bug34144-[MallocChecker] MallocChecker::MallocUpdateRefState(): Assertion `Sym' failed..
Aug 28 2017, 7:55 AM
NoQ added a comment to D37189: Fix an assertion failure that occured when custom 'operator new[]' return non-ElementRegion and 'c++-allocator-inlining' sets true..

I believe a custom operator new (or new[], regardless) can always return anything it wants. It can return a pointer to a concrete global variable, for example. So the only thing we can assert is that our operator is custom.

Aug 28 2017, 7:55 AM
NoQ added a comment to D36708: [analyzer] Fix Bug34144-[MallocChecker] MallocChecker::MallocUpdateRefState(): Assertion `Sym' failed..

We can probably land D37189 first, and then land this patch with a test?

Aug 28 2017, 7:50 AM
NoQ accepted D37120: [analyzer] Fix modeling arithmetic.

Thanks, LGTM!

Aug 28 2017, 7:35 AM
NoQ added inline comments to D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true.
Aug 28 2017, 7:27 AM
NoQ added a comment to D37023: [analyzer] Fix bugreporter::getDerefExpr() again..

Thank you for the review!

Aug 28 2017, 4:04 AM
NoQ accepted D37181: {StaticAnalyzer} LoopUnrolling: Keep track the maximum number of steps for each loop.
Aug 28 2017, 3:39 AM
NoQ added a comment to D37181: {StaticAnalyzer} LoopUnrolling: Keep track the maximum number of steps for each loop.

LGTM, thanks! Minor nit included. I hope our matchers actually cover all the cases (not a big deal if they don't though).

Aug 28 2017, 1:54 AM
NoQ accepted D36962: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (make more branches).

I guess later it'd be great to comment on every numTimesReached() why is it supposed to be reached exactly that many times, as it's often unclear, especially with tricky control flow. Because if somebody accidentally breaks the test, they'd have no idea what it tests or why it should work this way.

Aug 28 2017, 1:08 AM
NoQ accepted D37103: [StaticAnalyzer] LoopUnrolling fixes.

LGTM!

Aug 28 2017, 1:03 AM

Aug 26 2017

NoQ added a comment to D37120: [analyzer] Fix modeling arithmetic.

Yeah, LocAsInteger is supported very poorly in most places. We can only get away with it because people rarely cast pointers to integers.

Aug 26 2017, 2:15 AM

Aug 25 2017

NoQ added a comment to D37120: [analyzer] Fix modeling arithmetic.

I guess we'd need more assertions to catch invalid symbols, will have a look.

Aug 25 2017, 4:27 AM

Aug 24 2017

NoQ accepted D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way.

All right then, i approve!

Aug 24 2017, 7:48 AM
NoQ added a comment to D37103: [StaticAnalyzer] LoopUnrolling fixes.

Yeah, looks good.

Aug 24 2017, 7:32 AM
NoQ added a comment to D36962: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (make more branches).

The heuristic makes perfect sense to me.

Aug 24 2017, 7:27 AM
NoQ added a comment to D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal.

I see, it seems that we've constructed $x - 1 somewhere, where $x is a pointer, while such stuff normally looks as &element{SymRegion{$x}, -1}. I guess i'd have to take a more careful look at it soon.

Aug 24 2017, 7:12 AM

Aug 23 2017

NoQ added a comment to D37023: [analyzer] Fix bugreporter::getDerefExpr() again..

Yeah, line 86.

Aug 23 2017, 1:32 AM

Aug 22 2017

NoQ added inline comments to D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr()..
Aug 22 2017, 2:10 PM
NoQ added a dependent revision for D37023: [analyzer] Fix bugreporter::getDerefExpr() again.: D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr()..
Aug 22 2017, 2:09 PM
NoQ added a dependency for D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr().: D37023: [analyzer] Fix bugreporter::getDerefExpr() again..
Aug 22 2017, 2:09 PM
NoQ created D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr()..
Aug 22 2017, 2:08 PM
NoQ created D37023: [analyzer] Fix bugreporter::getDerefExpr() again..
Aug 22 2017, 2:01 PM
NoQ added a comment to D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way.

Thanks, works now! Apart from the minor comment on the hanging header file in the tests, this looks good and i have no further nits :)

Aug 22 2017, 12:09 AM

Aug 21 2017

NoQ added a comment to D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way.

Most importantly, do you think we can enable the checker by default now? Was this checker evaluated on any large codebase, and if so, how many true/false positives did it find, probably compared to the old checker?

Aug 21 2017, 7:02 AM
NoQ added a comment to D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way.

Tests are still not working - they pass now, but they don't actually test anything. Please make sure that tests actually work. Which means that

  1. Tests pass when you run make -j4 check-clang-analysis;
  2. Tests start failing when you change your code or expected-warning directives.
Aug 21 2017, 6:34 AM

Aug 19 2017

NoQ added a comment to D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way.

First of all, thanks everybody for working on this. I'd see what i can do.

Aug 19 2017, 9:44 AM

Aug 18 2017

NoQ accepted D36851: [analyzer] Fix modeling of ctors.

After the FIXME about record layout is addressed, i guess the next action item is to make sure the trivial constructor for the empty base is not evaluated conservatively (doh!). Because whatever bindings we make, for now they'd be blown away immediately during "construction".

Aug 18 2017, 1:25 AM

Aug 17 2017

NoQ added a comment to D36708: [analyzer] Fix Bug34144-[MallocChecker] MallocChecker::MallocUpdateRefState(): Assertion `Sym' failed..

Aha, right, makes sense. Yeah, there must be more problems with this mode. Which doesn't make the current patch less useful :)

Aug 17 2017, 8:39 AM

Aug 15 2017

NoQ created D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block..
Aug 15 2017, 8:55 AM
NoQ created D36737: [analyzer] Store design discussions in docs/analyzer for future use..
Aug 15 2017, 6:48 AM
NoQ updated the diff for D35216: [analyzer] Escape symbols when creating std::initializer_list..

Actually update the diff.

Aug 15 2017, 6:28 AM
NoQ added a comment to D35216: [analyzer] Escape symbols when creating std::initializer_list..

We've discussed it in person with Devin, and he provided more points to think about:

Aug 15 2017, 6:27 AM
NoQ added a comment to D36708: [analyzer] Fix Bug34144-[MallocChecker] MallocChecker::MallocUpdateRefState(): Assertion `Sym' failed..

Totally makes sense, thanks!

Aug 15 2017, 2:38 AM

Aug 14 2017

NoQ added a comment to D36690: [StaticAnalyzer] LoopWidening: Invalidate only the possibly changed regions.

Do i understand correctly that your code does widening for simple loops but leaves complex loops intact, unless the old widening is also turned on?

Aug 14 2017, 9:29 AM
NoQ accepted D35657: [StaticAnalyzer] LoopUnrolling: Exclude cases where the counter is escaped before the loop.

Looks great now!

Aug 14 2017, 7:09 AM
NoQ accepted D36564: [analyzer] Fix SimpleSValBuilder::simplifySVal.
Aug 14 2017, 7:06 AM