[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-07 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. In D92634#2484404 , @steakhal wrote: > Here is a link for our results on a few more projects. It might be useful for > you. > https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=D92634&items-per-page=50&sor

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-07 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > Typically in such cases bug visitors should be added/improved until it is > clear from the user-facing report why does the analyzer think so. They'd > highlight the important events, prevent path pruning, and potentially > suppress reports if the reason is dis

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. I have run clang static analysis on random open source projects. The very first finding that I look at seems (to me) to be a false positive. :-( My code seems to think that a variable `print_count` has the value INT_MAX for some reason and to me that seems impos

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > I also agree with @NoQ's D92634#2478703 > comment. well.. maybe it's better that I stop programming then and take this code I had and test it out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-05 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > BTW, I cannot optimize function f to returning zero directly with GCC-10.2.1 > and Clang-10.0.1 under -O3. Should I add any other flags? Or it is version > specific? Yeah I can't reproduce that with latest gcc/clang neither. Try gcc 4.x and gcc 5.x. > But fo

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-04 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > However, the mainstream compilers like GCC and Clang implement this as the > overflowed value, and some programmers also use this feature to do some > tricky things. hmm.. you mean if some -fwrapv flag is used right. yes I should disable this checking then.

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-02 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > Besides, the return value should be the exact value computed from the two > integers, even unknown, rather than undefined. As the developers may overflow > an integer on purpose. I am not sure what you mean. If there is undefined behavior then the value shoul

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2021-01-01 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki reclaimed this revision. danielmarjamaki added a comment. ok.. thanks for the reviews. I will see if I can make some new check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92634/new/ https://reviews.llvm.org/D92634 __

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2020-12-14 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. No reviews => I will not contribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92634/new/ https://reviews.llvm.org/D92634 ___

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

2020-12-03 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki created this revision. danielmarjamaki added reviewers: NoQ, xazax.hun. Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. danielmarjamaki requested review of this r

[PATCH] D38718: [Sema] No -Wtautological-pointer-compare warning on variables within parentheses

2018-01-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. Erik and I will not continue working on this. Feel free to take over the patch or write a new patch. Repository: rL LLVM https://reviews.llvm.org/D38718 ___ cfe-commits ma

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

2018-01-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. Herald added subscribers: llvm-commits, a.sidorin, rnkovacs. I will not continue working on this. Feel free to take over the patch or write a new patch. Repository: rL LLVM https://reviews.llvm.org/D39049 __

[PATCH] D30489: [analyzer] catch out of bounds for VLA

2018-01-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. Herald added subscribers: llvm-commits, a.sidorin, rnkovacs. I will not continue working on this. Feel free to take over the patch or write a new patch. Repository: rL LLVM https://reviews.llvm.org/D30489 __

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2018-01-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. Herald added subscribers: llvm-commits, a.sidorin, rnkovacs. I will not continue working on this. Feel free to take over the patch or write a new patch. Repository: rL LLVM https://reviews.llvm.org/D36471 __

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2018-01-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. Herald added subscribers: llvm-commits, a.sidorin, rnkovacs. I will not continue working on this. Feel free to take over the patch or write a new patch. Repository: rL LLVM https://reviews.llvm.org/D37897 __

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

2017-11-17 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > So what are the arguments that are passed to getSimplifiedOffset() in that > case? 0? That does not seem to be correct. yes. so the conclusion is: - this code does not work - this code is untested - this code is not even used in the use cases it was intended

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

2017-11-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > Could you do a similar analysis that I did above to check why does this not > work for the multidimensional case? (I.e.: checking what constraints are > generated and what the analyzer does with them.) the "location.dump()" will just say "x". the ProgramState

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-11-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 121726. danielmarjamaki added a comment. Herald added a subscriber: szepet. I have updated the patch so it uses evalBinOpNN. This seems to work properly. I have a number of TODOs in the test cases that should be fixed. Truncations are not handled pro

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

2017-10-26 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > Do you mind writing some tests with multidimensional arrays to check what do > we lose if we remove that code? I have spent a few hours trying to write a test case that shows there is false negatives caused by this change. And fail. I see lots of false negati

[PATCH] D30489: [analyzer] catch out of bounds for VLA

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 119590. danielmarjamaki added a comment. Herald added a subscriber: szepet. As suggested, use a ProgramState trait to detect VLA overflows. I did not yet manage to get a SubRegion from the DeclStmt that matches the location SubRegion. Therefore I am

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:139 -DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D, - bool RefersToEnclosingVariableOrCapture, - bool Get

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > I think it is much better when the assert failure tells the developer _what_ > value is failing, rather than saying "oops we are dead". yes of course, more informative assert messages is better. https://reviews.llvm.org/D38986 _

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. I like this patch overall.. here are some stylistic nits. Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:610 +} else { + if (*lInt >= *rInt) { +newRhsExt = lInt->getExtValue() - rInt->getExtVal

[PATCH] D39031: [Analyzer] Correctly handle parameters passed by reference when bodyfarming std::call_once

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a reviewer: danielmarjamaki. danielmarjamaki added a comment. Stylistically this looks pretty good to me. Just a minor nit. Comment at: lib/Analysis/BodyFarm.cpp:389 + for (unsigned int i = 2; i < D->getNumParams(); i++) { + +const ParmVarDecl *PDecl

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

2017-10-18 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki created this revision. Herald added a subscriber: szepet. Example code: void test3_simplified_offset(int x, unsigned long long y) { int buf[100]; if (x < 0) x = 0; for (int i = y - x; i > 0 && i < 100; i++) buf[i] = 0; // no-warning } Without this patc

[PATCH] D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type.

2017-10-16 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. LGTM https://reviews.llvm.org/D38801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

2017-10-16 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. LGTM.. however I would like approval from somebody else also. https://reviews.llvm.org/D38921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-10-16 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-13 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123 + // Is variable changed anywhere in TU? + for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) { +if (isChanged(D, VD)) danielmarjamaki w

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-13 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 118947. danielmarjamaki added a comment. Track modification of global static variables in CallGraph construction Repository: rL LLVM https://reviews.llvm.org/D37897 Files: include/clang/AST/Decl.h include/clang/StaticAnalyzer/Core/PathSensiti

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-12 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123 + // Is variable changed anywhere in TU? + for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) { +if (isChanged(D, VD)) danielmarjamaki w

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-12 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123 + // Is variable changed anywhere in TU? + for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) { +if (isChanged(D, VD)) dcoughlin wrote:

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-11 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D37897#892667, @dcoughlin wrote: > Apologies for the delay reviewing! As I noted inline, I'm pretty worried > about the performance impact of this. Is it possible to do the analysis in a > single traversal of the translation unit? I

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-10-11 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL315462: [Analyzer] Clarify error messages for undefined result (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D30295?vs=116865&id=118620#toc Repository: rL LLVM htt

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-10-10 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38718: [Sema] No -Wtautological-pointer-compare warning on variables within parentheses

2017-10-10 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. LGTM! However I would like to see a review from somebody else also. There are a number of diagnostics that might be affected. The Sema::DiagnoseAlwaysNonNullPointer diagnoses these: diag::warn_this_null_compare diag::warn_this_bool_conversion diag::warn_address_

[PATCH] D38718: Patch to Bugzilla 20951

2017-10-10 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a reviewer: danielmarjamaki. danielmarjamaki added a comment. I think a test for -Wtautological-pointer-compare should be added that shows that the bug is fixed. Comment at: test/Sema/conditional-expr.c:84 + //char x; + return (((x != ((void *) 0)) ? (*

[PATCH] D38675: [analyzer] MisusedMovedObjectChecker: Moving the checker out of alpha state

2017-10-10 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D38675#891912, @xazax.hun wrote: > In https://reviews.llvm.org/D38675#891750, @danielmarjamaki wrote: > > > > However, the checker seems to work with a low false positive rate. (<15 > > > on the LLVM, 6 effectively different) > > > >

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

2017-10-09 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. LGTM https://reviews.llvm.org/D38674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38675: [analyzer] MisusedMovedObjectChecker: Moving the checker out of alpha state

2017-10-09 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a reviewer: danielmarjamaki. danielmarjamaki added a comment. > However, the checker seems to work with a low false positive rate. (<15 on > the LLVM, 6 effectively different) This does not sound like a low false positive rate to me. Could you describe what the false posi

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki marked an inline comment as done. danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:107 +/** Recursively check if variable is changed in code. */ +static bool isChanged(const Stmt *S, const VarDecl *VD, bool Write) { + if (

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-10-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 117956. danielmarjamaki added a comment. Herald added a subscriber: szepet. Fixes according to review comments. Reuse ast matchers in LoopUnrolling.cpp. Avoid some recursion (however the isChanged() is still recursive but it is very small and simple)

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-10-04 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-10-04 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2017-09-29 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL314499: [Sema] Suppress warnings for C's zero initializer (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D28148?vs=82849&id=117107#toc Repository: rL LLVM https://r

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-09-27 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 116865. danielmarjamaki added a comment. fixed review comments Repository: rL LLVM https://reviews.llvm.org/D30295 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h lib/StaticAnalyzer/Checkers/ConversionChecker.cpp lib

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-09-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-09-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D37897#871858, @xazax.hun wrote: > Out of curiosity, does the false positive disappear after making the static > variables const? Yes that fixes the false positive Repository: rL LLVM https://reviews.llvm.org/D37897 _

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-09-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 115385. danielmarjamaki added a comment. Minor cleanups. Changed names. Updated comments. Repository: rL LLVM https://reviews.llvm.org/D37897 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngi

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

2017-09-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki created this revision. I saw a false positive where the analyzer made wrong conclusions about a static variable. Static variables that are not written have known values (initialized values). This is the (simplified) code that motivated me to create this patch: static char *al

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-09-14 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-09-14 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2017-09-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. This is not committed as far as I see.. do you have write permission or do you want that I commit it? https://reviews.llvm.org/D28148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-09-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 113969. danielmarjamaki added a comment. minor code cleanup Repository: rL LLVM https://reviews.llvm.org/D36471 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h lib/StaticAnalyzer/Core/ExprEngineC.cpp lib/StaticAna

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-09-06 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

2017-09-01 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki accepted this revision. danielmarjamaki added inline comments. This revision is now accepted and ready to land. Comment at: tools/scan-build-py/libscanbuild/analyze.py:165 +with open(filename, 'r') as in_file: +for line in in_file: +

[PATCH] D34512: Add preliminary Cross Translation Unit support library

2017-08-31 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. small nits Comment at: include/clang/CrossTU/CrossTranslationUnit.h:58 + +/// \brief This function can parse an index file that determines which +///translation unit contains which definition. I suggest that "can" is rem

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

2017-08-31 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: tools/scan-build-py/libscanbuild/analyze.py:165 +with open(filename, 'r') as in_file: +for line in in_file: +yield line I believe you can write: for line in op

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-08-31 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36670: misc-misplaced-widening-cast: fix assertion

2017-08-28 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL311984: [clang-tidy] Fix 'misc-misplaced-widening-cast' assertion error. (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D36670?vs=110940&id=113026#toc Repository: rL

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-08-23 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36672: [clang-tidy] readability-non-const-parameter: fixit on all function declarations

2017-08-14 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. LGTM. But others should approve. Repository: rL LLVM https://reviews.llvm.org/D36672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36670: misc-misplaced-widening-cast: fix assertion

2017-08-14 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. LGTM. I let others approve this. Repository: rL LLVM https://reviews.llvm.org/D36670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-09 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. Should evalAPSInt() have machinery to do standard sign/type promotions? I suggest that I add one more argument `bool promote = false`, do you think that sounds good? Repository: rL LLVM https://reviews.llvm.org/D36471 _

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-09 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 110378. danielmarjamaki added a comment. Refactoring, use BasicValueFactory::evalAPSInt Repository: rL LLVM https://reviews.llvm.org/D36471 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h lib/StaticAnalyzer/Core/Exp

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-08 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. In https://reviews.llvm.org/D36471#835410, @xazax.hun wrote: > Can't you reuse somehow some machinery already available to evaluate the > arithmetic operators? Those should already handle most of your TODOs and > overflows. Sounds good.. I have not seen that m

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-08 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 110220. danielmarjamaki added a comment. A minor code cleanup. No functional change. Repository: rL LLVM https://reviews.llvm.org/D36471 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h lib/StaticAnalyzer/Core/ExprEn

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-08 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki created this revision. In the code below the division result should be a value between 5 and 25. if (a >= 10 && a <= 50) { int b = a / 2; } This patch will calculate results for additions, subtractions and divisions. I intentionally do not try to handle all possible case

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-08-03 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 109590. danielmarjamaki added a comment. Cleaned up the patch a little. Thanks Gabor for telling me about SValBuilder::getKnownValue() Repository: rL LLVM https://reviews.llvm.org/D30295 Files: include/clang/StaticAnalyzer/Core/PathSensitive/C

[PATCH] D36141: [StaticAnalyzer] Fix FP in UnreachableCodeChecker

2017-08-02 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL309799: [StaticAnalyzer] Fix false positives for unreachable code in macros. (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D36141?vs=109086&id=109294#toc Repository:

[PATCH] D36141: [StaticAnalyzer] Fix FP in UnreachableCodeChecker

2017-08-01 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki created this revision. Herald added a subscriber: whisperity. This fixes a FP. Without the fix, the checker says that "static int x;" is unreachable. Repository: rL LLVM https://reviews.llvm.org/D36141 Files: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp test/An

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-06-22 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki marked 4 inline comments as done. danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:126 + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined due to shift count >=

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-06-22 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 103585. danielmarjamaki added a comment. Fix review comments Repository: rL LLVM https://reviews.llvm.org/D30295 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h lib/StaticAnalyzer/Checkers/ConversionChecker.cpp lib/S

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

2017-06-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. I will not continue working on this checker Repository: rL LLVM https://reviews.llvm.org/D32346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

2017-06-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. danielmarjamaki marked an inline comment as done. Closed by commit rL305669: [analyzer] Fix logical not for pointers with different bit width (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.or

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

2017-05-26 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-05-26 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

2017-05-16 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 99114. danielmarjamaki added a comment. Fix review comments Repository: rL LLVM https://reviews.llvm.org/D31029 Files: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h include/clang/StaticAnalyzer/Core/PathSensitive/SValBui

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-05-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 99022. danielmarjamaki added a comment. renamed exprComparesTo to svalComparesTo Repository: rL LLVM https://reviews.llvm.org/D30295 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h lib/StaticAnalyzer/Checkers/Conversio

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-05-15 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 98970. danielmarjamaki added a comment. minor tweak Repository: rL LLVM https://reviews.llvm.org/D30295 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h lib/StaticAnalyzer/Checkers/ConversionChecker.cpp lib/StaticAnal

[PATCH] D31650: [Analyzer] Detect when function pointer is freed

2017-05-02 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL301913: [analyzer] Detect bad free of function pointers (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D31650?vs=95929&id=97432#toc Repository: rL LLVM https://revi

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-04-27 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:116 +return false; + ConstraintManager &CM = State->getConstraintManager(); + ProgramStateRef StTrue, StFalse; xazax.hun wrote: > Any reason why do you get the con

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki accepted this revision. danielmarjamaki added a comment. This revision is now accepted and ready to land. If you have svn write permission then please do it. If you need svn write permission, please see http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access Repositor

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. Ping. Repository: rL LLVM https://reviews.llvm.org/D30295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D30489: [analyzer] catch out of bounds for VLA

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. This revision now requires changes to proceed. Ping. Any comments? Repository: rL LLVM https://reviews.llvm.org/D30489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/AST/ASTContext.cpp:1495 + ASTUnit *Unit = nullptr; + StringRef ASTFileName; + auto FnUnitCacheEntry = FunctionAstUnitMap.find(MangledFnName); As far as I see you can move this ASTFileName declaration down.

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:8 + +In the example code below the developer probably wanted to make room for an extra char in the allocation but misplaced the addition. + JonasToth wrote:

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. I am thinking about making my check more strict so it only warns in allocations. I believe the example code is much more motivating when there is allocation. In https://reviews.llvm.org/D32346#733430, @JonasToth wrote: > My thoughts on the check added. > Have

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

2017-04-25 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 96526. danielmarjamaki added a comment. Fixed review comments. Made code examples and documentation more motivational. Repository: rL LLVM https://reviews.llvm.org/D32346 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/Rea

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

2017-04-24 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. Thanks for all comments. I am working on fixing them. Updated patch will be uploaded soon. > Have you run it over a big codebase? What is the turnout? I did not see this warning yet. Maybe after fixing the comments (::strlen) there will be a difference. In htt

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

2017-04-21 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki created this revision. Herald added a subscriber: mgorny. I propose this new readability checker. Repository: rL LLVM https://reviews.llvm.org/D32346 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/Str

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-20 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. you can ignore my comment ... LGTM I don't want to accept this revision. Hopefully NoQ or somebody else can do that. Repository: rL LLVM https://reviews.llvm.org/D30771 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-20 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. I don't have further comments except that I would personally rewrite: // Get the value of the size argument. SVal TotalSize = State->getSVal(Arg1, LCtx); if (SuffixWithN) { const Expr *Arg2 = CE->getArg(2); TotalSize = evalMulForBufferSize(C, Arg1, A

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-20 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:337 + const Expr *BlockBytes, + ProgramStateRef State); static ProgramStateRef CallocMem(CheckerContext &C, cons

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:788 +SVal MallocChecker::SValBinMulOp(CheckerContext &C, const Expr *Blocks, + const Expr *BlockBytes, ProgramStateRef State) { dan

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki added a comment. > I hold the view that I need to respect original developers' code, and it need > a Global Patch for Capital variable, just like KDE's Use nullptr everywhere Yes I was too picky. Please respect the original developers' code. Comment at: lib/St

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

2017-04-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki requested changes to this revision. danielmarjamaki added inline comments. This revision now requires changes to proceed. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:788 +SVal MallocChecker::SValBinMulOp(CheckerContext &C, const Expr *Blocks, +

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-04-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki marked 4 inline comments as done. danielmarjamaki added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46 -} // end GR namespace +bool isExprResultConformsComparisonRule(CheckerContext &C, +

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

2017-04-19 Thread Daniel Marjamäki via Phabricator via cfe-commits
danielmarjamaki updated this revision to Diff 95740. danielmarjamaki added a comment. Fix review comments - renamed - reorder function arguments (CheckerContext last) Repository: rL LLVM https://reviews.llvm.org/D30295 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.

  1   2   >