[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal added a comment. Herald added a subscriber: jdoerfert. Herald added a project: clang. @hgabii Are you planning on finishing this? If not, I'd happily commandeer if not. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48866/new/

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal updated this revision to Diff 202913. steakhal marked 4 inline comments as done. steakhal added a comment. - Removed different signess related parts. - Don't warn for the casts which are already covered by '-Wcast-align' check. - Improved the diagnostic messages: - Now adds notes for

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal added a comment. In D48866#1527540 , @lebedev.ri wrote: > In D48866#1527506 , @steakhal wrote: > > > The problem with the `-Wcast-align` is that it will only fire for C-style > > bitcast expressions, not

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal marked 8 inline comments as done. steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp:69-70 + "cast from %0 to %1 may lead to access memory based on invalid " + "memory layout; pointed to type

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal updated this revision to Diff 202717. steakhal added a comment. Unfortunately the changes that I've made are not available in a diff because I've moved to the monorepo version. Although, you can see the changes in detail on my llvm-project github fork

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal commandeered this revision. steakhal added a reviewer: hgabii. steakhal added a comment. If you don't mind I will finish the leftover work. This will still be committed under your name. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal added a comment. The problem with the `-Wcast-align` is that it will only fire for C-style bitcast expressions, not for `reinterpret_cast` ones. example Does anyone know why is that behavior? CHANGES SINCE LAST ACTION

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal added a comment. What do you think, what should I improve in this checker? Your remarks, @lebedev.ri, were really valuable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48866/new/ https://reviews.llvm.org/D48866 ___ cfe-commits

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal added a comment. @Szelethus Your catch with the mispositioned report message

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal added a comment. @Szelethus The mispositioned report message was my fault. I used a different version of clang for the analysis and to upload the results, which resulted in some mispositioned reports. I've fixed the linked CodeChecker instance. CHANGES SINCE LAST ACTION

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal updated this revision to Diff 217665. steakhal added a comment. Changes: - Flag option marked as 'enabled by default'. - Reformat all the test cases for C, C++ and Obj C. - Now uses `-verify=tags` approach. - Fixes checker documentation. CHANGES SINCE LAST ACTION

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal marked 2 inline comments as done. steakhal added a comment. Fixes for @NoQ's comments. I will update the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66733/new/ https://reviews.llvm.org/D66733 ___ cfe-commits mailing list

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal created this revision. steakhal added reviewers: NoQ, krememek, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, Charusso, donat.nagy, dexonsmith, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a project: clang. Enables the users

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal marked 3 inline comments as done. steakhal added a comment. Thank you for your response @Szelethus. Fixed, updating the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66733/new/ https://reviews.llvm.org/D66733

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal updated this revision to Diff 217162. steakhal added a comment. Reformatted using `clang-format-diff.py`. Minor fixes which were requested by @Szelethus. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66733/new/ https://reviews.llvm.org/D66733 Files:

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal updated this revision to Diff 217164. steakhal added a comment. Fix copy-paste mistake. This time upload the correct version. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66733/new/ https://reviews.llvm.org/D66733 Files: clang/docs/analyzer/checkers.rst

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal added a comment. @NoQ What do you think, should it be under a flag (as it would be now), or enabled by default? I think these warnings are valuable and we should consider it enabling by default. An interesting fact is that previously rGf224820b45c6847b91071da8d7ade59f373b96f3

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

steakhal added inline comments. Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:191 static TaintPropagationRule -getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name, +getTaintPropagationRule(const GenericTaintChecker *Checker, +

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

steakhal added inline comments. Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:118 /// Check for CWE-134: Uncontrolled Format String. - static const char MsgUncontrolledFormatString[]; + static constexpr llvm::StringLiteral MsgUncontrolledFormatString = +

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

steakhal added a comment. Herald added a subscriber: whisperity. Do you have some time @Szelethus to check this change? Your experience and comments would help a lot to finish this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48866/new/ https://reviews.llvm.org/D48866

[PATCH] D69318: [analyzer] Add SufficientSizeArrayIndexingChecker

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SufficientSizeArrayIndexingChecker.cpp:75 + // Should not warn on literal index expressions. + if (dyn_cast(Index->IgnoreParenCasts())) +return; Shoudn't we use `isa<>` if we

[PATCH] D70596: [analyzer][docs] NFC: Extend documentation for MallocOverflow checker

steakhal created this revision. steakhal added reviewers: Szelethus, NoQ, krememek. Herald added subscribers: cfe-commits, Charusso, donat.nagy, dexonsmith, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a project: clang. Adds and example for

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:34-48 + Preprocessor::macro_iterator It = PP->macro_begin(); + while (It != PP->macro_end() && !SigtermValue.hasValue()) { +if (It->first->getName() == "SIGTERM")

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

steakhal added a comment. Thank you guys the responses. I cannot agree more. The sole reason why this option exists is that if you scroll back in the git blame of that line, you would find a commit, which removed this warning for this exact scenario. Possibly due to some seemingly false

[PATCH] D59516: [analyzer] Add custom filter functions for GenericTaintChecker

steakhal added a comment. I think this patch is ok. Although there are remarks: - I think the current implementation of the taint filtering functions does not follow the expected semantics. Now the modelling would remove taint before calling the function (//pre statement//). One might expect

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:28 + +static Preprocessor *PP; + Can't we place this `PP` variable to the checker class? Like the [[

[PATCH] D70876: [clang-tidy] Add spuriously-wake-up-functions check

steakhal added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:300-301 + `cert-con36-c `_, `bugprone-spuriously-wake-up-functions `_, "Yes" + `cert-con54-cpp `_, `bugprone-spuriously-wake-up-functions `_, "Yes" `cert-dcl03-c `_,

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:958 + + unsigned getNumArgs() const override { return getDecl()->getNumParams(); } + NoQ wrote: > Charusso wrote: > > `return

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:33-50 +void SignalInMultithreadedProgramCheck::registerMatchers(MatchFinder *Finder) { + auto signalCall = + callExpr( +

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG95a94df5a9c3: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

steakhal added a comment. In D71524#1902654 , @steakhal wrote: > In D71524#1889566 , @boga95 wrote: > > > @steakhal's revision is on the top of this. Changing the order will only > > cause unnecessary work on both

[PATCH] D75063: [analyzer] StdLibraryFunctionsChecker: Add NotNull Arg Constraint

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:684 +.Case({ +ReturnValueCondition(LessThanOrEq, ArgNo(2)), +}) Two lines below you are using the `{0U}` initialization

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of te affected container

steakhal added a comment. I don't have any technical comments on this patch since I haven't used `NoteTags` yet, only a couple of readability ones. Comment at: clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp:97-103 + auto *PSBR = dyn_cast(); +

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

This revision was automatically updated to reflect the committed changes. steakhal marked an inline comment as done. Closed by commit rG859bcf4e3bb9: [analyzer][taint] Add isTainted debug expression inspection check (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

steakhal added a comment. Herald added a subscriber: danielkiss. This patch introduced a crash while I was analyzing the libpressio . I was using the `CodeChecker` to drive the analysis with the `--enable-all` flag. The exact command was the following:

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

steakhal marked 2 inline comments as done. steakhal added a comment. Marking comments done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74131/new/ https://reviews.llvm.org/D74131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

steakhal marked 5 inline comments as done. steakhal added a comment. In D72035#1889320 , @Szelethus wrote: > Wow. Its a joy to see you do C++. LGTM. Are you planning to introduce > `CallDescriptionMap` at one point? :) Yes, definitely.

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

steakhal updated this revision to Diff 246420. steakhal added a comment. Declaring the purpose of this checker: > [...] > This would help to reduce the *noise* that the `TaintTest` debug checker > would > introduce and let you focus on the `expected-warning`s that you really care >

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

steakhal added a comment. In D71524#1889566 , @boga95 wrote: > @steakhal's revision is on the top of this. Changing the order will only > cause unnecessary work on both sides. I would happily rebase this patch if you want. Comment

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

steakhal created this revision. steakhal added reviewers: NoQ, Szelethus. steakhal added a project: clang. Herald added subscribers: cfe-commits, Charusso, donat.nagy, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, whisperity. This patch introduces the

[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

steakhal added a comment. I genuinely think that in the following case we should warn, since the user already had a chance to express the range assumption using an `assert`. I think that regardless which checker in what condition checks for a given constraint. If the expression is tainted, we

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

steakhal updated this revision to Diff 243146. steakhal added a comment. Clarified example usage, especially in contrast of eg.: `debug.TaintTest` checker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74131/new/ https://reviews.llvm.org/D74131

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

steakhal updated this revision to Diff 243144. steakhal added a comment. - Tests added. - Clang-format-diff applied. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74131/new/ https://reviews.llvm.org/D74131 Files:

[PATCH] D73536: [analyser][taint] Remove taint from symbolic expressions if used in comparisons

steakhal created this revision. steakhal added reviewers: NoQ, Szelethus. steakhal added a project: clang. Herald added subscribers: cfe-commits, JDevlieghere. steakhal added a subscriber: boga95. **Remove taint from symbolic expressions if used in comparison expressions.** **Problem statement

[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

steakhal added a subscriber: martong. steakhal added a comment. I'm convinced that we shouldn't remove taint from expressions used in comparisons. With the current configuration files, `sink` functions are not too useful. For now, I would delay developing a mechanism describing constraints

[PATCH] D74806: [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

steakhal updated this revision to Diff 245300. steakhal added a comment. Upload the right diff. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74806/new/ https://reviews.llvm.org/D74806 Files: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

[PATCH] D74806: [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

steakhal created this revision. steakhal added reviewers: NoQ, baloghadamsoftware, Szelethus. steakhal added a project: clang. Herald added subscribers: cfe-commits, martong, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a

[PATCH] D74806: [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

steakhal marked an inline comment as done. steakhal added a comment. In D74806#1882300 , @NoQ wrote: > This is fantastic, i love it! > > > all tests are preserved and passing; only *the message changed at some > > places*. In my opinion, these messages

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:407 +// FIXME Add detailed diagnostic. +std::string Msg = "Function argument constraint is not satisfied"; +auto R = std::make_unique(BT, Msg, N);

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

steakhal updated this revision to Diff 243492. steakhal added a comment. Rebased on top of master, instead of D71524 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72035/new/ https://reviews.llvm.org/D72035

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

steakhal added a comment. Herald added a subscriber: martong. If this patch is good to go, could someone commit it? I don't have commit access (yet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74131/new/ https://reviews.llvm.org/D74131

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:385 +unsigned getNumArgs(const CallEvent ) { + return Call.getNumArgs() + static_cast(isa(Call)); } I'm not sure why should we adjust (//workaround//) the

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

steakhal created this revision. steakhal added reviewers: NoQ, Szelethus, boga95. steakhal added a project: clang. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, whisperity. steakhal added a parent revision:

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

steakhal updated this revision to Diff 256420. steakhal marked 4 inline comments as done. steakhal added a comment. - add full diff context - NFC refactored `RangeSet` comparison function - add tests for compund `RangeSet`s like: `{[0,1],[5,6]} < {[9,9],[11,42],[44,44]}` - NFC clang-format test

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

steakhal added a comment. In D77792#1971921 , @Szelethus wrote: > You seem to have forgotten `-U` :^) Nice catch! Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:774 +Optional

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

steakhal updated this revision to Diff 256551. steakhal added a comment. - rewritten the `RangeSet::compare` function and checked the assumptions on WolframAlpha - moved the `RangeSet::compare` function to a cpp file - added comments to the `RangeSet::compare` function - fixed the comment in

[PATCH] D77802: [analyzer] Improved RangeSet::Negate support of unsigned ranges

steakhal added a comment. It took me some time to understand what you were doing. Probably a bit ASCII-art would have had help (eg. about the wrapping behavior). By the same token, I wouldn't mind more tests, especially exercising the implemented behavior. Probably `unit-tests` would fit better

[PATCH] D77802: [analyzer] Improved RangeSet::Negate support of unsigned ranges

steakhal added a comment. I'm impressed. Though, I had some nits, please don't take it to heart :) And consider joining the to the pre-merge beta testing project to benefit from buildbots and much more - for free. Comment at:

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

steakhal added a comment. In D78457#1991288 , @xazax.hun wrote: > > As turned out we don't even need a BugReporterVisitor for doing the > > crosscheck. > > We should simply use the constraints of the ErrorNode since that has all > > the necessary

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

steakhal created this revision. steakhal added reviewers: mikhail.ramalho, NoQ, Szelethus, baloghadamsoftware, xazax.hun. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, a.sidorin, rnkovacs, szepet, whisperity. Herald added a project: clang.

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

steakhal marked 2 inline comments as done. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113 const CheckerContext ) { - const FunctionDecl *FDecl = C.getCalleeDecl(CE); +

[PATCH] D77792: [analyzer] Extend constraint manager to be able to compare simple SymSymExprs

steakhal created this revision. steakhal added reviewers: NoQ, baloghadamsoftware, Charusso, Szelethus. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a project: clang. This

[PATCH] D74806: [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

This revision was automatically updated to reflect the committed changes. Closed by commit rG30e5c7e82fa1: [analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API (authored by steakhal). Herald added a subscriber: ASDenysPetrov. Changed prior to commit:

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

steakhal added a comment. In D71524#1917251 , @Szelethus wrote: > Are we sure this is what we want? If this is a heuristic, we should document > it well, and even then I'm not sure whether we want it. I'm also pretty sure > this would make the eventual

[PATCH] D78457: [analyzer][Z3-refutation] Fix a refutation BugReporterVisitor bug

steakhal added a comment. I'm planning to measure how impactful this patch is using the CodeChecker. Stay tuned if you are interested! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/ https://reviews.llvm.org/D78457 ___ cfe-commits

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

steakhal updated this revision to Diff 260104. steakhal edited the summary of this revision. steakhal added a comment. - added `GTEST_SKIP` FIXME comment - reformatted the summary of the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78704/new/ https://reviews.llvm.org/D78704

[PATCH] D78457: [analyzer][Z3-refutation] Fix a refutation BugReporterVisitor bug

steakhal updated this revision to Diff 260108. steakhal retitled this revision from "[analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor" to "[analyzer][Z3-refutation] Fix a refutation BugReporterVisitor bug". steakhal edited the summary of this revision. steakhal added

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

steakhal created this revision. steakhal added reviewers: NoQ, xazax.hun, rnkovacs, Szelethus, baloghadamsoftware, mikhail.ramalho. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, a.sidorin, szepet, whisperity, mgorny. Herald added a project: clang.

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

steakhal updated this revision to Diff 259531. steakhal added a comment. Upload the right diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78704/new/ https://reviews.llvm.org/D78704 Files: clang/unittests/StaticAnalyzer/CMakeLists.txt

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

steakhal updated this revision to Diff 259532. steakhal added a comment. - keep the visitor - fix the bug - add test demonstrating the bug and the fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/ https://reviews.llvm.org/D78457 Files:

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2871 + // Overwrite the associated constraint of the Symbol. + Constraints = CF.remove(Constraints, Sym); Constraints =

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

steakhal marked 7 inline comments as done. steakhal added a comment. I'm really embarrassed that I uploaded the wrong diff. I appologize. Most of the comments are done :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

steakhal updated this revision to Diff 259674. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/ https://reviews.llvm.org/D78457 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

steakhal updated this revision to Diff 259668. steakhal marked 6 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78704/new/ https://reviews.llvm.org/D78704 Files: clang/unittests/StaticAnalyzer/CMakeLists.txt

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

steakhal marked 4 inline comments as done. steakhal added a comment. I addressed most of the comments. Comment at: clang/unittests/StaticAnalyzer/CheckerRegistration.h:81 +template +bool runCheckerOnCodeWithArgs(const std::string , std::string , +

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

steakhal added a comment. In D86874#inline-803844 , @martong wrote: > I really feel that this check would have a better place in the implementation > of `eval`. This seems really counter-intuitive to do this stuff at the > Checker's level. Is

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal created this revision. steakhal added reviewers: NoQ, Szelethus, vsavchenko, xazax.hun. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a project:

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

steakhal added inline comments. Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34 + +void symbolic_uint_and_int0(unsigned len) { + (void)a[len + 1]; // no-warning martong wrote: > Hmm, this seems to be quite redundant with the `size_t` tests.

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:52 + /// Offset: SymIntExpr{conj{n, int}, +, 12, long long} + class RawOffsetCalculator final + : public MemRegionVisitor { martong wrote: > Since you

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

steakhal added a comment. I tried to run the benchmark on the small set of projects but it would take an enormous time to analyze all such projects 20 times each... Dedicating a box for this is unfeasible for me. So I stuck with analyzing only `tmux` with 20 iterations. The results are not

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal marked 2 inline comments as done. steakhal added inline comments. Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:1 +// RUN: %clang_analyze_cc1 --std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s + Szelethus wrote: > Isn't it

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

steakhal added a comment. In D85528#2232074 , @xazax.hun wrote: > I'm not opposed to landing this to master, as we will have more time there to > see whether there are any unwanted side effects in practice. I made some experiments on the following

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

steakhal added inline comments. Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34 + +void symbolic_uint_and_int0(unsigned len) { + (void)a[len + 1]; // no-warning martong wrote: > steakhal wrote: > > martong wrote: > > > Hmm, this seems to be

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, xazax.hun, martong, Szelethus. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a project:

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

steakhal resigned from this revision. steakhal added a comment. Herald added a subscriber: steakhal. I don't have much to say, but to keep up the good work xD I will follow this and the rest of your patches @Szelethus. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, xazax.hun, Szelethus, martong. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a project:

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, Szelethus, martong, balazske, baloghadamsoftware. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, donat.nagy, arphaman, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal updated this revision to Diff 289432. steakhal marked 2 inline comments as done. steakhal added a comment. We only analyze instantiated functions, which are not //dependently typed//. Safe to assume that every encountered `PredefinedExpression` has a defined (non-null) function name.

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal added inline comments. Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21 + clang_analyzer_dump(__func__); + clang_analyzer_dump(__FUNCTION__); + clang_analyzer_dump(__PRETTY_FUNCTION__); + // expected-warning@-3 {{{"func",0 S64b,char}}} + //

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal updated this revision to Diff 289395. steakhal added subscribers: riccibruno, rjmccall. steakhal added a comment. - Added tests for Microsoft extensions. - Added an `assert` requiring the `PredefinedExpression` to have a function name. I don't know how could a

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal updated this revision to Diff 289446. steakhal added a comment. - Added `no-warning`. - Added test-case for `__builtin_unique_stable_name` as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87004/new/ https://reviews.llvm.org/D87004

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

steakhal marked an inline comment as done. steakhal added a comment. Thank you all for the comments! Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:101 +// Such functions have no function name of predefined expressions such as: '__func__' etc. +

[PATCH] D87239: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D87239#2259428 , @martong wrote: > So, the problem is rather that the constraint check should be done in > checkPreCall, but that should be in

[PATCH] D86445: [analyzer][RFC] Simplify MetadataSymbol representation and resolve a CStringChecker FIXME

steakhal added a comment. In D86445#2260744 , @martong wrote: >> But if the string is invalidated (or the new length is completely unknown), >> **do not remove** the length from the state; instead, set it to >> SymbolConjured. > > Where exactly do you

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2264963 , @OikawaKirie wrote: > After reviewing the code of this snippet, I think it would be very difficult > to make a regression test case for the crash, as far as what I know about Z3 > and SMT solvers. > > First

[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

steakhal added a comment. OK, after a few hours of debugging, the test code simplifies to this: // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true %s -verify void foo(int

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

steakhal added a comment. In D86874#2255990 , @martong wrote: > Hi Balázs, > > Since reviews.llvm.org is offline, I am sending my comments below, inline. > Thanks for your huge effort in explaining all this! > > Overall, I have a feeling that this

[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

steakhal added a comment. In D85424#2247598 , @gamesh411 wrote: > `CReduce` did not manage to produce any meaningful result after a week worth > of runtime (more than ~2000 lines of code still remaining after reduction). > We could track this down by

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

steakhal added a comment. In D86874#2259105 , @Szelethus wrote: > I can only imagine how long it took for you to write all this, because > reading it wasn't that short either. Thank you so much! It really gave me a > perspective on how you see this

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

steakhal added a comment. In D86874#2256249 , @martong wrote: >> Calculation of the RegionRawOffsetV2 >> >> >> Let's see how does these subscript expressions used during the calculation >> of the `RegionRawOffsetV2`:

  1   2   3   4   5   6   7   8   9   10   >