NoQ (Artem Dergachev)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 3 2015, 9:16 AM (137 w, 4 d)

Recent Activity

Yesterday

NoQ added inline comments to D45839: [analyzer] Add support for WebKit "unified sources"..
Mon, Apr 23, 2:48 PM
NoQ added inline comments to D45839: [analyzer] Add support for WebKit "unified sources"..
Mon, Apr 23, 2:31 PM
NoQ accepted D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header..

I could also move RangedConstraintManager.h under include

We probably don't want to do that: currently it can only be imported by files in Core, and we probably should keep it that way

Mon, Apr 23, 1:02 PM
NoQ added inline comments to D45839: [analyzer] Add support for WebKit "unified sources"..
Mon, Apr 23, 12:52 PM
NoQ added a comment to D45517: [analyzer] WIP: False positive refutation with Z3.
In D45517#1074057, @NoQ wrote:

So, yeah, that's a good optimization that we're not invoking the solver on every node. But i don't think we should focus on improving this optimization further; instead, i think the next obvious step here is to implement it in such a way that we only needed to call the solver once for every report. We could simply collect all constraints from all states along the path and put them into the solver all together. This will work because symbols are not mutable and they don't reincarnate.

Won't collecting all constraints and solving a ~100ish equations at once take a long time? Maybe the timeout limit for Z3 will need to be slightly increased for refutation then.

Mon, Apr 23, 12:40 PM
NoQ added a comment to D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header..

Another approach would be to instead teach RangedConstraintManager to convert it's constraints to Z3. That would be an unwanted dependency, but the change would be much smaller, and the internals of the solver would not have to be exposed. @NoQ thoughts?

Mon, Apr 23, 12:23 PM
NoQ added inline comments to D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header..
Mon, Apr 23, 12:03 PM
NoQ added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Guys, what do you think about a checker that warns on uninitialized fields only when at least one field is initialized? I'd be much more confident about turning such check on by default. We can still keep a pedantic version.

Mon, Apr 23, 11:56 AM

Fri, Apr 20

NoQ added inline comments to D45839: [analyzer] Add support for WebKit "unified sources"..
Fri, Apr 20, 5:12 PM
NoQ updated the diff for D45839: [analyzer] Add support for WebKit "unified sources"..

A more accurate extension check.

Fri, Apr 20, 5:12 PM
NoQ added inline comments to D45839: [analyzer] Add support for WebKit "unified sources"..
Fri, Apr 20, 2:43 PM
NoQ updated the diff for D45839: [analyzer] Add support for WebKit "unified sources"..

Address most comments.

Fri, Apr 20, 2:43 PM
NoQ accepted D45407: [StaticAnalyzer] Added notes to the plist output.

Checked this out, seems to be safely ignored for now. The .plist format is pretty resistant to this sort of stuff because most APIs to handle it are almost treating it as native arrays/dictionaries, so i guess it should be fine.

Fri, Apr 20, 2:06 PM · Restricted Project
NoQ added a comment to D45517: [analyzer] WIP: False positive refutation with Z3.

The visitor currently checks states appearing as block edges in the exploded graph. The first idea was to filter states based on the shape of the exploded graph, by checking the number of successors of the parent node, but surprisingly, both succ_size() and pred_size() seemed to return 1 for each node in the graph (except for the root), even if there clearly were branchings in the code (and on the .dot picture). To my understanding, the exploded graph is fully constructed at the stage where visitors are run, so I must be missing something.

Fri, Apr 20, 1:45 PM

Thu, Apr 19

NoQ added a comment to D45774: [analyzer] cover more cases where a Loc can be bound to constants.

Looks great, thanks!

Thu, Apr 19, 6:46 PM
NoQ updated the diff for D45839: [analyzer] Add support for WebKit "unified sources"..

Hopefully a more portable way of testing this stuff.

Thu, Apr 19, 4:55 PM
NoQ 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.

Thu, Apr 19, 4:31 PM
NoQ closed D45335: [analyzer] RetainCount: Accept more "safe" CFRetain wrappers..
Thu, Apr 19, 4:10 PM
NoQ accepted D45335: [analyzer] RetainCount: Accept more "safe" CFRetain wrappers..

Committed in rC330375 but i misplaced the phabricator link again, sorry.

Thu, Apr 19, 4:10 PM
NoQ added a comment to D45117: [analyzer] Fix diagnostics in callees of interesting callees..

rC330375 has nothing to do with this review; i misplaced the link in the commit message again.

Thu, Apr 19, 4:08 PM
NoQ created D45839: [analyzer] Add support for WebKit "unified sources"..
Thu, Apr 19, 2:59 PM

Mon, Apr 16

NoQ created D45706: [CFG] [analyzer] Add construction contexts for loop condition variables..
Mon, Apr 16, 3:41 PM
NoQ added a comment to D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs.

(also we'll need CFG dump tests)

Mon, Apr 16, 2:44 PM
NoQ added a comment to D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer..

Hmm, yeah, indeed, i must have overlooked it, nice one :) I'll see if i can fix the other place as well.

Mon, Apr 16, 2:19 PM
NoQ created D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer..
Mon, Apr 16, 1:52 PM

Fri, Apr 13

NoQ added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

Sorry, overwhelmed a bit, i'll try to get to this as soon as possible.

Fri, Apr 13, 8:37 PM · Restricted Project
NoQ added a comment to D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs.

(i'd much rather do the latter)

Fri, Apr 13, 8:32 PM
NoQ accepted D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs.

Wow, you actually did that.

Fri, Apr 13, 8:31 PM
NoQ accepted D45491: [analyzer] Do not invalidate the `this` pointer..

Yeah, i think this makes sense, thanks! It feels a bit weird that we have to add it as an exception - i wonder if there are other exceptions that we need to make. Widening over the stack memory space should be a whitelist, not a blacklist, because we can easily enumerate all stack variables and see which of them can be modified at all from the loop. But until we have that, this looks like a reasonable workaround.

Fri, Apr 13, 8:27 PM
NoQ updated the diff for D45650: [CFG] [analyzer] Don't treat argument constructors as temporary constructors..

Add tests where the argument is passed by reference. These tests work correctly (i.e. they correctly identify the argument constructor as a temporary constructor) because such constructors would include a MaterializeTemporaryExpr that is not allowed in the middle of the partial construction context. For the same reason remove the defensive check in the branch of ConstructionContext::createFromLayers that deals with MaterializeTemporaryExpr-based layers.

Fri, Apr 13, 8:21 PM
NoQ created D45650: [CFG] [analyzer] Don't treat argument constructors as temporary constructors..
Fri, Apr 13, 8:12 PM
NoQ accepted D45557: [Analyzer] Fix for SValBuilder expressions rearrangement.

Yup thanks!

Fri, Apr 13, 10:58 AM

Thu, Apr 12

NoQ added inline comments to D45557: [Analyzer] Fix for SValBuilder expressions rearrangement.
Thu, Apr 12, 12:42 PM

Wed, Apr 11

NoQ added a comment to D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option).

We've found a crash on our internal buildbot, would you like to have a look?:

Wed, Apr 11, 3:32 PM
NoQ added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

I wasn't too clear on this one: unknown fields are regarded as uninitialized, what I wrote was a temporary change in the code so I could check whether it would work.

Woops, I meant that unknown fields are regarded az initialized.

Wed, Apr 11, 1:19 PM
NoQ added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Ok, thanks, looking forward to it!

Wed, Apr 11, 11:57 AM
NoQ added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

That's a lot of code, so it'll take me some time to understand what's going on here. You might be able to help me by the large patch in smaller logical chunks.

Wed, Apr 11, 11:44 AM

Tue, Apr 10

NoQ added a comment to D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs.

I mean, like, if we try to work with the existing AST then we're stuck with a prvalue expression that represents an lvalue and will be assigned a Loc value, which is pretty weird anyway. Getting rid of the ParentMap in favor of providing enough context (eg. in the CFG or in checker callbacks) whenever we might want to use it sounds like a much saner solution. There might be other "unknown unknowns" about rewriting the AST, but our current problem looks as safe as we'll ever get.

Tue, Apr 10, 8:02 PM
NoQ added inline comments to D45491: [analyzer] Do not invalidate the `this` pointer..
Tue, Apr 10, 11:29 AM
NoQ added a comment to D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs.

The ultimate solution would probably be to add a fake cloned asm statement to the CFG (instead of the real asm statement) that would point to the correct output child-expression(s) that are untouched themselves but simply have their noop casts removed.

I have some concerns about this solution. It will result in difference between AST nodes and nodes that user will receive during analysis and I'm not sure that it is good. What is even worse here is that a lot of AST stuff won't work properly - ParentMap, for example.

Yep. But i'd rather avoid using the ParentMap. Also we already do this for DeclStmts that declare more than one VarDecl (split them up into single-decl statements), and the practical effect of such simplification is barely noticeable even though DeclStmts are so much more common.

Tue, Apr 10, 11:12 AM

Mon, Apr 9

NoQ added a reviewer for D45458: MallocChecker refactoring of calls checkers: xazax.hun.

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

Mon, Apr 9, 2:50 PM
NoQ accepted D45416: [analyzer] ExprEngine: model GCC inline asm rvalue cast outputs.

Eww. Weird AST.

Mon, Apr 9, 2:40 PM
NoQ added inline comments to D45417: [analyzer] Don't crash on printing ConcreteInt of size >64 bits.
Mon, Apr 9, 1:58 PM

Sat, Apr 7

NoQ edited reviewers for D45407: [StaticAnalyzer] Added notes to the plist output, added: NoQ; removed: dergachev.a.
Sat, Apr 7, 4:16 PM · Restricted Project
NoQ added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

The output looks reasonable to me, but we'll need to see if other consumers of the plist output (IDEs that supports the analyzer, such as Xcode) will be able to accept the modified output (at least, will be able to ignore it). I'll have a look.

Sat, Apr 7, 4:15 PM · Restricted Project

Thu, Apr 5

NoQ created D45335: [analyzer] RetainCount: Accept more "safe" CFRetain wrappers..
Thu, Apr 5, 2:24 PM

Wed, Apr 4

NoQ added inline comments to D44238: [CFG] Fix automatic destructors when a member is bound to a reference..
Wed, Apr 4, 4:24 PM
NoQ added inline comments to D44238: [CFG] Fix automatic destructors when a member is bound to a reference..
Wed, Apr 4, 11:39 AM

Tue, Apr 3

NoQ added inline comments to D45241: [analyzer] Invalidate union regions properly. Don't hesitate to load the default binding later..
Tue, Apr 3, 7:17 PM
NoQ created D45241: [analyzer] Invalidate union regions properly. Don't hesitate to load the default binding later..
Tue, Apr 3, 7:13 PM

Sun, Apr 1

NoQ added a comment to D45149: MallocChecker, adding specific BSD calls.

Nice, thanks.

Sun, Apr 1, 2:36 PM

Fri, Mar 30

NoQ added inline comments to D44956: [analyzer] Fix liveness calculation for C++17 structured bindings.
Fri, Mar 30, 6:23 PM
NoQ accepted D44956: [analyzer] Fix liveness calculation for C++17 structured bindings.

LGTM!

Fri, Mar 30, 6:16 PM
NoQ accepted D45115: [analyzer] Fix assertion crash in CStringChecker.

More LocAsInteger problems. We need more flexible symbolic expressions.

Fri, Mar 30, 6:08 PM
NoQ accepted D45113: [analyzer] Cache computation of regionAsOffset.

The huge switch in getAsOffset() does really nothing good compared to a virtual function. I suspect that this alone could have made it faster, depending on how the switch is compiled. And with a virtual function, we could also easily add the new field and the lookup only to those 3-4 regions that aren't base regions - which might as well be faster. But this already looks good, we should have tried this earlier!

Fri, Mar 30, 6:08 PM
NoQ created D45117: [analyzer] Fix diagnostics in callees of interesting callees..
Fri, Mar 30, 5:56 PM
NoQ added a comment to D45115: [analyzer] Fix assertion crash in CStringChecker.

Test?

Fri, Mar 30, 4:14 PM
NoQ added inline comments to D45071: [analyzer] Track null or undef values through pointer arithmetic..
Fri, Mar 30, 12:11 PM
NoQ updated the diff for D45071: [analyzer] Track null or undef values through pointer arithmetic..

Substraction is an additive operation. Added tests for that. Added even more tests.

Fri, Mar 30, 12:11 PM
NoQ updated the diff for D44955: [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr..

Use auto.

Fri, Mar 30, 12:04 PM
NoQ added inline comments to D44854: [analyzer] Be more careful about C++17 copy elision..
Fri, Mar 30, 11:55 AM

Thu, Mar 29

NoQ created D45071: [analyzer] Track null or undef values through pointer arithmetic..
Thu, Mar 29, 5:16 PM
NoQ added inline comments to D45010: [analyzer] Minor improvements to region printing.
Thu, Mar 29, 3:11 PM
NoQ added inline comments to D44956: [analyzer] Fix liveness calculation for C++17 structured bindings.
Thu, Mar 29, 11:59 AM
NoQ accepted D45010: [analyzer] Minor improvements to region printing.
Thu, Mar 29, 11:57 AM

Wed, Mar 28

NoQ updated the diff for D44854: [analyzer] Be more careful about C++17 copy elision..

Fix a comment in tests.

Wed, Mar 28, 5:49 PM
NoQ updated the diff for D44854: [analyzer] Be more careful about C++17 copy elision..

Conditional operators, like call-expressions in D44273, may also be glvalues of record type instead of simply being reference-typed.

Wed, Mar 28, 5:49 PM
NoQ added a comment to D44956: [analyzer] Fix liveness calculation for C++17 structured bindings.

I guess the interesting part here is what happens if only a but not b is used (or vice versa).

Wed, Mar 28, 4:05 PM
NoQ added a comment to D44934: [analyzer] Improve the modeling of `memset()`..

In addition, memset can bind anything to the region, so getBindingForDerivedDefaultValue()'s logic needs some adjustment. The solution in this patch is certainly not correct.

Wed, Mar 28, 4:02 PM
NoQ added a comment to D44934: [analyzer] Improve the modeling of `memset()`..

Why do you need separate code for null and non-null character? The function's semantics doesn't seem to care.

Wed, Mar 28, 4:00 PM

Tue, Mar 27

NoQ accepted D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option).

This looks good with a super tiny nit regarding comments about signed integers. George, are you happy with the changes? (:

Tue, Mar 27, 4:08 PM
NoQ created D44955: [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr..
Tue, Mar 27, 1:48 PM

Mar 23 2018

NoQ created D44854: [analyzer] Be more careful about C++17 copy elision..
Mar 23 2018, 4:47 PM

Mar 21 2018

NoQ added a comment to D44756: [analyzer] [NFC] A convenient getter for getting a current stack frame.

Fair enough!

Mar 21 2018, 5:25 PM
NoQ accepted D44759: [analyzer] [NFC] Move worklist implementation to WorkList.cpp.
Mar 21 2018, 5:16 PM
NoQ retitled D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method. from [analyzer] NFC: Move construction context allocation into a helper method. to [CFG] [analyzer] NFC: Move construction context allocation into a helper method..
Mar 21 2018, 5:12 PM
NoQ added a dependency for D44763: [CFG] [analyzer] Add C++17-specific constructor-initializer construction contexts.: D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method..
Mar 21 2018, 5:11 PM
NoQ added a dependent revision for D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method.: D44763: [CFG] [analyzer] Add C++17-specific constructor-initializer construction contexts..
Mar 21 2018, 5:11 PM
NoQ retitled D44763: [CFG] [analyzer] Add C++17-specific constructor-initializer construction contexts. from [analyzer] Add C++17-specific constructor-initializer construction contexts. to [CFG] [analyzer] Add C++17-specific constructor-initializer construction contexts..
Mar 21 2018, 5:11 PM
NoQ created D44763: [CFG] [analyzer] Add C++17-specific constructor-initializer construction contexts..
Mar 21 2018, 5:10 PM
NoQ added a comment to D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method..

E.g. we have llvm::make_shared<>(), so we could also have Allocator::make_obj<...>(...)

Mar 21 2018, 5:04 PM
NoQ updated the diff for D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method..

I'd rather keep the helper method within the class. This allows us to keep constructors private, which is something i forgot to do originally.

Mar 21 2018, 4:12 PM
NoQ updated the diff for D44755: [analyzer] Suppress more C++17-related crashes..

Add a comment for the confusing part.

Mar 21 2018, 3:01 PM
NoQ added a dependent revision for D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts.: D44755: [analyzer] Suppress more C++17-related crashes..
Mar 21 2018, 1:32 PM
NoQ added a dependency for D44755: [analyzer] Suppress more C++17-related crashes.: D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts..
Mar 21 2018, 1:32 PM
NoQ created D44755: [analyzer] Suppress more C++17-related crashes..
Mar 21 2018, 1:32 PM

Mar 20 2018

NoQ updated the summary of D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method..
Mar 20 2018, 7:49 PM
NoQ added a dependency for D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method.: D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts..
Mar 20 2018, 7:48 PM
NoQ added a dependent revision for D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts.: D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method..
Mar 20 2018, 7:48 PM
NoQ created D44725: [CFG] [analyzer] NFC: Move construction context allocation into a helper method..
Mar 20 2018, 7:48 PM
NoQ updated the diff for D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts..

Address review comments.

Mar 20 2018, 7:37 PM
NoQ created D44721: [analyzer] Enable c++-temp-dtor-inlining by default?.
Mar 20 2018, 6:43 PM
NoQ added a comment to D44557: [analyzer] CStringChecker.cpp - Code refactoring on bug report..
In D44557#1042792, @MTC wrote:
In D44557#1042357, @NoQ wrote:

Sorry, one moment, i'm seeing a few regressions after the previous refactoring but i didn't look at them closely yet to provide a reproducer. I'll get back to this.

Thank you for the code review, @NoQ !

The regression is maybe caused by https://reviews.llvm.org/D44075, my patch! CheckBufferAccess() may not guarantee checkNonNull() will be called. If that's true, please revert https://reviews.llvm.org/rC326782 and I'll fix it.

Mar 20 2018, 6:01 PM
NoQ updated the diff for D44281: [analyzer] Suppress more MallocChecker positives in reference counting pointer destructors..
  • fix auto.
  • don't use regexs yet because it's clean enough anyway; maybe later.
Mar 20 2018, 5:10 PM

Mar 19 2018

NoQ accepted D44503: [analyzer] Improve performance of NoStoreFuncVisitor.

We've had a 4x analysis time increase on a specific file due to this code interacting in weird manner with temporary constructor inlining.

Mar 19 2018, 2:13 PM
NoQ 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?

Mar 19 2018, 2:09 PM
NoQ added a comment to D44557: [analyzer] CStringChecker.cpp - Code refactoring on bug report..

Sorry, one moment, i'm seeing a few regressions after the previous refactoring but i didn't look at them closely yet to provide a reproducer. I'll get back to this.

Mar 19 2018, 2:09 PM
NoQ accepted D44606: [analyzer] Fix the crash in `IteratorChecker.cpp` when `SymbolConjured` has a null Stmt..

Just in case: we indeed do not guarantee that SymbolConjured corresponds to a statement; it is, however, not intended, but rather a bug. Consider:

 1	void clang_analyzer_eval(bool);
 2
 3	class C {
 4	  int &x;
 5	public:
 6	  C(int &x): x(x) {}
 7	  ~C();
 8	};
 9
10	void foo() {
11	  int x = 1;
12	  {
13	    C c(x);
14	  }
15	  int y = x;
16	  {
17	    C c(x);
18	  }
19	  int z = x;
20	  clang_analyzer_eval(y == z);
21	}
Mar 19 2018, 2:04 PM

Mar 16 2018

NoQ added inline comments to D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts..
Mar 16 2018, 6:43 PM
NoQ created D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts..
Mar 16 2018, 6:36 PM