[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso updated this revision to Diff 215159. Charusso marked 4 inline comments as done. Charusso added a comment. - Rainbow Butterfly Unicorn Kitty CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 F

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso added a comment. I realized that it is meaningless what separator we use to list the checkers, so I have picked `;`. The `,` is limited to the `analyzer-config` list. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:192-197 + /// Pair of checker

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso updated this revision to Diff 215103. Charusso marked 4 inline comments as done. Charusso retitled this revision from "[analyzer] Analysis: "Disable" core checkers" to "[analyzer] Analysis: Silence checkers". Charusso edited the summary of this revision. Charusso set the repository for th

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. Any analyzer config flag is equally accessible to anyone as the driver flags as they are both flags. The only difference is the config flags are more code to implement, and a lot more difficult to use. @NoQ, why the hell would we pick another type of flag which makes z

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1626460 , @NoQ wrote: > I'd like to hear @Szelethus's opinion one more time on this patch. I'm > perfectly fine with abandoning the idea and going for in-checker > suppressions, simply because there's at least one stro

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1625971 , @NoQ wrote: > In D66042#1625000 , @Szelethus wrote: > > > if we add this flag, people responsible for developing interafaces for the > > analyzer might end up using it.

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1626122 , @NoQ wrote: > > use it locally > > What do you mean here? If you want to use the patch for evaluating your > results, you might as well untick the checker in the scan-build's index.html > interface. The point

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1625971 , @NoQ wrote: > In D66042#1625000 , @Szelethus wrote: > > > Your patch title, and the things things that you said make me believe that > > there are only a handful of cor

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624684 , @NoQ wrote: > My idea here was that this new feature isn't going to be user-facing. We > aren't promising to support all combinations of > enabled-disabled-silenced-dependent-registercheckerhacks, but instead

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624475 , @Szelethus wrote: > In D66042#1624469 , @Charusso wrote: > > > In a long-term rewriting the Analyzer from scratch would be ideal. There is > > no problem with this patc

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624468 , @Szelethus wrote: > In D66042#1624462 , @Charusso wrote: > > > My solution preserve the checkers as they are and yours definitely would > > rewrite them. Checker writin

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624432 , @Szelethus wrote: > In D66042#1624365 , @Charusso wrote: > > > In D66042#1624320 , @Szelethus > > wrote: > > > > > I think it w

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624320 , @Szelethus wrote: > I think it would make a lot more sense to create a separate (and hidden!) > coreModeling package that would do all the modeling, and regard core as a > highly recommended, but not mandator

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:192-197 + /// Pair of checker name and enable/disable to do analysis. + std::vector> CheckerAnalysisVector; + + /// Vector of checker names to do not emit warnings. + std::ve

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso updated this revision to Diff 214500. Charusso added a comment. - Fix a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang/include/clang/Driver/CC1Options.td clang/incl

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso updated this revision to Diff 214498. Charusso marked 5 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang/include/clang/

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added inline comments. Comment at: clang/include/clang/Driver/CC1Options.td:127-128 +def analyzer_disable_warning : Separate<["-"], "analyzer-disable-warning">, + HelpText<"Choose analyzer checkers of the warnings to disable">; +def analyzer_disable_warning_EQ : Joine

[PATCH] D65182: [analyzer] Add fix-it hint support.

Charusso accepted this revision. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:103 void enableWerror() { ShouldEmitAsError = true; } + void enableFixitsAsRemarks() { FixitsAsRemarks = true; } NoQ wrote: >

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso updated this revision to Diff 214490. Charusso added a comment. - Remove one misleading 'no-warning' comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang/include/clang/Driv

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This patch introduces a new option: `-analyzer-disable-warning`

[PATCH] D65182: [analyzer] Add fix-it hint support.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I really like that patch, as no human would rewrite 600 `dyn_cast` and `getAs` to `cast` and `castAs`. Thanks you! Comment at: clang/include/clang/StaticAnalyzer/Checker

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

This revision was automatically updated to reflect the committed changes. Closed by commit rL368383: [analyzer] CastValueChecker: Model castAs(), getAs() (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

This revision was automatically updated to reflect the committed changes. Closed by commit rL368382: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added a comment. Thanks you for the review! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; + NoQ wrote: > Charusso wrote: > > NoQ wrote: > > > Please explain "Checki

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214277. Charusso marked 6 inline comments as done. Charusso added a comment. - Better naming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; + NoQ wrote: > Please explain "Checking" and "Sugar". Checking what? Sugar

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214234. Charusso marked 7 inline comments as done. Charusso added a comment. - Added a test case for casting *to* a reference. - Better naming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214186. Charusso added a comment. - The call as `getAsCXXRecordDecl()` sometimes run into an assertion failure, so we need to avoid it: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:333: ExprEngine::createTemporaryRegionIfNeeded(): Assertion `!InitValW

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214041. Charusso added a comment. - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a class. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checke

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214038. Charusso added a comment. - Fix a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp clang/test/Analysis/cast-value.cpp Index

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51 + + const CallDescriptionMap SugarCastCDM = { + {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs}, NoQ wrote: > I'd rather have only one map from c

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214037. Charusso marked 9 inline comments as done. Charusso added a comment. - Make it usable with references. - Test references. - Better messages on simple `cast<>`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.or

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } NoQ wrote: > Charusso wrote: > > NoQ wrote

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 213952. Charusso marked 5 inline comments as done. Charusso added a comment. - Remove symbol-conjuring. - Add a forgotten test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/Static

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } NoQ wrote: > Charusso wrote: > > That is a

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added a comment. In D64374#1594617 , @Szelethus wrote: > I guess `getAs` and `castAs` methods could join the party too. I'm evaluating > plenty of results on LLVM, and those seem to be in the same problem category. In D64374#1618694

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso marked 4 inline comments as done. Charusso added a comment. This is a little-bit WIP as the symbol conjuring is very naive. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.ge

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Thanks to Kristóf Umann for the great idea! Repository: rC

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso added a comment. Thanks for the review! It is much better: F9740817: report-Driver.cpp-operator()-6-2.html Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420 + if (!IsAssuming) { +PathDiagnosticLocation Loc(

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso updated this revision to Diff 213763. Charusso marked 2 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/lib

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso added a comment. Here is an example of the new `MemberExpr::getBase()` based report: F9736772: report-Driver.cpp-operator()-6-1.html Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420 + if (!IsAssuming) { +P

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso updated this revision to Diff 213610. Charusso marked 4 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/lib

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a comment. It also reverts https://reviews.llvm.

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

Charusso added a comment. It also reverts https://reviews.llvm.org/rL362632 which is not a fix, but a problem instead. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65663/new/ https://reviews.llvm.org/D65663 ___ cfe

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

This revision was automatically updated to reflect the committed changes. Closed by commit rL367608: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its… (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso added a comment. Thanks for the review and for the idea! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65587/new/ https://reviews.llvm.org/D65587 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso added a subscriber: george.karpenkov. Charusso added inline comments. Comment at: clang/test/Analysis/loop-unrolling.cpp:349-353 #ifdef DFS -clang_analyzer_numTimesReached(); // expected-warning {{10}} +clang_analyzer_numTimesReached(); // expected-warning {{16}

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso updated this revision to Diff 212895. Charusso marked an inline comment as done. Charusso edited the summary of this revision. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65587/new/ https://reviews.llvm.org/D65587 Files: clang/include/clang

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:486 const StackFrameContext *getStackFrame(AnalysisDeclContext *Ctx, - LocationContext const *Paren

[PATCH] D65587: [analyzer] StackFrameContext: Add NodeBuilderContext::blockCount() to its profile

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, zzheng, szepet, baloghadamsoftware, xazax.hun. With this addition we can distinguish between `StackSpa

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:496 + // as a bitwise operation result could be null. + if (RS.getConcreteValue() && RS.getConcreteValue()->getExtValue() == 0) +return State; NoQ wrote: >

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso updated this revision to Diff 212440. Charusso marked 6 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65239/new/ https://reviews.llvm.org/D65239 Files: clang/include/clang/AST/Expr.h clang/include/clang/StaticAnaly

[PATCH] D65427: [analyzer] exploded-graph-rewriter: Implement Store pointers.

Charusso accepted this revision. Charusso added a comment. Oh, of course. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65427/new/ https://reviews.llvm.org/D65427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D65427: [analyzer] exploded-graph-rewriter: Implement Store pointers.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Thanks! Is it working in dark-mode as expected? Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:647 else: +self._dump(' (%s)' % st.ptr)

[PATCH] D65345: [analyzer] exploded-graph-rewriter: Implement manual graph trimming.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. It is a great idea, thanks! Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:918 +# TargetedTrimmer keeps paths that lead to specific nodes and discards all +#

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I like the HTML output, thanks! Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:846 +print(' $ dot -Tsvg input.dot -o output.svg') +

[PATCH] D65344: [analyzer] exploded-graph-rewriter: NFC: Replace explorers with trimmers.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. In my mind an explorer would trim me a graph so I like the new abstraction to differentiate the two. Thanks! Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso marked 3 inline comments as done. Charusso added a comment. In D65239#1599889 , @NoQ wrote: > Aha, great, the overall structure of the code is correct! Thanks! Now it is more formal. The only problem it does not change anything on LLVM reports,

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso updated this revision to Diff 211956. Charusso edited the summary of this revision. Charusso added a comment. - Restrict the generic contradiction-based range evaluation to only affect that left-hand side operands which constraint range are concrete zero. CHANGES SINCE LAST ACTION ht

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

Charusso added a comment. In D65250#1601187 , @NoQ wrote: > In D65250#1600776 , @grandinj wrote: > > > There is no need to wrap SVG in HTML if you want to display it in a > > web-browser > > > I guess it's worth it

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

Charusso accepted this revision. Charusso added a comment. When I started programming I read that the programming is British English, but LLVM project has more 'behavior' than 'behaviour', so as being democratic it should be fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65180/new

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. We do not support evaluating bitwise operations, so that when w

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

This revision was automatically updated to reflect the committed changes. Closed by commit rL366391: [analyzer] MallocChecker: Prevent Integer Set Library false positives (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added a comment. In D64680#1590619 , @NoQ wrote: > Great, thanks! Thanks for the review! I like that new name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 ___

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552 + FunctionStr = Lexer::getSourceText( + CharSourceRange::getTokenRange( + {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}), + C.getSourc

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 210458. Charusso marked 4 inline comments as done. Charusso added a comment. - More fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552 + FunctionStr = Lexer::getSourceText( + CharSourceRange::getTokenRange( + {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}), + C.getSourc

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 210347. Charusso marked 9 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso marked 4 inline comments as done. Charusso added a comment. In D64680#1584315 , @NoQ wrote: > P.S. I think you should attach the report to Phabricator directly, as the > link will expire as soon as these reports get regenerated. Luckily the sta

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 209849. Charusso added a comment. - Remove unnecessary `DoNothing` kind. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/re

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added a comment. Here is an example of the mentioned use-after-free by pointer-escaping as an argument: https://llvm.org/reports/scan-build/report-DeclBase.cpp-getFromVoidPointer-0-1.html#EndPath CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso updated this revision to Diff 209654. Charusso marked 2 inline comments as done. Charusso added a comment. - Fix. - Move the logic to `free()` for better matching. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64680/new/ https://reviews.llvm.org/D64680 Files: clang/lib/Stati

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso added a comment. In D64680#1584076 , @NoQ wrote: > Change of plans: let's suppress the warning when our `free()` is done within > the function that has `__isl_take` in its definition. So, like, ascend the > chain of location contexts and check y

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Integer Set Library using retain-count based allocation which i

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Charusso accepted this revision. Charusso added a comment. The `impure-warning` sounds like some alpha, not-well-defined warning, other than that I like the movement to rethink existing checkers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64274/new/ https://reviews.llvm.org/D64274

[PATCH] D64611: [analyzer] exploded-graph-rewriter: Improve source location dumps.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Much better! Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64611/new/ https://reviews.llvm.org/D64611 __

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208847. Charusso added a comment. - Add the llvm namespace to the test file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

This revision was automatically updated to reflect the committed changes. Closed by commit rL365582: [analyzer] CastValueChecker: Model casts (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added a comment. Thanks for the review! In D64374#1577279 , @NoQ wrote: > That doesn't sound like it's high on our list. That's not too many times. > Probably very few false positives come out of it. Also modeling smart > pointers is much harde

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added a comment. In D64374#1577266 , @NoQ wrote: > Can you provide more info, eg. the full backtrace? Well, `unique_dyn_cast<>` and `unique_dyn_cast_or_null<>` is used like 20 times in the LLVM codebase, whoops. We want to model it. Full info:

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208831. Charusso added a comment. - Add the forgotten `llvm` namespace to the `CallDescription`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added a comment. In D64374#1577235 , @NoQ wrote: > Whoops. I underestimated you (: > > Ok, anyway, this was my last comment. Great job! Remember to make sure that > it works on the actual LLVM, not only on tests; mocked-up test headers are > ver

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:172 + !CE->getArg(0)->getType()->getPointeeCXXRecordDecl()) +return false; + If we cannot obtain any of the CXXR

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208824. Charusso marked 6 inline comments as done. Charusso added a comment. - Move to `apiModeling.llvm`. - Prevent unknown casts. - Refactor. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208588. Charusso added a comment. - Simplify the new `getNoteTag()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/St

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:61 + + return Ty.getAsString(); +} NoQ wrote: > Use after free! `QualType::getAsString()` returns a temporary `std::string`. > You're returning a `StringRef` that

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208586. Charusso marked 9 inline comments as done. Charusso added a comment. - Fix. - More tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang/include/clang/StaticAnalyzer/Checkers/Checker

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208574. Charusso added a comment. - Fix a typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/StaticAnalyzer/Core/Pat

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso added a comment. Thanks! My mind was really set to actually model these with `classof()`, whoops. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:25-27 + using CastCheck = + std::function; NoQ wrote: > Kinda nice, but a simple

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso updated this revision to Diff 208573. Charusso marked 12 inline comments as done. Charusso added a comment. - Fix - New `getNoteTag()` which accepts a plain note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64374/new/ https://reviews.llvm.org/D64374 Files: clang/include/cl

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:175 + else*/ +Param = CE->getArg(0); + This CXXMemberCallExpr business is necessary? I am not sure as I have not see

[PATCH] D64374: [analyzer] CastValueChecker: Model casts

Charusso created this revision. Charusso added reviewers: NoQ, ravikandhadai. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. It models the LLVM casts: - `cast<>` - `

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

Charusso accepted this revision. Charusso added a comment. Document!!4!44! It is great you have started to limit the notes, thanks! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110 + ConditionTracking +}; + What about

[PATCH] D64264: [analyzer] exploded-graph-rewriter: Implement a topology-only mode.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. May you would remove the "Program points:" part as we only have that, but I think we have enough space for that tiny note. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION http

[PATCH] D64263: [analyzer] exploded-graph-rewriter: Implement a single-path mode.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Smart move! The left-most usually the shortest path, but what if not? I would calculate the length, other than that, cool. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION http

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

This revision was automatically updated to reflect the committed changes. Closed by commit rL365103: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of… (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed p

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

Charusso updated this revision to Diff 207939. Charusso marked 9 inline comments as done. Charusso added a comment. - Done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63915/new/ https://reviews.llvm.org/D63915 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

Charusso added a comment. Thanks for the reviews! In D63915#1569410 , @Szelethus wrote: > This checker isn't in alpha -- did you evaluate it on LLVM? Other than that, > looks great! Yes, it is made for LLVM and tested out 4 times. Thanks! CHANGES SIN

[PATCH] D64153: [analyzer] exploded-graph-rewriter: Add a grayscale mode.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Well, it is always awesome to think about the others. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64153/new/ https://reviews.llvm.org/D64153 _

<    1   2   3   4   5   6   7   >