NoQ (Artem Dergachev)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 3 2015, 9:16 AM (118 w, 6 d)

Recent Activity

Mon, Dec 11

NoQ added a reviewer for D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?: dcoughlin.
Mon, Dec 11, 9:49 AM
NoQ updated the diff for D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?.

Add a FIXME test.

Mon, Dec 11, 9:44 AM
NoQ added a comment to D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?.

Yeah, we usually try to avoid omissions of modeling in on-by-default checkers because the user may accidentally run into projects in which the unmodeled idiom is common, and then he'd get false positives all over the place. In my case it was just two new positives, both false due to this dispatch_barrier_sync idiom.

Mon, Dec 11, 9:38 AM

Fri, Dec 8

NoQ updated the diff for D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?.

Update the other run-line.

Fri, Dec 8, 5:03 PM
NoQ updated the summary of D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?.
Fri, Dec 8, 5:00 PM
NoQ created D41042: [analyzer] StackAddrEscape: Delay turning on by default a little bit?.
Fri, Dec 8, 4:59 PM

Thu, Dec 7

NoQ added inline comments to D37189: Fix an assertion failure that occured when custom 'operator new[]' return non-ElementRegion and 'c++-allocator-inlining' sets true..
Thu, Dec 7, 4:29 PM
NoQ updated the diff for D40939: [analyzer] Avoid element regions of void type..

Rebase on top of D40584.

Thu, Dec 7, 8:49 AM
NoQ added a comment to D40939: [analyzer] Avoid element regions of void type..

I think the new behavior is correct in the sense that in our region hierarchy byte offsets (such as arithmetic on void pointers) are normally represented as char-type element regions. For instance, we have a similar mechanism is implemented in pointer casts case, when the byte offset of the pointer is not divisible by the casted object size: we just add a character element region to account for the remainder of the offset.

Thu, Dec 7, 8:15 AM

Wed, Dec 6

NoQ added a comment to D40939: [analyzer] Avoid element regions of void type..

I accidentally noticed that problem

Wed, Dec 6, 7:09 PM
NoQ created D40939: [analyzer] Avoid element regions of void type..
Wed, Dec 6, 6:49 PM
NoQ added a comment to D39965: [Analyzer] Split Critical Sections.

There's been some time i spent on (unsuccessful) static data race detection with similar approach, and one problem i never knew how to tackle was how to discriminate between variables that need to be surrounded by locks versus variables that are just accidentally surrounded by locks. Like, you may have all accesses to a variable around the whole project be surrounded by the exact same (non-empty) set of locks, but in fact all these accesses are in the same thread and the locks are here for a different reason. People often have relatively large critical sections, with a lot of stuff mixed in, and even if they can theoretically minimize the size of their critical section, and it might even be a good idea, they'd still not necessarily want to prefer this to other tasks they have, and would rather treat such positives as false.

Wed, Dec 6, 5:40 PM
NoQ updated the diff for D40560: [analyzer] WIP: Get construction into `operator new` running in simple cases..

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.

Wed, Dec 6, 5:08 PM

Tue, Dec 5

NoQ added a comment to D40809: [WIP] [analyzer] Dump counterexample traces as C programs.

So we have an ExplodedGraph, in that there's an ExplodedNode against which the report is being thrown, we have a bunch of BugReporterVisitors that walk from that node to the root of the graph and mark places they find interesting with PathDiagnosticPieces of different kinds, and PathDiagnosticConsumers (such as Plist or Html) that look at the vector of pieces and display it to the user.

Tue, Dec 5, 5:49 PM
NoQ added inline comments to D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer..
Tue, Dec 5, 3:13 PM
NoQ updated the diff for D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer..

Note that there is no constructor call here. This is aggregate initialization. And there's not really any part of this that's new, except that a class with base classes is now an a aggregate. You'll see the same kind of AST formed in all C++ language modes with a slightly modified example:

struct A {
  A();
};

struct B {
  A a;
  int x;
};

void foo() {
  B b = {};
}

The analyzer should presumably treat the new example the exact same way it treats that case.

Thank you Richard! This sheds light on what situations do we need to cover.

Tue, Dec 5, 3:09 PM
NoQ updated subscribers of D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer..

A reply from @rsmith was accidentally lost:

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171204/211785.html

(seems that phabricator cuts away all text below the first "On ..., Such-and-such wrote:" line)

Tue, Dec 5, 2:33 PM
NoQ added a comment to D40809: [WIP] [analyzer] Dump counterexample traces as C programs.
In D40809#945551, @NoQ wrote:

If you want to turn this into a form of user-facing analyzer output variant, you may want to implement it as a PathDiagnosticConsumer rather than a visitor.

Tue, Dec 5, 1:11 PM
NoQ added a comment to D40809: [WIP] [analyzer] Dump counterexample traces as C programs.

This looks great for understanding what exactly is going on in a large real-world report.

Tue, Dec 5, 1:07 PM
NoQ updated the diff for D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type..

Rebase on top of D39862 (attn. George!~).

Tue, Dec 5, 9:36 AM
NoQ updated the diff for D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer..

Add a forgotten //no-warning.

Tue, Dec 5, 9:11 AM
NoQ updated the summary of D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer..
Tue, Dec 5, 9:09 AM
NoQ created D40841: [analyzer] Fix a crash on C++17 AST for non-trivial construction into a trivial brace initializer..
Tue, Dec 5, 9:08 AM

Mon, Dec 4

NoQ accepted D39709: [analyzer] [NFC] remove duplicated function.

Yep, looks good.

Mon, Dec 4, 11:15 AM
NoQ added inline comments to D40584: [analyzer] do not crash on subscripts into ObjC properties.
Mon, Dec 4, 11:05 AM
NoQ created D40793: [analyzer] Improve SymbolicRegion::dump() for heap pointers..
Mon, Dec 4, 10:34 AM

Tue, Nov 28

NoQ added a comment to D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions..

Hey wb! Get well :)

Tue, Nov 28, 9:11 AM
NoQ added a comment to D35109: [Analyzer] SValBuilder Comparison Rearrangement.

The reason why i don't want to commit the MAX/4 approach now (for >/< case) is that it has too little useful effects until the iterator checker is enabled by default. However, it is a core change that immediately affects all users with all its negative effects (such as performance and code complexity). When i say that (1) this approach has little useful effects and (2) this approach may cause performance issues, both (1) and (2) are only based on intuition (my or Devin's). If somebody investigates the impact of the MAX/4 change and shows that both concerns are in fact wrong (the approach is indeed very useful for all modeling and/or has negligible performance impact), i think it should land. Otherwise, i think it shouldn't land now, but delayed until the iterator checker is ready to be enabled by default.

Tue, Nov 28, 9:02 AM
NoQ added a comment to D40560: [analyzer] WIP: Get construction into `operator new` running in simple cases..

for the sake of this proof-of-concept, i've crudely disabled garbage collection on the respective moments of time

Tue, Nov 28, 8:13 AM
NoQ created D40560: [analyzer] WIP: Get construction into `operator new` running in simple cases..
Tue, Nov 28, 8:00 AM

Mon, Nov 27

NoQ added a comment to D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures..

Sorry, i wanted to quickly look at why this thing is lvalue, but this didn't seem to be happening, so i guess i'd just commit for now.

Mon, Nov 27, 9:33 AM

Tue, Nov 21

NoQ added a comment to D40073: [Analyzer] Non-determinism: don't sort indirect goto LabelDecl's by addresses.

Thank you for the patch!~

Tue, Nov 21, 3:22 AM

Mon, Nov 20

NoQ accepted D39438: [analyzer] Diagnose stack leaks via block captures.

Thanks, looks great, please commit!~

Mon, Nov 20, 7:35 AM

Wed, Nov 15

NoQ added a comment to D35109: [Analyzer] SValBuilder Comparison Rearrangement.

The unconstrained rearrangements for +/-/==/!= are definitely good to go regardless of anything else.

Wed, Nov 15, 5:40 AM

Nov 10 2017

NoQ added a comment to D39438: [analyzer] Diagnose stack leaks via block captures.

Looks great, thanks! Minor inline stuff.

Nov 10 2017, 12:01 AM

Nov 9 2017

NoQ accepted D39543: [analyzer] Document the issue hash debugging facility.

these kinds of tasks should have the same level of attention as others

Nov 9 2017, 11:41 PM
NoQ added a comment to D39862: [analyzer] do not crash when trying to convert an APSInt to an unexpected type.

Of course I'm new, but I disagree with this statement: in order to have a robust API, the function should not crash, unless the caller violates an explicit precondition.
getSVal is just a function for getting a symbolic value for a particular statement, it seems totally valid to query it for an expression which type is void.

Nov 9 2017, 11:29 PM
NoQ added a comment to D39862: [analyzer] do not crash when trying to convert an APSInt to an unexpected type.

I'm curious if the crash would turn into an assertion failure during getRawSVal() after D38801 is committed.

Nov 9 2017, 1:20 PM

Nov 8 2017

NoQ added a comment to D39803: [analyzer] pr34766: Fix a crash on explicit construction of std::initializer_list..

I think I lack context to completely get what is going on here: I assume we don't model the assignment here?

Nov 8 2017, 11:39 PM
NoQ created D39803: [analyzer] pr34766: Fix a crash on explicit construction of std::initializer_list..
Nov 8 2017, 8:03 AM
NoQ created D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures..
Nov 8 2017, 6:53 AM

Nov 7 2017

NoQ accepted D39707: [analyzer] assume bitwise arithmetic axioms.

Yep, nice and clean~

Nov 7 2017, 6:04 AM
NoQ updated the diff for D39682: [analyzer] Fix a crash on logical operators with vectors..

A better name for the test function.

Nov 7 2017, 2:13 AM

Nov 6 2017

NoQ updated the diff for D39682: [analyzer] Fix a crash on logical operators with vectors..

Yep, right!

Nov 6 2017, 5:25 AM
NoQ added a comment to D35109: [Analyzer] SValBuilder Comparison Rearrangement.

How to find the N if we only use == or !=?

Nov 6 2017, 4:50 AM
NoQ created D39682: [analyzer] Fix a crash on logical operators with vectors..
Nov 6 2017, 4:45 AM
NoQ added a comment to D35109: [Analyzer] SValBuilder Comparison Rearrangement.

A breakthrough with credit going to Devin: Note that whenever we're not dealing with >/</<=/>= (but only with additive ops and == and !=, and we have everything of the same type) we can rearrange regardless of constraints, simply because Z/nZ is an abelian group.

Nov 6 2017, 3:48 AM

Nov 2 2017

NoQ added a comment to D39543: [analyzer] Document the issue hash debugging facility.

Yup, thanks!

Nov 2 2017, 5:16 AM

Oct 31 2017

NoQ added a comment to D35109: [Analyzer] SValBuilder Comparison Rearrangement.
In D35109#837723, @NoQ wrote:

It's something similar to assuming that the string length is within range [0, INT_MAX/4] in CStringChecker: we can easily assume that no overflow is happening in computations involving string lengths or iterator positions, but not on generic integers. Which lead me to believing that we could maintain a no-overflow variant of evalBinOp (questionable).

Would anything go wrong if we only enable this code when both symbols are known to be within range [-max/4, max/4]? And in the checker, add the respective assumption. I believe it's a very clear way to express that no overflow is happening. In fact, in the program state we can add an API ProgramStateRef assumeNoOverflow(SVal, QualType), which tries to assume that the value is within range [-max/4, max/4] for signed types or [0, max/4] for unsigned types (and fails when such assumption is known be violated), so that to avoid duplicating similar trick in every checker.

Oct 31 2017, 12:07 PM
NoQ added a comment to D39438: [analyzer] Diagnose stack leaks via block captures.

What i was trying to say is - Hey this check looks useful!~ Great idea.

Oct 31 2017, 8:13 AM
NoQ added inline comments to D39438: [analyzer] Diagnose stack leaks via block captures.
Oct 31 2017, 8:12 AM
NoQ updated the summary of D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type..
Oct 31 2017, 7:40 AM
NoQ added a comment to D38844: [analyzer] Make issue hash related tests more concise.

Hey, i just recalled that we have documentation for ExprInspection functions in docs/analyzer/DebugChecks.rst, you may want to add your function there as well :)

Oct 31 2017, 7:35 AM

Oct 30 2017

NoQ added inline comments to D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions..
Oct 30 2017, 9:13 AM
NoQ created D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions..
Oct 30 2017, 9:12 AM
NoQ added a comment to D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written.

however as far as I see this will mean the LoopUnroller AST matchers can't be reused unless I change them.

Oct 30 2017, 7:55 AM
NoQ accepted D37935: [Analyzer] Use the same filename for the header and the implementation of BugReporterVisitor.

Yep all right with me :)

Oct 30 2017, 5:05 AM
NoQ accepted D38844: [analyzer] Make issue hash related tests more concise.

Great stuff!

Oct 30 2017, 4:49 AM
NoQ added a comment to D38921: [analyzer] LoopUnrolling: update the matched assignment operators.

Thanks Gabor, i didn't think about it but it looks very nice to have such matcher.

Oct 30 2017, 4:43 AM
NoQ added a comment to D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type..

Yeah, cleaning up this API would be great - as long as everybody loves to have the API broken and rewrite stuff.

Oct 30 2017, 4:34 AM
NoQ added a comment to D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2.
// 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.
Oct 30 2017, 3:37 AM
NoQ added a comment to D38986: [Analyzer] Better unreachable message in enumeration.

Does llvm_unreachable() guarantee that the string construction code is completely removed from release builds?

Oct 30 2017, 3:29 AM
NoQ added a comment to D39159: [analyzer] Improves the logic of GenericTaintChecker identifying stdin..

Maybe we could also test both declaration variants, i.e.:

// RUN: %clang_analyze_cc1                  -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
// RUN: %clang_analyze_cc1 -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify
Oct 30 2017, 3:20 AM

Oct 24 2017

NoQ added a comment to D31868: [analyzer] Check NULL pointer dereference issue for memset function.
In D31868#904814, @MTC wrote:

One of the possible improvements for future work here would be to actually bind the second argument value to the buffer instead of just invalidating it. Like, after memset(buf, 0, sizeof(buf)) the analyzer should know that all values in the buf array are 0. In the analyzer we have the notion of *default bindings* to handle that (see documentation in docs/analyzer/RegionStore.txt for more details).

BindDefault() is the only function that can make the default binding, is it? If so, evalMemset() uses bindDefault(), the binding may not take effect. Because the current BindDefault() logic is that if the memory area has been initialized, then the default binding will no longer be done, see https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Core/RegionStore.cpp#L429. Before evalMemset(), MallocMemAux() in MallocChecker.cpp may have already made the default binding. Am I right?

Oct 24 2017, 3:05 AM

Oct 23 2017

NoQ accepted D39174: [analyzer] Fix handling of labels in getLValueElement .

Great. Probably one more reason to turn all Locs into regions.

Oct 23 2017, 12:37 AM

Oct 16 2017

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

Neat! Tests?

Oct 16 2017, 9:02 AM

Oct 13 2017

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

Oct 12 2017

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

Oct 12 2017, 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.

Oct 12 2017, 1:56 AM

Oct 11 2017

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.

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

Oct 10 2017

NoQ added inline comments to D38673: [analyzer] MisusedMovedObjectChecker: Fix false positive on state-resetting, handling method calls on base-class sub-objects.
Oct 10 2017, 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.

Oct 10 2017, 2:51 AM

Oct 4 2017

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

(whoops accidentally pushed the same reply twice, never mind)

Oct 4 2017, 11:00 AM
NoQ added inline comments to D35216: [analyzer] Escape symbols when creating std::initializer_list..
Oct 4 2017, 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.

Oct 4 2017, 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).

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

Whoops forgot to submit inline comments.

Oct 4 2017, 8:59 AM

Oct 3 2017

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

Yep, looks good.

Oct 3 2017, 8:44 AM

Oct 2 2017

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

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

Sep 28 2017

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!

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

Add a forgotten comment.

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

Sep 27 2017

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

Sep 26 2017

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 .

Sep 26 2017, 3:30 PM

Sep 25 2017

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.

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

Update to use .rst formatting.

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

Fix some comments in tests.

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

Looks good!

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

Sep 19 2017

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.

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

Yeah, sounds good to me as well.

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

*approves more active use of DEBUG()*

Sep 19 2017, 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