[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy added a comment. Herald added a subscriber: dkrupp. polite ping Repository: rC Clang https://reviews.llvm.org/D52730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164 +// double and possibly long double on some systems +RepresentsUntilExp = 53; break; + case 32: xazax.hun wrote: > A link to the source of the

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy updated this revision to Diff 170169. donat.nagy added a comment. Give a reference for the significand size values of the IEEE 754 floating point types; cleanup comments and formatting. Repository: rC Clang https://reviews.llvm.org/D52730 Files: lib/StaticAnalyzer/Checkers/Conv

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy marked 2 inline comments as done. donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164 +// double and possibly long double on some systems +RepresentsUntilExp = 53; break; + case 32: d

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163 + +switch (FloatingSize) { + case 64: NoQ wrote: > Continuing the float semantics discussion on the new revision - Did you > consider `llvm::APFloat`? (h

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy updated this revision to Diff 171678. donat.nagy added a comment. Use APFloat to determine precision of floating point type Additionally, fix a typo in the tests. Repository: rC Clang https://reviews.llvm.org/D52730 Files: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp tes

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy marked 7 inline comments as done. donat.nagy added a comment. I found a solution for determining the precision value of a floating point type. Is this acceptable? Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163 + +switch (FloatingSize) { +

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy updated this revision to Diff 172922. donat.nagy marked an inline comment as done. donat.nagy added a comment. Use ASTContext::getFloatTypeSemantics() Repository: rC Clang https://reviews.llvm.org/D52730 Files: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp test/Analysis/co

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy added a comment. Could someone with commit rights commit this patch (if it is acceptable)? I don't have commit rights myself. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:158-162 +unsigned FloatingSize = AC.getTypeSize(DestType); +// getAllO

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy created this revision. donat.nagy added reviewers: alexfh, hokein, aaron.ballman, xazax.hun, whisperity. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs, mgorny. Implement a check for detecting if/else if/else chains where two or more branches are Type I

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy updated this revision to Diff 174943. donat.nagy added a comment. Implement suggested improvements Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54757 Files: clang-tidy/bugprone/BranchCloneCheck.cpp clang-tidy/bugprone/BranchCloneCheck.h clang-tidy/bugprone/Bu

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy marked 6 inline comments as done. donat.nagy added a comment. An example of duplicated code in Clang (it appears in llvm/tools/clang/lib/Frontend/Rewrite/RewriteMacros.cpp starting from line 128): // If this is a #warning directive or #pragma mark (GNU extensions), // comment the

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy updated this revision to Diff 175502. donat.nagy added a comment. Remove braces, move if parent checking to ASTMatcher, other minor improvements Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54757/new/ https://reviews.llvm.org/D54757 Fil

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy marked 15 inline comments as done. donat.nagy added a comment. I will add tests for macro handling later. Comment at: clang-tidy/bugprone/BranchCloneCheck.cpp:71 +// Check whether this `if` is part of an `else if`: +if (const auto *IP = +dyn_cast(

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy updated this revision to Diff 175697. donat.nagy marked 5 inline comments as done. donat.nagy added a comment. Minor simplifications, add tests for macro handling Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54757/new/ https://reviews.ll

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy marked 3 inline comments as done. donat.nagy added a comment. **AstDiff:** I looked at the AstDiff library, but I think that it is overkill for my goals. The main feat of the AstDiff library is analyzing and presenting the differences between two AST subtrees, while this checker only

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy updated this revision to Diff 175701. donat.nagy added a comment. Correct a typo (ELSE instead of else) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54757/new/ https://reviews.llvm.org/D54757 Files: clang-tidy/bugprone/BranchCloneChec

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy marked an inline comment as done. donat.nagy added inline comments. Comment at: test/clang-tidy/bugprone-branch-clone.cpp:4 +void test_basic1(int in, int &out) { + if (in > 77) +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bug

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy updated this revision to Diff 176066. donat.nagy added a comment. Handle ternary operators, improve documentation Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54757/new/ https://reviews.llvm.org/D54757 Files: clang-tidy/bugprone/Branc

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy marked 8 inline comments as done. donat.nagy added a comment. At the end of the documentation I stated that the check examines the code after macro expansion. Is this note clear enough? Comment at: test/clang-tidy/bugprone-branch-clone.cpp:4 +void test_basic1(int in

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy updated this revision to Diff 176408. donat.nagy added a comment. Minor correction in documentation Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54757/new/ https://reviews.llvm.org/D54757 Files: clang-tidy/bugprone/BranchCloneCheck.cp

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy marked 2 inline comments as done. donat.nagy added a comment. I applied this check to the llvm + clang codebase, and I was surprised to see that it produced about 600 results (for comparison, the clang-tidy checks which are enabled by default produce approximately 6000 results togethe

[PATCH] D54757: [clang-tidy] new check: bugprone-branch-clone

donat.nagy added a comment. Macro-generated long switches: I cannot access the check results right now, but if I recall correctly, the check assigned these errors to certain lines in the `.inc` files. This means that (1) it is not very easy to to suppress them with `//NOLINT` comments, but (2)

[PATCH] D52423: Make ConversionChecker load StdCLibraryFunctionsChecker

donat.nagy created this revision. donat.nagy added a reviewer: dergachev.a. Herald added a subscriber: cfe-commits. ConversionChecker produces false positives when it encounters the idiomatic usage of certain well-known functions (e.g. getc() and the character classification functions like isalpha

[PATCH] D52423: [analyzer] Make ConversionChecker load StdCLibraryFunctionsChecker

donat.nagy added a comment. Yes, moving StdCLibraryFunctionsChecker to an always-loaded package is probably a better solution than adding this one particular dependency link. (Evaluating these functions may be useful for other checkers as well, although it does not seem to change the results of

[PATCH] D52722: [analyzer] Move StdCLibraryFunctions to apiModeling

donat.nagy created this revision. donat.nagy added a reviewer: NoQ. Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin, szepet, eraman, xazax.hun. Herald added a reviewer: george.karpenkov. StdCLibraryFunctionsChecker models the evaluation of several well-known functions

[PATCH] D52423: [analyzer] Make ConversionChecker load StdCLibraryFunctionsChecker

donat.nagy abandoned this revision. donat.nagy added a comment. I submitted a new patch, which moves stdCLibraryFunctions to apiModeling (https://reviews.llvm.org/D52722). Repository: rC Clang https://reviews.llvm.org/D52423 ___ cfe-commits mail

[PATCH] D52730: [analysis] ConversionChecker: handle floating point

donat.nagy created this revision. donat.nagy added reviewers: NoQ, george.karpenkov, rnkovacs, baloghadamsoftware, mikhail.ramalho. Herald added subscribers: cfe-commits, Szelethus. Extend the alpha.core.Conversion checker to handle implicit converions where a too large integer value is converted

[PATCH] D52722: [analyzer] Move StdCLibraryFunctions to apiModeling

donat.nagy added a subscriber: martong. donat.nagy added a comment. I don't have commit rights, please commit for me @NoQ or @martong Repository: rC Clang https://reviews.llvm.org/D52722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy updated this revision to Diff 167929. donat.nagy added a comment. Fix formatting in tests; add a comment. Repository: rC Clang https://reviews.llvm.org/D52730 Files: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp test/Analysis/conversion.c Index: test/Analysis/conversion.c

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

donat.nagy marked 6 inline comments as done. donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:175 + + if (RepresentsUntilExp >= sizeof(unsigned long long)*8) { return false; NoQ wrote: > Szelethus wrote: > > Sz

[PATCH] D52742: [analyzer][WIP] Add macro expansions to the plist output

donat.nagy added a comment. Does this checker handle the expansion of variadic macros introduced in C++11 (see syntax (3) and (4) here ) and the `#` and `##` preprocessor operators? Comment at: lib/StaticAnalyzer/Core/P

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

donat.nagy added inline comments. Comment at: clang/test/AST/ast-dump-overloaded-operators.cpp:27 // CHECK-NEXT: | `-ParmVarDecl {{.*}} col:19{{( imported)?}} 'E' -// CHECK-NEXT: `-FunctionDecl {{.*}} line:14:6{{( imported)?}} test 'void ()' +// CHECK-NEXT: -FunctionDecl {{.

[PATCH] D158156: [analyzer] Add C++ array delete checker

donat.nagy added a comment. Looks good, thanks for the improvements! One minor remark: your commit uses backticks (`) around the class names in the bug reports, which is commonly used to mark code fragments e.g. in commit messages; however, in the bug reports IIRC most other checkers use apostr

[PATCH] D159519: [clang][AST][ASTImporter] improve AST comparasion on VarDecl & GotoStmt

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. LGTM. I'm not very familiar with this area, but the change seems to be a very clean improvement. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D145229: [analyzer] Improve the documentation of the alpha.security.taint.TaintPropagation checker

donat.nagy added a comment. Mostly LGTM, but I'd like to ask you to ensure consistent line wrapping (to e.g. 72 or 80 characters) in the free-flowing text that was added or rewritten by this commit. (I know that this is a minor question because the linebreaks won't be visible in the final outpu

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy created this revision. Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, dkrupp, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. donat.nagy requested review of this revi

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy added a comment. I tested this commit on several open source projects (in the default mode Pedantic=false), the following table summarizes the results: | vim_v8.2.1920_baseline_vs_vim_v8.2.1920_with_bitwise_shift | New reports

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 544317. donat.nagy added a comment. (Fix incorrect filename mark in the license header.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 Files: clang/docs/analyz

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy added a comment. In D156312#4537200 , @NoQ wrote: >> (4) UBOR exhibits buggy behavior in code that involves cast expressions > > Hmm, what makes your check more resilient to our overall type-incorrectness? The code of UBOR (UndefResultChecker.

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked 21 inline comments as done. donat.nagy added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:29-30 + +using namespace clang; +using namespace ento; + steakhal wrote: > This is used quite often and hinders t

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 549091. donat.nagy marked 2 inline comments as done. donat.nagy added a comment. Uploading a new version based on the reviews. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.o

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy updated this revision to Diff 549881. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157104/new/ https://reviews.llvm.org/D157104 Files: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp clang/test/Analysis/out-of-bounds.c In

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy marked 2 inline comments as done. donat.nagy added a comment. I didn't upload the test results yet because right now there is some incompatibility between our local fork and the upstream. I hope that it'll be fixed soon, but until then I can't use our automated tools to run the analys

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked 13 inline comments as done. donat.nagy added a comment. I handled the inline comments, I'll check the example code and edit the release notes tomorrow. Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:272 + if (const auto ConcreteRight =

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 549978. donat.nagy marked 7 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 Files: clang/docs/analyzer/checkers.rst clang/include/clang/

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 550318. donat.nagy edited the summary of this revision. donat.nagy added a comment. I verified that the checker handles the examples in the documentation correctly (and added them to the test suite). However, as I was tweaking the examples in the document

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy added a comment. (The release notes entry is still missing, I'll try to add it tomorrow.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 ___ cfe-com

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy added a comment. In D156312#4589251 , @steakhal wrote: > I don't think we should do anything about it unless it's frequent enough. > Try to come up with a heuristic to be able to measure how often this happens, > if you really care. > Once you

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 550703. donat.nagy added a comment. Updated the release notes. This could be the final version of this commit if you agree that it's good enough. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 Files:

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked 2 inline comments as done. donat.nagy added inline comments. Comment at: clang/docs/analyzer/checkers.rst:81 + +Ensure the shift operands are in proper range before shifting. + donat.nagy wrote: > steakhal wrote: > > We should exactly elaborate

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 550706. donat.nagy added a comment. A few minutes ago I accidentally re-uploaded the previous version of the patch; now I'm fixing this by uploading the actually updated variant. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG25b9696b61e5: [analyzer] Upstream BitwiseShiftChecker (authored by donat.nagy). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D158156: [analyzer] Add C++ array delete checker

donat.nagy added a comment. This checker is a straightforward, clean implementation of the SEI-CERT rule EXP51-CPP, it'll be a good addition to Static Analyzer. I'm satisfied with the implementation, although there might be others who have a more refined taste in C++ coding style. The only issu

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy added a comment. The results on open-source projects are depressing, but acceptable. This checker is looking for a serious defect, so it doesn't find any true positives on stable versions of open-source projects; however it produces a steady trickle of false positives because the Cla

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy added a comment. A (somewhat delayed) ping to @steakhal because you asked me to tell you when I have the results. (If you've already seen the table that I uploaded on Friday, then you've nothing to do.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D158156: [analyzer] Add C++ array delete checker

donat.nagy added a comment. By the way, what would you think about putting the implementation of the two checkers (DeleteWithNonVirtualDtor, ArrayDelete) into two separate checker classes and placing the shared code into a common base class of these classes? I think this could be a significant

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy added a comment. Yup, there was a superfluous line break that was corrected by git clang-format; thanks for bringing it to my attention. As this is a very trivial change, I'll merge the fixed commit directly, without uploading it into Phabricator. Comment at: clang

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3e014038b373: [analyzer] Improve underflow handling in ArrayBoundV2 (authored by donat.nagy). Changed prior to commit: https://reviews.llvm.org/D1

[PATCH] D158156: [analyzer] Add C++ array delete checker

donat.nagy added a comment. In D158156#4604242 , @steakhal wrote: > One more thing to mention. Its usually illadvised to rename files or do > changes that would seriously impact git blame, unless we have a really good > reason doing so. > Aesthetics usu

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

donat.nagy added a comment. The change looks promising, I only have minor remarks. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:125 - const NoteTag *Note = - C.getNoteTag([SymbolicEnvPtrRegion, FunctionName]( - PathSensi

[PATCH] D158499: [analyzer] Compute FAM dynamic size

donat.nagy added a comment. In D158499#4608974 , @danix800 wrote: > One of the observable issue with inconsistent size type is > > void clang_analyzer_eval(int); > > typedef unsigned long long size_t; > void *malloc(unsigned long size); > void

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:301-302 + pluralSuffix(MaximalAllowedShift)); +R->addNote(LeftNote, PathDiagnosticLocation{LHS, Ctx.getSourceManager(), +

[PATCH] D155715: [clang][analyzer] Improve StdCLibraryFunctions socket send/recv functions.

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. LGTM, I don't have anything significant to add. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155715/new/ https://reviews.llvm.org/D1557

[PATCH] D153954: [clang][analyzer] Fix empty enum handling in EnumCastOutOfRange checker

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. LGTM, with a little bikeshedding ;-) Comment at: clang/test/Analysis/enum-cast-out-of-range.cpp:203 + +enum class empty_unfixed {}; + Consider using

[PATCH] D153954: [clang][analyzer] Fix empty enum handling in EnumCastOutOfRange checker

donat.nagy added inline comments. Comment at: clang/test/Analysis/enum-cast-out-of-range.cpp:203 + +enum class empty_unfixed {}; + steakhal wrote: > donat.nagy wrote: > > Consider using "specified" and "unspecified" instead of "fixed" and > > "unfixed", because

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked an inline comment as done. donat.nagy added a comment. @steakhal Thanks for the review! I answered some of your questions; and I'll handle the rest soon. Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:150 + SVB.evalBinOp(State, Co

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy created this revision. Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, dkrupp, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. donat.nagy requested review of this revi

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy added a comment. Note: this commit is split off from the larger commit https://reviews.llvm.org/D150446, which combined this simple improvement with other, more complex changes that had problematic side-effects. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D150446: [analyzer] Check ArraySubscriptExprs in ArrayBoundCheckerV2

donat.nagy abandoned this revision. donat.nagy added a comment. Herald added a subscriber: wangpc. I'm abandoning this commit because it amalgamates several unrelated changes and I think it'd be better to handle them separately: 1. First, there is a very simple, independent improvement related t

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

donat.nagy added reviewers: dkrupp, steakhal, Szelethus, gamesh411. donat.nagy added a comment. I'll try to upload results on open source projects, probably on Tuesday (I'm on vacation on Monday). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15710

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

donat.nagy added a comment. Why is this checker placed in the "unix" group (instead of e.g. "core")? As far as I understand it can check some POSIX-specific functions with a flag (that's off by default) but the core functionality checks platform-independent standard library functions. Moreover

[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

donat.nagy added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:1189 +``onwership_returns``: Functions with this annotation return dynamic memory. +The second annotation parameter is the size of the returned memory in bytes. + Szelethus wrote:

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy planned changes to this revision. donat.nagy marked 5 inline comments as done. donat.nagy added a comment. I'll soon upload a refactored version. Comment at: clang/docs/analyzer/checkers.rst:44-50 +Moreover, if the pedantic mode is activated by +``-analyzer-config co

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked 2 inline comments as done. donat.nagy added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:301-302 + pluralSuffix(MaximalAllowedShift)); +R->addNote(LeftNote, PathDiagnosticLocation{LHS, Ctx.getSou

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy updated this revision to Diff 548626. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 Files: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/l

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked 6 inline comments as done. donat.nagy added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:94-98 +void BitwiseShiftValidator::run() { + if (isValidShift()) { +Ctx.addTransition(State, createNoteTag()); + } +} ---

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

donat.nagy marked 4 inline comments as done. donat.nagy added a comment. Thanks for the suggestions, I answered those where I had something to say and I'll implement the rest of them. In D156312#4576120 , @steakhal wrote: > Have you considered checking

[PATCH] D158156: [analyzer] Add C++ array delete checker

donat.nagy added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:47 +protected: + class PtrCastVisitor : public BugReporterVisitor { public: I like that yo switched to a more descriptive class name :) Com

[PATCH] D158156: [analyzer] Add C++ array delete checker

donat.nagy added inline comments. Comment at: clang/test/Analysis/ArrayDelete.cpp:85-88 +Derived *d3 = new DoubleDerived[10]; +Base *b3 = d3; // expected-note{{Conversion from derived to base happened here}} +delete[] b3; // expected-warning{{Deleting an array of pol

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

donat.nagy added a comment. In D153776#4467627 , @balazske wrote: > The success/failure note tags are not added to all functions, `dup` is one of > these, this means at `dup` no note tag is shown. A next patch can be made to > add these messages to all

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

donat.nagy added a comment. I don't have concrete remarks, but I would like to note that I'm happy to see this cleanup :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154325/new/ https://reviews.llvm.org/D154325 _

[PATCH] D154423: [clang][analyzer] Add all success/failure messages to StdLibraryFunctionsChecker.

donat.nagy added a comment. These code changes seem to be promising. Please upload results on the open source projects (like the ones that you uploaded previously), and I hope that after verifying those I we can finally merge this set of commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

donat.nagy added a comment. The commit looks good in general, I have a few minor suggestions and a serious question about the state transition bureaucracy. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:65-66 + // If set to true, consider getenv call

[PATCH] D154509: [clang][analyzer] StdLibraryFunctionsChecker: Allow NULL buffer in `fread` and `fwrite` if size is zero.

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:413-414 +using ValueConstraint::ValueConstraint; +ArgNo SizeArg1N; +st

[PATCH] D153954: [WIP][clang][analyzer] Relax alpha.cplusplus.EnumCastOutOfRange checker

donat.nagy added a comment. I think the old "report when the value stored in an enum type is not equal to one of the enumerators" behavior would be useful as an opt-in checker that could be adopted by some projects; while the new "relaxed" version is too bland to be really useful. I'd suggest k

[PATCH] D153954: [WIP][clang][analyzer] Relax alpha.cplusplus.EnumCastOutOfRange checker

donat.nagy added a comment. In D153954#4480296 , @steakhal wrote: > [...] I'd not recommend moving the actual implementation anywhere. I agree, I mostly spoke figuratively, suggesting that this checker could end up in the optin group when it's eventuall

[PATCH] D154509: [clang][analyzer] StdLibraryFunctionsChecker: Allow NULL buffer in `fread` and `fwrite` if size is zero.

donat.nagy added a comment. By the way here is a concrete example where this patch is needed to squash a false positive: https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_stdclf_notetag_interesting_test_3&is-unique=on&report-hash=49939ae1896dff1ad7820

[PATCH] D154423: [clang][analyzer] Add all success/failure messages to StdLibraryFunctionsChecker.

donat.nagy added a comment. I looked through most of the open source results, and they look promising. However I've seen one questionable tendency: there are many reports (e.g. this one

[PATCH] D154423: [clang][analyzer] Add all success/failure messages to StdLibraryFunctionsChecker.

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. Ok, then I think you can finally merge this commit and its two predecessors. I'm happy to see that this improvement is complete. I hope that you can later add a special case for the st

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153776/new/ https://reviews.llvm.org/D153776 __

[PATCH] D154423: [clang][analyzer] Add all success/failure messages to StdLibraryFunctionsChecker.

donat.nagy added a comment. In D154423#4485009 , @balazske wrote: > It would be more simple to handle the standard streams in `StreamChecker` > only. [...] That's a reasonable plan, especially if we can bring that checker out of alpha in the foreseeabl

[PATCH] D153612: [clang][analyzer] Add and change NoteTags in StdLibraryFunctionsChecker.

donat.nagy added a comment. (Just added some side remarks about string handling annoyances.) @balazske Please (1) handle the `dyn_cast` request of @steakhal and also (2) do something with this "how to convert `StringRef` to `char *`" question that we're bikeshedding. I hope that after those we

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

donat.nagy added a comment. Thanks for creating this commit, it's a useful improvement! I added some inline comments on minor implementation details; moreover, note that "Releted checkers:" (instead of "related") is a typo in the commit message. I also have a less concrete question about the ro

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

donat.nagy created this revision. donat.nagy added reviewers: Szelethus, steakhal, gamesh411, NoQ. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. donat.nagy requested review of this re

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

donat.nagy added inline comments. Herald added a subscriber: ormris. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:35-36 public Checker { - mutable std::unique_ptr BT; + mutable std::unique_ptr BT; mutable std::unique_ptr TaintBT; -

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

donat.nagy marked an inline comment as done. donat.nagy added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp:27 class BoolAssignmentChecker : public Checker< check::Bind > { -mutable std::unique_ptr BT; +mutable std::unique_ptr

[PATCH] D158968: [analyzer] Fix assertion on casting SVal to NonLoc inside the IteratorRange checker

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. LGTM. I'm not familiar with the Iterator checker family, but this is a very straightforward change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

donat.nagy accepted this revision. donat.nagy added a comment. This revision is now accepted and ready to land. Thanks for the updates! I didn't spot any important issue, so I'm accepting this commit, but let's wait for the opinion of @steakhal before merging this. In D158707#4613955

[PATCH] D158855: [analyzer][NFC] Remove useless class BuiltinBug

This revision was automatically updated to reflect the committed changes. donat.nagy marked an inline comment as done. Closed by commit rG8a5cfdf7851d: [analyzer][NFC] Remove useless class BuiltinBug (authored by donat.nagy). Changed prior to commit: https://reviews.llvm.org/D158855?vs=553498&i

  1   2   >