[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

danielmarjamaki added a comment. I would recommend that this is either fixed soon or that we commit my changes so it can be implemented more properly later. Right now users will see false positives. Repository: rL LLVM https://reviews.llvm.org/D31097 _

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

danielmarjamaki added a comment. I would propose that I rename and cleanup RangeConstraintManager::uglyEval() and add it. When I tested it, the Z3 does not seem to handle this. Repository: rL LLVM https://reviews.llvm.org/D30489 ___ cfe-commits

[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function

danielmarjamaki added a comment. Please click "Done" on fixed review comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2038 + // If the size can be nonzero, we have to check the other arguments. + if (StateNonZeroSize) { +State = StateNonZeroSize; -

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

danielmarjamaki added a comment. > Or I can do it for you if you wish. yes please Repository: rL LLVM https://reviews.llvm.org/D31097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

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

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] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

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] D30489: [analyzer] catch out of bounds for VLA

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] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

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] D38718: [Sema] No -Wtautological-pointer-compare warning on variables within parentheses

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] D19586: Misleading Indentation check

danielmarjamaki added inline comments. Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:42 +const Stmt *Inside = nullptr; + +if (const auto *CurrentIf = dyn_cast(CurrentStmt)) { I would rename Inside to Inner. That would make InnerLoc mor

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

danielmarjamaki added a comment. I am not against that the error is shown as long as it's not misleading/wrong. To avoid misleading, in my humble opinion the error message should say "array index out of bounds". Repository: rL LLVM https://reviews.llvm.org/D28278

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

danielmarjamaki added a comment. In https://reviews.llvm.org/D28297#642523, @NoQ wrote: > Looks good. I assume the crash is in `getTypeInfo()`; do you have any idea > what are exact prerequisites for using this method? So that there were no > more crashes here. Yes. The crash happens during t

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rL295545: [analyzer] Fix crash in CastToStruct when there is no record definition (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D28297?vs=83062&id=89019#toc Repository:

[PATCH] D30192: [Sema] Detecting more array index out of bounds

danielmarjamaki created this revision. Diagnose array index out of bounds when there is overloaded C++ operators also. Repository: rL LLVM https://reviews.llvm.org/D30192 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/array-bounds.cpp Index: test/SemaCXX/array-bounds.cpp ===

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

danielmarjamaki reopened this revision. danielmarjamaki added a comment. This revision is now accepted and ready to land. I reverted the change because there were buildbot failures. Repository: rL LLVM https://reviews.llvm.org/D28297 ___ cfe-comm

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

danielmarjamaki added a comment. In https://reviews.llvm.org/D28278#677905, @zaks.anna wrote: > Does the code you added detects array out of bounds cases without false > positives? Is it an option to just have this checkers produce a more precise > error message in the specific case. > > A lot

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

danielmarjamaki created this revision. The error messages are confusing when shift result is undefined because the shift count is negative or exceeds the bit width. I have seen that users often draw the conclusion that Clang thinks some operand is uninitialized and determine that Clang shows a

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

danielmarjamaki updated this revision to Diff 89507. danielmarjamaki added a comment. It was reported in the bugzilla report that my first fix did not fix all crashes. A new example code was provided that triggered a new crash. I have updated the patch so both crashes are fixed. https://review

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

danielmarjamaki requested review of this revision. danielmarjamaki added a comment. I have updated the patch and want a new review. https://reviews.llvm.org/D28297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators

danielmarjamaki abandoned this revision. danielmarjamaki added a comment. I will not work on this in the near future Repository: rL LLVM https://reviews.llvm.org/D24861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

danielmarjamaki updated this revision to Diff 89540. danielmarjamaki added a comment. Making the error message more precise. https://reviews.llvm.org/D28278 Files: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp test/Analysis/uninit-vals-ps.c Index: test/Analysis/uninit-vals-ps.c =

[PATCH] D30157: [analyzer] Improve valist check

danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:189 + const auto *EReg = dyn_cast_or_null(Reg); + return EReg && VaListModelledAsArray ? EReg->getSuperRegion() : Reg; +} I would personally recommend parentheses a

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

danielmarjamaki updated this revision to Diff 89641. danielmarjamaki added a comment. Fixed review comment. Broke out function. Repository: rL LLVM https://reviews.llvm.org/D28278 Files: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp test/Analysis/uninit-vals-ps.c Index: test/Analy

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki created this revision. This patch clarify the error messages about uninitialized function arguments. It can be really hard to see the problem if there are 10-20 arguments like: printf("debug:", a, b.c, d, e, ...); with no info about which argument is uninitialized and with

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

This revision was automatically updated to reflect the committed changes. Closed by commit rL296326: [analyzer] clarify 'result is garbage value' when it is out of bounds (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D28278?vs=89641&id=89854#toc Repository:

[PATCH] D30192: [Sema] Detecting more array index out of bounds

This revision was automatically updated to reflect the committed changes. Closed by commit rL296477: [Sema] Detect more array index out of bounds when C++ overloaded operators are… (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D30192?vs=89183&id=90027#toc Rep

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki updated this revision to Diff 90030. danielmarjamaki added a comment. minor updates. Use llvm::getOrdinalNumber(). Use llvm::Twine. Repository: rL LLVM https://reviews.llvm.org/D30341 Files: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp test/Analysis/NewDelete-che

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki marked an inline comment as done. danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:211 // Generate a report for this bug. - StringRef Desc = - describeUninitializedArgumentInCall(Call, Is

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

danielmarjamaki created this revision. This is a work in progress patch. I try to detect such error: void foo(int X) { int Buf[X]; Buf[X] = 0; } The patch successfully detects this bug. However it writes FP when you try to take the address. I would like to know if you think my

[PATCH] D30157: [analyzer] Improve valist check

danielmarjamaki added a comment. I am running this checker right now on various projects. Here are currently seen results.. https://drive.google.com/open?id=0BykPmWrCOxt2STZMOXZ5OGlwM3c Feel free to look at it and see if there are FPs or TPs. https://reviews.llvm.org/D30157

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:211 // Generate a report for this bug. - StringRef Desc = - describeUninitializedArgumentInCall(Call, IsFirstArgument); + std::string Desc = +

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:211 // Generate a report for this bug. - StringRef Desc = - describeUninitializedArgumentInCall(Call, IsFirstArgument); + std::string Desc = +

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

danielmarjamaki added a comment. To me it seems that the extent is calculated properly in ArrayBoundsV2. Existing code: DefinedOrUnknownSVal extentVal = rawOffset.getRegion()->getExtent(svalBuilder); This ugly little debug code will extract the needed VLA information from the extentVal..

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki updated this revision to Diff 90471. danielmarjamaki added a comment. Fix review comment Repository: rL LLVM https://reviews.llvm.org/D30341 Files: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp test/Analysis/NewDelete-checker-test.cpp test/Analysis/diagnostics/u

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

danielmarjamaki added a comment. No the argument is not shown with tilde/column number. Code example: void f(int x, ...); void dostuff() { int x[10]; f(12,3,4,5,x[3],6,7,8); } Output: C:\Users\danielm>\llvm\build\Debug\bin\clang -cc1 -analyze -analyzer-checker=core test3.c

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

danielmarjamaki added a comment. > Also, in your state dumps no information is actually lost. The fact that the > value of variable sz is reg_$0 is trivial: you could ask the Store what's > the value of the variable sz and it'd say reg_$0 if there are no bindings > over it. Thanks. I have tr

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rL297187: [analyzer] Fix crashes in CastToStruct checker for undefined structs (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D28297?vs=89507&id=90900#toc Repository:

[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rL297283: [analyzer] Clarify 'uninitialized function argument' messages (authored by danielmarjamaki). Changed prior to commit: https://reviews.llvm.org/D30341?vs=90471&id=91012#toc Repository: rL LLVM

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

danielmarjamaki abandoned this revision. danielmarjamaki added a comment. I believe https://reviews.llvm.org/D32164 is better Repository: rL LLVM https://reviews.llvm.org/D31097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

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

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.

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

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] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

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] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

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

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

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

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

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] D32346: [clang-tidy] New readability check for strlen argument

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] D32346: [clang-tidy] New readability check for strlen argument

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

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

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

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] D30691: [analyzer] Support for naive cross translational unit analysis

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] D31029: [analyzer] Fix logical not for pointers with different bit width

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] D30489: [analyzer] catch out of bounds for VLA

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] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

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] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

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

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] D31650: [Analyzer] Detect when function pointer is freed

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

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] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

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] D31029: [analyzer] Fix logical not for pointers with different bit width

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] D92634: [Analyzer] Diagnose signed integer overflow

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

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

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

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

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

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

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

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

<    1   2