Page MenuHomePhabricator

dkrupp (Daniel Krupp)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 8 2015, 1:45 AM (205 w, 6 d)

Recent Activity

Tue, May 14

dkrupp added a comment to D57858: [analyzer] Add a new frontend flag to display all checker options.
In D57858#1500635, @NoQ wrote:

Some alpha checkers are considerably more mature than others and are quite usable. In our experience, there are some users who are keen to run these checkers on their code and report back any false positives to us. So in this sense these are not "developer only" checkers. So I think we should let the users list them, read their descriptions and try them out. Some of them will come back with useful feedback as to how to improve them further.

What are such checkers currently? Like, the ones that aren't clearly "missing limbs" and that have somebody happy to address feedback sent against them?

Do you have a chance to call out to your users for testing the checker and actively request feedback, as @Szelethus did on the mailing list?

I feel that we could do some sort of "early access checkers" programme, but i believe this would require a more careful PR than just dumping a list of alpha checkers on our users' heads.

Some users would not care if the checker gives some more false positives than the "mature" checkers if they can catch some true positives with them.

Yeah, and these are pretty much the users we're trying to protect from themselves :)

Tue, May 14, 4:42 AM · Restricted Project

Mon, May 13

dkrupp added a comment to D57858: [analyzer] Add a new frontend flag to display all checker options.
In D57858#1498640, @NoQ wrote:

So, like, the global picture is as follows. In our case the Driver (i.e., --analyze) is not much more user facing than frontend flags. It's still fairly unusable directly, as our Static Analyzer is generally not a command-line tool. The real user-facing stuff is the GUIs such as scan-build or CodeChecker. These GUIs decide themselves on what options they want to expose. For instance, you have a right to decide that CodeChecker shouldn't support the aggressive mode of the move-checker and don't expose it as an option in your GUI. In this sense it's not really useful to provide a centralized -help of all user-facing options.

But it sounds as if you want to change this situation and provide a single source of truth on what are the user-facing options. Particular GUIs can still ignore them, but you don't want to hardcode flags in CodeChecker, but instead you want to rely on clang to provide the list of supported options for you and, as a side effect, for scan-build users (if you also add a scan-build help flag). I'm totally in favor of crystallizing such list of user-facing flags, and this patch sounds like a good step in that direction, assuming that non-user-facing options are hidden.

That describes my intention quite well :)

I'm still in favor of hiding alpha checkers (as they are for development only, much like debug flags; i'd recommend hiding them in the CodeChecker UI as well)

Now, why do we care about frontend/driver flags when they're unusable by definition? That's because we have a mental trauma after seeing a few powerusers actively explore those flags, observe that they don't work, and then tell everybody that the Analyzer is broken. So there's a threshold, based on a tiny but painful bit of practical experience, that says that documentation of developer-only features on llvm.org or in code comments is fine, but documentation printed by the released binary itself is not fine.

What you say sounds very reasonable. Still, I am kind of hesitant about hiding all alpha checkers: I initially intended to hide only are developer-only checkers (modeling, debug). I guess if we define alpha checkers (as you stated numerous times) as incomplete, under development, are missing half their limbs and crash if you look at them the wrong way, sure, they belong in the developer-only category. But checkers such as mine (UninitializedObjectChecker), for the longest time were very stable, have been enabled by default for our internal projects, despite only recently moving out of alpha.

Then agaaain, if we're that stubborn about alpha checkers, we could might as well dig them out of -analyzer-checker-help-hidden, and leave the rest there. Untangling what alpha checkers depend on one another could be solved by making yet another frontend flag that would display checker dependencies, which would be super easy since D54438, or create an -analyzer-checker-help-alpha flag that would display alpha, but not developer-only checkers. @dkrupp @o.gyorgy Do you have any feelings on this?

and we should probably automatically hide options of checker that are hidden.

Checker options are a different kind of animal entirely. I think we should definitely let the user fine-tune some modeling checkers, for instance, unix.DynamicMemoryModeling:Optimistic, despite us not really wanting to let anyone (even developers really) mess around whether unix.DynamicMemoryModeling should be enabled. While that specific option is, to put it nicely, a little esoteric, making some decisions the analyzer makes less conservative, or limiting state splits to help performance may be desirable in the future.

Let's move the rest of the discussion directly related to hiding checker options to D61839!

Mon, May 13, 8:37 AM · Restricted Project

Fri, May 3

dkrupp updated the diff for D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

I have fixed all your comments and rebased the patch to the latest master.

Fri, May 3, 10:13 AM · Restricted Project

Apr 8 2019

dkrupp added inline comments to D60281: [analyzer] Add docs for cplusplus.InnerPointer.
Apr 8 2019, 12:02 AM · Restricted Project

Mar 26 2019

dkrupp added a comment to D57858: [analyzer] Add a new frontend flag to display all checker options.

@dcoughlin I don't necessarily agree with you.
Let me explain why we think this feature is important.

Mar 26 2019, 7:25 AM · Restricted Project

Jan 4 2019

dkrupp updated the diff for D54429: [analyzer] Creating standard Sphinx documentation.

Thanks @NoQ .
So I created a very simple main page with the table of contents only http://cc.elte.hu/clang-docs/docs/html/ClangStaticAnalyzer.html

Jan 4 2019, 4:56 AM · Restricted Project

Dec 21 2018

dkrupp added a comment to D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

Thanks for your comments. I fixed them all. I also added the handling of redundant sizeof() and alignof() operators on the way. Please check if OK now...

Dec 21 2018, 6:13 AM · Restricted Project
dkrupp updated the diff for D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

All comments fixed.

Dec 21 2018, 6:03 AM · Restricted Project

Dec 10 2018

dkrupp added a comment to D54429: [analyzer] Creating standard Sphinx documentation.

@dcoughlin @NoQ ping...

Dec 10 2018, 5:19 AM · Restricted Project

Dec 5 2018

dkrupp added a comment to D55255: Fix a false positive in misplaced-widening-cast.

Committed, Thank you for the patch! Was there a bug-report for this issue? If yes can you please close it/reference?

Dec 5 2018, 12:36 AM · Restricted Project

Dec 4 2018

dkrupp updated the diff for D55255: Fix a false positive in misplaced-widening-cast.

Comments addressed. Please commit if looks good, I don't have commit rights.
Thanks.

Dec 4 2018, 4:46 AM · Restricted Project
dkrupp created D55255: Fix a false positive in misplaced-widening-cast.
Dec 4 2018, 12:40 AM · Restricted Project

Dec 3 2018

dkrupp added inline comments to D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.
Dec 3 2018, 5:02 AM · Restricted Project
dkrupp updated the diff for D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

new undef/defined testcase added

Dec 3 2018, 4:59 AM · Restricted Project

Dec 1 2018

dkrupp added a comment to D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

I see your point, but here's why I think it isn't a bug: I like to see macros as constexpr variables, and if I used those instead, I personally wouldn't like to get a warning just because they have the same value. In C, silencing such a warning isn't even really possible.

Another interesting thought, @donat.nagy's check works by comparing actual nodes of the AST, while this one would work with Lexer, but they almost want to do the same thing, the only difference is that the first checks whether two pieces of code are equivalent, and the second checks whether they are the same. Maybe it'd be worth to extract the logic into a simple areStmtsEqual(const Stmt *S1, const Stmt *S2, bool ShouldCompareLexically = false) function, that would do AST based comparison if the last param is set to false, and would use Lexer if set to true. After that, we could just add command line options to both of these checks, to leave it up to the user to choose in between them. Maybe folks who suffer from heavily macro-infested code would rather bear the obvious performance hit Lexer causes for little more precise warnings, and (for example) users of C++11 (because there are few excuses not to prefer constexpr there) could run in AST mode.

edit: I'm not actually all that sure about the performance hit. Just a guess.

But I'm just thinking aloud really.

Dec 1 2018, 3:18 AM · Restricted Project
dkrupp added a comment to D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

@JonasToth this is the Lexer based expression equality check I talked about in D54757#1311516. The point of this patch is that the definition is a macro sure could be build dependent, but the macro name is not, so it wouldn't warn on the case I showed. What do you think?

Yes, this approach is possible.
IMHO it is still a bug/redudant if you do the same thing on both paths and warning on it makes the programmer aware of the fact. E.g. the macros might have been something different before, but a refactoring made them equal and resulted in this situation.

Dec 1 2018, 3:12 AM · Restricted Project
dkrupp updated the diff for D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.

-clang-format applied
-clang:: namespace qualifiers removed

Dec 1 2018, 3:01 AM · Restricted Project

Nov 30 2018

dkrupp created D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check.
Nov 30 2018, 8:00 AM · Restricted Project

Nov 23 2018

dkrupp added a comment to D54429: [analyzer] Creating standard Sphinx documentation.

@dcoughlin could you please look into this?

Nov 23 2018, 2:51 AM · Restricted Project

Nov 13 2018

dkrupp updated the diff for D54429: [analyzer] Creating standard Sphinx documentation.

-scanbuild and xcode pictures are included now
-intro text ("What is Static Analysis?" etc.) are put under the Introduction section
-Download section is created, but I am not sure how well was the this Mac OSX binary release section was maintained. Should users really download from this site or through a package manager instead?

Nov 13 2018, 7:16 AM · Restricted Project

Nov 12 2018

dkrupp updated the diff for D54429: [analyzer] Creating standard Sphinx documentation.

making the diff full context.

Nov 12 2018, 8:16 AM · Restricted Project
dkrupp created D54429: [analyzer] Creating standard Sphinx documentation.
Nov 12 2018, 8:05 AM · Restricted Project

Oct 17 2018

dkrupp added inline comments to D53024: [analyzer][www] Add more open projects.
Oct 17 2018, 2:17 AM

Jul 18 2018

dkrupp added a comment to D30691: [analyzer] Support for naive cross translational unit analysis.

Which means that for some calls we aren't even trying to make a CTU lookup.

Thanks @NoQ, we will take a look at it!

Jul 18 2018, 4:47 AM

Jul 13 2018

dkrupp added a comment to D48831: [Analyzer] alpha.unix.cstring.OutOfBounds checker enable/disable fix.

@NoQ do we need any more update to this patch? Thanks.

Jul 13 2018, 6:04 AM

Jul 3 2018

dkrupp added inline comments to D48831: [Analyzer] alpha.unix.cstring.OutOfBounds checker enable/disable fix.
Jul 3 2018, 6:30 AM
dkrupp updated the diff for D48831: [Analyzer] alpha.unix.cstring.OutOfBounds checker enable/disable fix.

The patch has been updated.
Changes:

Jul 3 2018, 6:21 AM

Jul 2 2018

dkrupp added reviewers for D48831: [Analyzer] alpha.unix.cstring.OutOfBounds checker enable/disable fix: baloghadamsoftware, NoQ.
Jul 2 2018, 8:00 AM
dkrupp created D48831: [Analyzer] alpha.unix.cstring.OutOfBounds checker enable/disable fix.
Jul 2 2018, 7:56 AM

Apr 16 2018

dkrupp added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Would be interesting to extend this checker (maybe in an upcoming patch) to report on uninitialized members not only in constructors, but also copy constructors and move constructors.

Apr 16 2018, 4:37 AM

Dec 15 2017

dkrupp added inline comments to D41150: [CFG] Adding new CFGStmt LoopEntrance for the StaticAnalyzer.
Dec 15 2017, 5:01 AM

Dec 13 2017

dkrupp requested changes to D41150: [CFG] Adding new CFGStmt LoopEntrance for the StaticAnalyzer.
Dec 13 2017, 12:28 AM

Nov 3 2017

dkrupp added a reviewer for D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr: bruno.
Nov 3 2017, 8:52 AM
dkrupp added a reviewer for D32947: [ASTImporter] FriendDecl importing improvements: bruno.
Nov 3 2017, 8:51 AM
dkrupp added a reviewer for D38692: [ASTImporter] Support importing FunctionTemplateDecl and CXXDependentScopeMemberExpr: bruno.
Nov 3 2017, 8:50 AM
dkrupp added a reviewer for D30876: [ASTImporter] Unnamed structs import: bruno.
Nov 3 2017, 8:49 AM
dkrupp added a reviewer for D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr: bruno.
Nov 3 2017, 8:49 AM

Oct 15 2017

dkrupp requested changes to D30691: [analyzer] Support for naive cross translational unit analysis.

Please fix the incompatibility between analyze-build and lib/CrossTU in the format of externalFnMap.txt mappfing file.

Oct 15 2017, 7:28 AM

Aug 21 2017

dkrupp updated subscribers of D34512: Add preliminary Cross Translation Unit support library.

The creation of this library (libCrossTU) is approved for importing function definitions. @zaks.anna, @NoQ , @klimek could you please help us reviewing the code itself?

Aug 21 2017, 1:42 AM

Jun 19 2017

dkrupp added a comment to D30691: [analyzer] Support for naive cross translational unit analysis.

It would be best to just add the scan-build-py support to the tree, especially, since the new scrips are not tested.

OK. We will update this patch with the scan-build-py changes and remove the ctu-build.py and ctu-analyze.py scripts.

Jun 19 2017, 10:26 AM

Jun 14 2017

dkrupp added a comment to D30691: [analyzer] Support for naive cross translational unit analysis.

Thanks for the reviews so far.
I think we have addressed all major concerns regarding this patch:

Jun 14 2017, 8:48 AM

Dec 16 2016

dkrupp retitled D27849: crash in MallocChecker from to crash in MallocChecker.
Dec 16 2016, 7:01 AM

Sep 19 2016

dkrupp added a comment to D24307: calculate extent size for memory regions allocated by C++ new expression.

Thanks. Gabor, could you please merge this? I don't have commit right.

Sep 19 2016, 3:27 AM

Sep 13 2016

dkrupp updated subscribers of D19586: Misleading Indentation check.
Sep 13 2016, 9:59 AM
dkrupp updated subscribers of D21298: [Clang-tidy] delete null check.
Sep 13 2016, 9:47 AM
dkrupp updated subscribers of D22346: [Clang-tidy] CERT-MSC50-CPP (std:rand() ).
Sep 13 2016, 9:41 AM

Sep 12 2016

dkrupp added a comment to D24307: calculate extent size for memory regions allocated by C++ new expression.

issues fixed

Sep 12 2016, 9:36 AM

Sep 9 2016

dkrupp added inline comments to D24307: calculate extent size for memory regions allocated by C++ new expression.
Sep 9 2016, 6:23 AM
dkrupp updated the diff for D24307: calculate extent size for memory regions allocated by C++ new expression.

I tried to address all your comments.

Sep 9 2016, 6:16 AM
dkrupp added inline comments to D24307: calculate extent size for memory regions allocated by C++ new expression.
Sep 9 2016, 1:44 AM

Sep 8 2016

dkrupp added inline comments to D24307: calculate extent size for memory regions allocated by C++ new expression.
Sep 8 2016, 12:58 AM

Sep 7 2016

dkrupp retitled D24307: calculate extent size for memory regions allocated by C++ new expression from to calculate extent size for memory regions allocated by C++ new expression.
Sep 7 2016, 11:06 AM

Nov 16 2015

dkrupp updated subscribers of D9600: Add scan-build python implementation.
Nov 16 2015, 5:01 AM

Oct 22 2015

dkrupp added a comment to D12906: [RFC] Bug identification("issue_hash") change for CmpRuns.py.

its a good idea to include in LLVM/Clang i will propose it

Oct 22 2015, 6:19 AM

Oct 21 2015

dkrupp added a comment to D12906: [RFC] Bug identification("issue_hash") change for CmpRuns.py.

In D10305#224956, @zaks.anna wrote:
For example, you could keep the information about the reports in the plist files and use those to
render the reports in HTML.

If you're okay with adding HTML file name in plist for each bug, I will prepare a new patch for that.
Thanks for the review!

I think you misunderstood my comment. I am not talking about using the existing HTML files here but rather having an HTML viewer, which you could use to browse source code. This viewer would be extended to read the bug reports from the plist files and display them. Currently, we create an html file with source code + report info for each bug report. This does not scale when you have a lot of reports on a single large file (ex: sqlite).

What I describe above is a larger project. What workflow are you trying to support? I think adding the issue hash to the HTML file is fine if you find it to be useful for your workflow...

Oct 21 2015, 9:50 AM

Sep 22 2015

dkrupp added a comment to D10305: [Clang Static Analyzer] Bug identification.

Regarding testing:
I think we should create a RecursiveASTvistor based "test checker" that matches every statement and declaration and reports a bug there.
Then we could create a test file similar to what we have in /tools/clang/test/Analysis/diagnostics/report-issues-within-main-file.cpp
where the expected plist output can be written at the end of the file.

Sep 22 2015, 2:02 AM

Jun 26 2015

dkrupp updated subscribers of D10156: [Static Analyzer] Fix Analysis being skipped for code with declarations in .h file.
Jun 26 2015, 12:29 AM

Jun 24 2015

dkrupp added a comment to D10305: [Clang Static Analyzer] Bug identification.

when calculating the has for the following fields

Jun 24 2015, 2:55 AM

Jun 8 2015

dkrupp updated subscribers of D9555: [clang-tidy] Support for Static Analyzer plugins.
Jun 8 2015, 1:54 AM