Page MenuHomePhabricator

vsavchenko (Valeriy Savchenko)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 6 2020, 5:32 AM (40 w, 5 d)

Recent Activity

Yesterday

vsavchenko added inline comments to D94673: [analyzer][CTU] API for CTU macro expansions.
Fri, Jan 15, 1:04 AM · Restricted Project

Tue, Jan 12

vsavchenko added inline comments to D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics..
Tue, Jan 12, 3:05 AM

Wed, Jan 6

vsavchenko requested review of D94177: [analyze] Add better support for leaks (and similar diagnostics).
Wed, Jan 6, 7:16 AM · Restricted Project

Tue, Jan 5

vsavchenko added a comment to D93630: [Attr] Apply GNU-style attributes to expression statements.

I guess I want to clarify one point here, after this patch the parser will not assume that statement always follows statement attributes. We simply turn off the assumption that what follows is a declaration, parser will simply determine whether it is a statement or a declaration, it can do it on its own without attribute's "help".

We used to say that if there's a GNU attribute (or we're parsing a declaration statement), what follows must be a declaration. Now we're using the attributes in the given attribute list to decide whether what follows is a declaration or a statement (if all of the attributes are statement attributes, we don't treat what follows as being a declaration unless we're in a declaration statement). Or am I misreading the code?

We do not decide whether it is declaration or statement. I do get that it is a matter of perspective, but right now I prefer to read it like this:
BEFORE: if we've seen a GNU-style attribute parse whatever comes next as a declaration
AFTER: if we've seen a GNU-style attribute except for the case when all of the parsed attributes are known to be statement attributes, parse whatever comes next as a declaration

So, essentially, when we see statement attributes only, attribute/declaration heuristic is canceled and we parse exactly the way we would've parsed as if no attributes present.

I think this is a defensible design, but it still has the potential to change the parsing behavior in some circumstances and I want to make sure those circumstances are ones we know about. Note, I still think we should coordinate this change with the GCC folks. GCC has a lot of attributes that Clang does not have, so my concern with GCC is that Clang supports writing attributes in this way and GCC can't support it for some reason and it causes issues for an attribute we want to share between implementations. I don't have a specific case in mind that may cause an issue but they have a lot of attributes I'm unfamiliar with.

Tue, Jan 5, 10:57 AM · Restricted Project
vsavchenko committed rGfec1a442e3b1: [-Wcalled-once-parameter] Introduce 'called_once' attribute (authored by vsavchenko).
[-Wcalled-once-parameter] Introduce 'called_once' attribute
Tue, Jan 5, 7:28 AM
vsavchenko closed D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Tue, Jan 5, 7:28 AM · Restricted Project
vsavchenko added inline comments to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Tue, Jan 5, 7:05 AM · Restricted Project
vsavchenko added a comment to D93630: [Attr] Apply GNU-style attributes to expression statements.

I guess I want to clarify one point here, after this patch the parser will not assume that statement always follows statement attributes. We simply turn off the assumption that what follows is a declaration, parser will simply determine whether it is a statement or a declaration, it can do it on its own without attribute's "help".

We used to say that if there's a GNU attribute (or we're parsing a declaration statement), what follows must be a declaration. Now we're using the attributes in the given attribute list to decide whether what follows is a declaration or a statement (if all of the attributes are statement attributes, we don't treat what follows as being a declaration unless we're in a declaration statement). Or am I misreading the code?

Tue, Jan 5, 6:28 AM · Restricted Project
vsavchenko added a comment to D93630: [Attr] Apply GNU-style attributes to expression statements.

I guess I want to clarify one point here, after this patch the parser will not assume that statement always follows statement attributes. We simply turn off the assumption that what follows is a declaration, parser will simply determine whether it is a statement or a declaration, it can do it on its own without attribute's "help".

Tue, Jan 5, 2:53 AM · Restricted Project
vsavchenko added inline comments to D93630: [Attr] Apply GNU-style attributes to expression statements.
Tue, Jan 5, 1:26 AM · Restricted Project

Wed, Dec 23

vsavchenko added a comment to D93630: [Attr] Apply GNU-style attributes to expression statements.

Yeah, I kind of figured that might be the cause. I'm not 100% convinced (one way or the other) if the suppress attribute should get a GNU spelling. The [[]] spellings are available in all language modes (we have -fdouble-square-bracket-attributes to enable this) and don't run afoul of the "guess what this attribute appertains to" problem that GNU-style attributes do.

Wed, Dec 23, 7:09 AM · Restricted Project
vsavchenko updated the diff for D93630: [Attr] Apply GNU-style attributes to expression statements.

Refine condition for statement attributes

Wed, Dec 23, 7:01 AM · Restricted Project
vsavchenko added inline comments to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Wed, Dec 23, 6:19 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Correct some comments and names

Wed, Dec 23, 6:19 AM · Restricted Project
vsavchenko added inline comments to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Wed, Dec 23, 5:29 AM · Restricted Project

Tue, Dec 22

vsavchenko updated the diff for D93630: [Attr] Apply GNU-style attributes to expression statements.

Maintain previous behavior unless preceeded by a statement attribute

Tue, Dec 22, 9:00 AM · Restricted Project
vsavchenko added a comment to D93630: [Attr] Apply GNU-style attributes to expression statements.

However, taking a step back -- what attributes would need this functionality (and couldn't be written on something within the expression statement)?

Tue, Dec 22, 8:41 AM · Restricted Project
vsavchenko added a comment to D93630: [Attr] Apply GNU-style attributes to expression statements.

@aaron.ballman I totally agree, but I also would like to understand.
__attribute__ is a GNU extension, right? Then why does it affect the grammar of C? I always thought that attributes should be somewhat transparent for parsers, but it looks like in this situation all compilers automatically assume that __attribute__ begins a declaration.
It is unclear to me why *x;, [[unknown]] *x; (dereference of x) and __attribute__((unknown)) *x; (declaration of int *) have different meanings.

Tue, Dec 22, 8:02 AM · Restricted Project
vsavchenko updated the diff for D93630: [Attr] Apply GNU-style attributes to expression statements.

Fix block.c test

Tue, Dec 22, 6:04 AM · Restricted Project

Mon, Dec 21

vsavchenko added reviewers for D93630: [Attr] Apply GNU-style attributes to expression statements: aaron.ballman, rsmith, jkorous, NoQ.
Mon, Dec 21, 5:21 AM · Restricted Project
vsavchenko requested review of D93630: [Attr] Apply GNU-style attributes to expression statements.
Mon, Dec 21, 5:19 AM · Restricted Project

Fri, Dec 18

vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Add more test cases

Fri, Dec 18, 7:54 AM · Restricted Project
vsavchenko added inline comments to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Fri, Dec 18, 7:48 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Add one gigantic comment on status kinds and the reasons behind some of the design choices

Fri, Dec 18, 6:41 AM · Restricted Project
vsavchenko added a comment to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Your test cases suggest that this is not inter-procedurally checked; it seems to require all of the APIs to be annotated in order to get the same effect that an inter-procedural check could determine on its own.

Fri, Dec 18, 6:27 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Detect all conventions for captured parameters as well

Fri, Dec 18, 3:14 AM · Restricted Project
vsavchenko added a comment to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Fri, Dec 18, 12:16 AM · Restricted Project

Dec 17 2020

vsavchenko added reviewers for D93464: [analyzer] Refine suppression mechanism to better support AST checks: NoQ, xazax.hun, jkorous, aaron.ballman, martong, Szelethus, steakhal.
Dec 17 2020, 8:59 AM · Restricted Project
vsavchenko requested review of D93464: [analyzer] Refine suppression mechanism to better support AST checks.
Dec 17 2020, 8:57 AM · Restricted Project
vsavchenko added a comment to D93110: [analyzer] Implement a first version of suppressions via attributes.

Have you explored how this attribute will work with clang frontend diagnostics or clang-tidy diagnostics?

Dec 17 2020, 6:24 AM · Restricted Project
vsavchenko added inline comments to D93110: [analyzer] Implement a first version of suppressions via attributes.
Dec 17 2020, 6:03 AM · Restricted Project

Dec 16 2020

vsavchenko added a comment to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Is the attribute considered to be a property of the parameter or a property of the function the parameter is declared in? e.g.,

void someOtherFunc(void (^cb)(void)) {
  if (something())
    cb();
}

void barWithCallback(void (^callback)(void) __attribute__((called_once))) {
  someOtherFunc(callback);
}

Should code like that also diagnose given that callback is not called on every code path? From the test cases you have, it looks like you explicitly allow this code without diagnosing it, which suggests the attribute is really a property of the function and not a property of the parameter. This in turn makes me wonder whether a better design is for -Wcompletion-handler to diagnose any function with a completion handler-like parameter if the parameter is not called exactly once within the function, and the user marks the functions that should be exempt from the checking rather than marking the parameters that should be checked (or does this have too many false positives in practice)? Alternatively, should the check be implemented as a clang static analyzer check that tracks an annotated parameter across the inter-procedural CFG and diagnoses such code?

Dec 16 2020, 8:00 AM · Restricted Project
vsavchenko added inline comments to D93110: [analyzer] Implement a first version of suppressions via attributes.
Dec 16 2020, 6:39 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Add more info to the docs

Dec 16 2020, 1:39 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Turn off conventional check heuristic

Dec 16 2020, 1:06 AM · Restricted Project

Dec 15 2020

vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Introduce a suppression for double call warning by nullifying the parameter.

Dec 15 2020, 9:17 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Fix minor things

Dec 15 2020, 7:39 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Refine conventions for completion handlers

Dec 15 2020, 7:28 AM · Restricted Project

Dec 14 2020

vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Add documentation for new attribute and fix review remarks

Dec 14 2020, 6:23 AM · Restricted Project

Dec 11 2020

vsavchenko added inline comments to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Dec 11 2020, 8:24 AM · Restricted Project
vsavchenko added a comment to D93110: [analyzer] Implement a first version of suppressions via attributes.

Right now, I reused existing suppress attribute. However I did it in a rather unconventional manner. I allowed 0 arguments in one spelling and >1 in another, which seems odd.
I can see a couple other possible solutions here:

Dec 11 2020, 5:57 AM · Restricted Project
vsavchenko added reviewers for D93110: [analyzer] Implement a first version of suppressions via attributes: NoQ, xazax.hun, martong, steakhal, Szelethus, ravikandhadai, dcoughlin, jkorous.
Dec 11 2020, 5:52 AM · Restricted Project
vsavchenko requested review of D93110: [analyzer] Implement a first version of suppressions via attributes.
Dec 11 2020, 5:50 AM · Restricted Project

Dec 9 2020

vsavchenko added a comment to D92928: [analyzer] Highlight arrows for currently selected event.

Here is the HTML file for the test.

Dec 9 2020, 3:21 AM · Restricted Project
vsavchenko added reviewers for D92928: [analyzer] Highlight arrows for currently selected event: NoQ, xazax.hun, Szelethus, steakhal, martong, ASDenysPetrov.
Dec 9 2020, 3:18 AM · Restricted Project
vsavchenko requested review of D92928: [analyzer] Highlight arrows for currently selected event.
Dec 9 2020, 3:17 AM · Restricted Project
vsavchenko updated the diff for D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports.

Replace let with const

Dec 9 2020, 3:16 AM · Restricted Project
vsavchenko added a comment to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

@aaron.ballman Can you please take a look at the attribute side of this?

Dec 9 2020, 12:08 AM · Restricted Project

Dec 4 2020

vsavchenko updated the diff for D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports.

Fix incorrect comment

Dec 4 2020, 2:46 AM · Restricted Project
vsavchenko added a comment to D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports.

It is really a good idea!

Thanks 😊

The operations that would not leave an event in the report are now clearly printed.

But there are three arrows that confuse me in the example report: the assignment x = 0 (x -> 0 -> x), the function call dereference(x) (x -> dereference), and the return statement return *x (int -> *x). I know the arrow is based on the evaluation order of the engine. But from the view of a user, I think these arrows are confusing to some extent.

For the first two, I think it would be better to point just the statement (maybe a CFGElement) without inner arrows (x -> 0 -> x and x -> dereference), or point to the location of the operator itself rather than the BeginLoc (e.g. x -> 0 -> =). For the third one, an arrow from the function name to the first CFGElement looks good to me. And an arrow from the returned expr to the return type or to a special mark (e.g. ⬅️) can also be added, together with function calls (an arrow from the callstmt to a special mark, e.g. ➡️).

Sorry, for the confusion. I did not come up with the idea of arrows and neither did I chose which tokens are connected by arrows. It is an existing feature, and it existed for quite a long time. The analyzer can produce plist files that contain all these locations, plist files are further used by Xcode to draw arrows. You can see a very old example on our website: https://clang-analyzer.llvm.org
Essentially I took this existing information and used it in the HTML report generation as well. The biggest chunk of this commit is the algorithm for drawing SVG curves.

Dec 4 2020, 2:44 AM · Restricted Project
vsavchenko added a comment to D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports.

Here is how it looks for that test file:

Dec 4 2020, 12:41 AM · Restricted Project
vsavchenko requested review of D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports.
Dec 4 2020, 12:36 AM · Restricted Project

Dec 3 2020

vsavchenko accepted D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis..

LGTM!

Dec 3 2020, 7:56 AM · Restricted Project

Nov 30 2020

vsavchenko accepted D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64.

That's a good fix!
How did this happen though that max value of off_t was even used for fd. Seems pretty odd!

I used a semi-automated approach to create these summaries from cppcheck : I translated the XML into C++ source code, but could not do the translation for ranges. E.g., 0: was just simply copied and I had to manually modify the generated C++ code to have the proper range. And that's where I made the wrong index for the param (cppcheck starts from idx 1, here we start from idx 0). Here the 5th param has the off_t type, so I thought we have to get the max for off_t.

Nov 30 2020, 5:11 AM · Restricted Project
vsavchenko added a comment to D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64.

That's a good fix!
How did this happen though that max value of off_t was even used for fd. Seems pretty odd!

Nov 30 2020, 3:45 AM · Restricted Project

Nov 25 2020

vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Add another comment

Nov 25 2020, 5:48 AM · Restricted Project
vsavchenko added inline comments to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Nov 25 2020, 3:21 AM · Restricted Project
vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Add more comments and fix another test.

Nov 25 2020, 3:19 AM · Restricted Project

Nov 24 2020

vsavchenko updated the diff for D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Fix tests and a typo

Nov 24 2020, 10:28 AM · Restricted Project
vsavchenko added a comment to D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.

Probably irrelevant comment from the C++ world: If I needed this concept in C++, I'd probably piggyback on the existing semantic analysis of std::move: I'd design a class OnceCallable with an operator() && that both called and nulled-out its object parameter, and then call it like std::move(myCallback)(). Then if I ever tried to call it a second time, the static analyzer would (hopefully) complain that I was accessing a moved-from myCallback. But you want this for existing APIs that take the built-in block type, so never mind. :)

Nov 24 2020, 10:21 AM · Restricted Project
vsavchenko requested review of D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute.
Nov 24 2020, 8:39 AM · Restricted Project

Nov 23 2020

vsavchenko added a comment to D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base..

LGTM! But I'd like other folks to take a look at it first.

Nov 23 2020, 12:47 AM · Restricted Project

Oct 20 2020

vsavchenko added a comment to D89638: [Analyzer][WebKit] Add attributes to suppress specific checkers.

I was thinking about an alternative approach (5th approach (?), yeah, I think we need an RFC).

What if we had just one attribute with a StringArgument ?
Actually, we already have that with the annotate attribute.
How do you like this?

struct [[clang::annotate("CSA:supress:RefCntblBaseVirtualDtor")]] Derived2
    : RefCntblBase{};

Disadvantages: we must process strings whenever a node has the annotate attr attached, we have to come up with a "DSL".
Advantages: total flexibility.
WDYT?

Oct 20 2020, 8:38 AM

Oct 19 2020

vsavchenko added inline comments to D89638: [Analyzer][WebKit] Add attributes to suppress specific checkers.
Oct 19 2020, 10:19 AM
vsavchenko added inline comments to D89638: [Analyzer][WebKit] Add attributes to suppress specific checkers.
Oct 19 2020, 8:38 AM
vsavchenko added a comment to D89638: [Analyzer][WebKit] Add attributes to suppress specific checkers.

Great job, I think it's awesome to have more control over the analyzer with the language itself.

Oct 19 2020, 2:23 AM

Sep 21 2020

vsavchenko added a comment to D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs.

Great job! I also wanted to support more operations for range-based logic.

Sep 21 2020, 8:33 AM · Restricted Project
vsavchenko accepted D88019: [analyzer][solver] Fix issue with symbol non-equality tracking.

Amazing!

Sep 21 2020, 7:35 AM · Restricted Project
vsavchenko added a comment to D88019: [analyzer][solver] Fix issue with symbol non-equality tracking.

After this "accepted" and a huge thank you 😄

Sep 21 2020, 5:49 AM · Restricted Project
vsavchenko added a comment to D88019: [analyzer][solver] Fix issue with symbol non-equality tracking.

I came up with exactly the same fix! Great job!
I just wanted to refactor it and not having

Sep 21 2020, 5:39 AM · Restricted Project
vsavchenko added a comment to D82445: [analyzer][solver] Track symbol equivalence.

Hi Valery,

Together with @steakhal we have found a serious issue.
The below code crashes if you compile with -DEXPENSIVE_CHECKS.
The analyzer goes on an unfeasible path, the State has a contradiction.

We think that getRange(Sym("a != b") should return a set that does not contain 0.
https://github.com/llvm/llvm-project/blob/e63b488f2755f91e8147fd678ed525cf6ba007cc/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp#L2064
Currently that goes down to infer(QualType("int")) which results a RangeSet[INT_MIN, INT_MAX].
Stach trace attached.

// RUN: %clang_analyze_cc1 %s \
// RUN:   -analyzer-checker=core \
// RUN:   -analyzer-checker=debug.ExprInspection \
// RUN:   -triple x86_64-unknown-linux \
// RUN:   -verify

// expected-no-diagnostics

void f(int a, int b) {
  int c = b - a;
  if (c <= 0)
    return;
  // c > 0
  // b - a > 0
  // b > a
  if (a != b)
    return;
  // a == b
  // This is an unfeasible path, the State has a contradiction.
  if (c < 0) // crash here
    ;
}
#0  (anonymous namespace)::SymbolicRangeInferrer::inferRange<clang::ento::SymExpr const*> (BV=..., F=..., State=..., Origin=0x55b9979bed08) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:715
#1  0x00007fa2314dddc9 in (anonymous namespace)::RangeConstraintManager::getRange (this=0x55b99791ab10, State=..., Sym=0x55b9979bed08) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2012
#2  0x00007fa2314de1e9 in (anonymous namespace)::RangeConstraintManager::assumeSymEQ (this=0x55b99791ab10, St=..., Sym=0x55b9979bed08, Int=..., Adjustment=...) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2064
#3  0x00007fa23150470d in clang::ento::RangedConstraintManager::assumeSymUnsupported (this=0x55b99791ab10, State=..., Sym=0x55b9979bed08, Assumption=false) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp:136
#4  0x00007fa2315353a9 in clang::ento::SimpleConstraintManager::assumeAux (this=0x55b99791ab10, State=..., Cond=..., Assumption=false) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp:62
#5  0x00007fa23153518b in clang::ento::SimpleConstraintManager::assume (this=0x55b99791ab10, State=..., Cond=..., Assumption=false) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp:46
#6  0x00007fa2315350e5 in clang::ento::SimpleConstraintManager::assume (this=0x55b99791ab10, State=..., Cond=..., Assumption=false) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp:41
#7  0x00007fa2313d39b3 in clang::ento::ConstraintManager::assumeDual (this=0x55b99791ab10, State=..., Cond=...) at ../../git/llvm-project/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:105
#8  0x00007fa2313d3bfc in clang::ento::ProgramState::assume (this=0x55b9979beef8, Cond=...) at ../../git/llvm-project/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:681
#9  0x00007fa231436b8a in clang::ento::ExprEngine::evalEagerlyAssumeBinOpBifurcation (this=0x7fffdf9ce9d0, Dst=..., Src=..., Ex=0x55b9979893f0) at ../../git/llvm-project/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:3058
Sep 21 2020, 1:50 AM · Restricted Project

Sep 7 2020

vsavchenko accepted D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions.

LGTM!
Thanks!

Sep 7 2020, 2:40 AM · Restricted Project

Sep 2 2020

vsavchenko accepted D87004: [analyzer] Evaluate PredefinedExpressions.

Thanks!
Great job!

Sep 2 2020, 7:53 AM · Restricted Project
vsavchenko added inline comments to D87004: [analyzer] Evaluate PredefinedExpressions.
Sep 2 2020, 6:41 AM · Restricted Project
vsavchenko added a comment to D87004: [analyzer] Evaluate PredefinedExpressions.

Awesome!
Can we cover __builtin_unique_stable_name as well?

Sep 2 2020, 6:39 AM · Restricted Project
vsavchenko requested changes to D87004: [analyzer] Evaluate PredefinedExpressions.
Sep 2 2020, 3:39 AM · Restricted Project

Sep 1 2020

vsavchenko accepted D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

I'd prefer to have a couple more green lights. In particular, another look on the constraint manager and Objective C stuff would be great.

Sep 1 2020, 1:37 AM · Restricted Project

Aug 28 2020

vsavchenko committed rG9300ca541164: [doxygen] Fix bad doxygen results for BugReporterVisitors.h (authored by OikawaKirie).
[doxygen] Fix bad doxygen results for BugReporterVisitors.h
Aug 28 2020, 2:41 AM
vsavchenko closed D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h.
Aug 28 2020, 2:41 AM · Restricted Project

Aug 27 2020

vsavchenko added a comment to D86465: [analyzer][solver] Redesign constraint ranges data structure.

These are really promising figures, nice work! (And the measurement stuff itself is also a great addition, thanks for that!)

Thanks 😊

Not that it would matter much, but I was just wondering why there is a slightly bigger memory usage in some of the docker/small projects? The most conspicuous is oatpp.

The measurements fluctuate quite significantly and as you can see the means for both old and new experiments for oatpp are pretty close. So, I would say that the memory differences are not conclusive.
This being said, the performance measurements should also be taken with a grain of salt. We can't really say that we get 5-10% performance improvement for sure, but that analysis tends to be faster instead.

Aug 27 2020, 6:31 AM · Restricted Project
vsavchenko updated the diff for D86465: [analyzer][solver] Redesign constraint ranges data structure.

Fix review remarks

Aug 27 2020, 6:24 AM · Restricted Project

Aug 26 2020

vsavchenko added a comment to D86465: [analyzer][solver] Redesign constraint ranges data structure.

Here are some benchmarking results.
In docker:



Natively on my Mac (no memory measurements due to this issue during psutil installation):

Aug 26 2020, 3:14 AM · Restricted Project
vsavchenko added a comment to D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

@vsavchenko I just realized a significant portion of your work for this release is the following list:

git log llvmorg-11-init..llvmorg-11.0.0-rc2 --oneline -- clang/utils/analyzer/

Is it a correct guess that while your primary audience are the analyzer developers, it wouldn't hurt to mention it in the release notes?

Aug 26 2020, 2:05 AM · Restricted Project
vsavchenko added inline comments to D86465: [analyzer][solver] Redesign constraint ranges data structure.
Aug 26 2020, 1:47 AM · Restricted Project

Aug 25 2020

vsavchenko added inline comments to D86465: [analyzer][solver] Redesign constraint ranges data structure.
Aug 25 2020, 2:18 AM · Restricted Project
vsavchenko added inline comments to D86465: [analyzer][solver] Redesign constraint ranges data structure.
Aug 25 2020, 1:10 AM · Restricted Project
vsavchenko added a comment to D86465: [analyzer][solver] Redesign constraint ranges data structure.
In D86465#2235289, @NoQ wrote:

If the numbers are confirmed to be as good as what i've sneak-peeked so far, that should be pretty amazing. Also whoa, test coverage!~

I'll add the charts with performance in the next couple of days

WDYT about moving introducing a generic "SmallImmutableSet" in llvm/ADT to back your implementation?

Oof, it is not really possible. First of all, existing RangeSet didn't really ensure that ranges do not intersect (add is/was not checking for that), and this is left for the user. Additionally, it is designed and optimized specifically for RangeSet queries and operations. Like intersect and contains in a more generic Set sense mean entirely different things.

Aug 25 2020, 12:36 AM · Restricted Project

Aug 24 2020

vsavchenko added inline comments to D86465: [analyzer][solver] Redesign constraint ranges data structure.
Aug 24 2020, 9:52 AM · Restricted Project
vsavchenko requested review of D86465: [analyzer][solver] Redesign constraint ranges data structure.
Aug 24 2020, 8:31 AM · Restricted Project
vsavchenko added a comment to D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size.

Yep, I guess that is the cause. I'll take a look. Did you try it with this fast fix?

I tried, but it lacks further fixes.

Currently, I have this:

diff --git a/clang/utils/analyzer/Dockerfile b/clang/utils/analyzer/Dockerfile
index f74ff8aa95c..7727d92f98f 100644
--- a/clang/utils/analyzer/Dockerfile
+++ b/clang/utils/analyzer/Dockerfile
@@ -1,4 +1,4 @@
-FROM ubuntu:bionic
+FROM ubuntu:bionic-20200526
 
 RUN apt-get update && apt-get install -y \
     apt-transport-https \
@@ -12,12 +12,13 @@ RUN wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc 2>/dev/nul
 RUN apt-add-repository -y 'deb https://apt.kitware.com/ubuntu/ bionic main'
 
 # test system dependencies
+# TODO: should we really depend on the cmake version 3.18.2 here? I don't think so.
 RUN apt-get update && apt-get install -y \
     git=1:2.17.1-1ubuntu0.7 \
     gettext=0.19.8.1-6ubuntu0.3 \
     python3=3.6.7-1~18.04 \
-    python3-pip=9.0.1-2.3~ubuntu1.18.04.1 \
-    cmake=3.17.3-0kitware1 \
+    python3-pip=9.0.1-2.3~ubuntu1.18.04.2 \
+    cmake=3.18.2-0kitware1 \
     ninja-build=1.8.2-1
 
 # box2d dependencies

I'm not sure if its the company firewall or something else.

Aug 24 2020, 7:32 AM · Restricted Project
vsavchenko committed rGaec12c1264ac: [analyzer][tests] Add a notion of project sizes (authored by vsavchenko).
[analyzer][tests] Add a notion of project sizes
Aug 24 2020, 6:13 AM
vsavchenko closed D83942: [analyzer][tests] Add a notion of project sizes.
Aug 24 2020, 6:13 AM · Restricted Project
vsavchenko added a comment to D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size.

Here is a short summary how to do regression testing (check that all warnings are the same):

Oh thanks for the detailed guide, I will make the experiment.

However the ./SATest.py docker --build-image seems broken.

E: Version '9.0.1-2.3~ubuntu1.18.04.1' for 'python3-pip' was not found
The command '/bin/sh -c apt-get update && apt-get install -y     git=1:2.17.1-1ubuntu0.7     gettext=0.19.8.1-6ubuntu0.3     python3=3.6.7-1~18.04     python3-pip=9.0.1-2.3~ubuntu1.18.04.1     cmake=3.17.3-0kitware1     ninja-build=1.8.2-1' returned a non-zero code: 100

In the DockerFile it should be python3-pip=9.0.1-2.3~ubuntu1.18.04.2 instead.
Shouldn't we anchor it to a specific version of bionic in the FROM clause?

Aug 24 2020, 5:49 AM · Restricted Project
vsavchenko added a comment to D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size.
In D86295#2231760, @NoQ wrote:

I believe @vsavchenko's docker thingy already knows how to do that.

Yep, it sure does! Additionally, there is a benchmark subcommand that can chart memory consumption for measured projects.

Could you point me to some docs to help getting started? I haven't used this docker image.

Aug 24 2020, 3:54 AM · Restricted Project
vsavchenko added a comment to D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size.
In D86295#2231760, @NoQ wrote:

I believe @vsavchenko's docker thingy already knows how to do that.

Aug 24 2020, 2:16 AM · Restricted Project
vsavchenko added inline comments to D86027: [analyzer] Add bool operator modeling for unque_ptr.
Aug 24 2020, 1:09 AM · Restricted Project
vsavchenko added inline comments to D86293: [analyzer] Add modeling of Eq operator in smart ptr.
Aug 24 2020, 1:04 AM · Restricted Project
vsavchenko accepted D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h.

Awesome! I will submit the patch!

Aug 24 2020, 12:53 AM · Restricted Project

Aug 13 2020

vsavchenko accepted D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis..

Awesome, looks great!

Aug 13 2020, 9:58 AM · Restricted Project
vsavchenko committed rG9cbfdde2ea06: [analyzer] Fix crash with pointer to members values (authored by vsavchenko).
[analyzer] Fix crash with pointer to members values
Aug 13 2020, 8:05 AM