[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/ https://revie

[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 https://reviews.llvm.org/D48866/ne

[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 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 https://reviews.llvm.org/D48866/ne

[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 i

[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 th

[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 f

[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] 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 ma

[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 t

[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 positive

[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] 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 [[ https://github.com/llvm/llvm-project/blob/master/cla

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

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Although I'm not the most experienced one here, I think it's good to go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/ https://re

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal added a comment. In D84316#2168462 , @NoQ wrote: > Shared accessors look amazing. > > [...] you're splitting up the part which performs boring bookkeeping for the > state trait from the part which models `strlen()` and other various functions.

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal marked 4 inline comments as done. steakhal added a comment. In D84316#2169195 , @Szelethus wrote: > [...] you really cant make CStringChecker work without CStringModeling How should I fuse the `CStringModeling` and the `CStringChecker` together

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal updated this revision to Diff 280398. steakhal added a comment. - Created the `CStringChecker` subdirectory holding the related files. - `CStringChecker` split up to 4 files: - `CStringChecker.h`: Declaration of the checker class. - `CStringChecker.cpp`: Definitions ONLY of the cstirn

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal added a comment. Thanks for checking this @balazske. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43 + ProgramStateRef &State, const Expr *Ex, + SVal Buf, bool Hypot

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43 + ProgramStateRef &State, const Expr *Ex, + SVal Buf, bool Hypothetical = false); +

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

steakhal planned changes to this revision. steakhal added a comment. Thank you all for the comments. In D84736#2179828 , @NoQ wrote: > I think the overall idea is correct. So I will keep going in this direction, you can expect the more complex version o

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

steakhal added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:19 +#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" +#include "llvm/Config/config.h" +#include "gtest/gtest.h" mgorny wrote: > `config.

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

steakhal updated this revision to Diff 281556. steakhal marked 3 inline comments as done. steakhal added a comment. Reworked `ElementRegion` handling. Previously it handled only element regions in the form of: `Element{X,n} OP Element{X,m}` Now supports `Element{X,n} OP X` and `X OP Element{X,n}`

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

steakhal added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:19 +#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" +#include "llvm/Config/config.h" +#include "gtest/gtest.h" whisperity wrote: > stea

[PATCH] D84929: [analyzer] Fix out-of-tree only clang build by not relaying on private header

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

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

steakhal added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:19 +#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" +#include "llvm/Config/config.h" +#include "gtest/gtest.h" steakhal wrote: > whispe

[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

[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: https://reviews.llvm.or

[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 f

[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 RangeConstraintManager::tryAssu

[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 `Ran

[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] D83190: [analyzer] Model iterator random incrementation symmetrically

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:267 BinaryOperatorKind OK = BO->getOpcode(); - SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext()); + Expr *LHS = BO->getLHS(); + Expr *RHS = BO->getRHS(); --

[PATCH] D83555: [analyzer] Fix ctu-on-demand-parsing: REQUIRES: linux -> REQUIRES: system-linux

This revision was automatically updated to reflect the committed changes. Closed by commit rGd96a47c61625: [analyzer] ctu-on-demand-parsing tests: replace linux -> system-linux (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Git

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

steakhal added a comment. In D83660#2148834 , @NoQ wrote: > Looks like a copy-paste error indeed. > > @OikawaKirie do you accidentally have a test case to reproduce the crash, so > that we could add it to our regression test suite? 'Cause traditionally we

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

steakhal added a comment. In D83660#2149509 , @OikawaKirie wrote: > > Might in the future I would spend some time on it - we will see. > > [...] If there is anything that our research group can do, you are free to > contact us. I would use the official

[PATCH] D78189: [analyzer] StdLibraryFunctionsChecker: Add statistics

steakhal marked an inline comment as done. steakhal added a comment. Besides @balazske's comments, looks good to me. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:504-505 -if (NewState && NewState != State) +if (NewState && NewState !=

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

steakhal added a comment. Could you add some tests demonstrating the refined diagnostics of the checker? It would help to validate the quality of the emitted diagnostics. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:88 /// obviously uint32_t s

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

steakhal added a comment. Unfortunately, I'm not qualified enough to have much to say. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:1 +//===--- Env32CCheck.cpp - clang-tidy -===// +// Env32CCheck.cpp ->

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGe22cae32c5c4: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D78704?vs=260104&id=274133#toc Repository: rG

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGde361df3f6d0: [analyzer][Z3-refutation] Fix a refutation BugReporterVisitor bug (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/n

[PATCH] D82856: [analyzer][Z3-refutation] Add statistics for refutation visitor

steakhal created this revision. steakhal added reviewers: NoQ, 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: clang. This patc

[PATCH] D82856: [analyzer][Z3-refutation] Add statistics for refutation visitor

steakhal added a comment. In D82856#2122330 , @xazax.hun wrote: > Yay! This looks good to me and I love statistics, so a huge +1. > I have one question though. Isn't this counting all the reports in an > equivalence class? I.e. if the analyzer finds som

[PATCH] D82856: [analyzer][Z3-refutation] Add statistics for refutation visitor

steakhal updated this revision to Diff 274510. steakhal added a comment. - adds statistic to undecidable branch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82856/new/ https://reviews.llvm.org/D82856 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: clang/lib/Sta

[PATCH] D82900: [analyzer][Z3-refutation] Add statistics tracking invalidated bug report classes

steakhal created this revision. steakhal added reviewers: NoQ, 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: clang. Counts ho

[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: clang/

[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. `FalsePos

[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 inform

[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 &C) { - const FunctionDecl *FDecl = C.getCalleeDecl(CE); +

[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. st

[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 clang

[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 = CF

[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 clang/unittes

[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 &Code, std::string &Diags, +

[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/ https://reviews.llv

[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 clang/lib/StaticAnalyzer/Cor

[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 File

[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 a

[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] D84979: [analyzer][NFC] Refine CStringLength modeling API

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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43 + ProgramStateRef &State, const Expr *Ex, + SVal Buf, bool Hypothetical = false); +

[PATCH] D84929: [analyzer] Fix out-of-tree only clang build by not relaying on private header

This revision was automatically updated to reflect the committed changes. Closed by commit rG63d3aeb529a7: [analyzer] Fix out-of-tree only clang build by not relaying on private header (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D84979: [analyzer][NFC] Refine CStringLength modeling API

steakhal added a comment. It seems the Pre-merge clang-format went crazy. I think just ignore the lint warnings for the unchanged lines. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84979/new/ https://reviews.llvm.org/D84979 _

[PATCH] D85026: [analyzer] Minor refactoring of SVal::getSubKind function

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I like the change. I also proved the equivalence, just for fun really :D from z3 import * def proof_equality(F, G): s = Solver() s.add(Not(F == G)) r = s.check() if r

[PATCH] D85034: [analyzer] Simplified functions SVal::getAsSymbolicExpression and similar ones

steakhal resigned from this revision. steakhal added a comment. Herald added a subscriber: steakhal. It would be great to simplify these. I have also wondered once why those different functions exist. Does anyone know what was the original intention of these functions? Since I'm not confident in

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal added a comment. In D84316#2187372 , @19n07u5 wrote: > The title is a little bit confusing because only the C-string size model is > going to be separated and be accessible. Could you elaborate on why is the title not precise? It seems that the

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

steakhal planned changes to this revision. steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1035-1038 +return evalBinOpNN(state, BO_Mul, Index, + makeArrayI

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

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

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

steakhal planned changes to this revision. steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104 +const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) { + const Type *CanonicalType = Ty

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

steakhal updated this revision to Diff 283936. steakhal added a comment. - Moved the FIXME closer to the subject. - Added tests for covering incomplete enums as well. - Added `REQUIRES: z3`. This will mark the test case `unsupported` on every buildbots. See my notes about this behavior at D83677

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

steakhal added a comment. LGTM In D85424#2199471 , @baloghadamsoftware wrote: > Unfortunately, I could not create test for it. It is extremely rare that the > //Analyzer// creates an `UndefinedVal`. You can work around the issue by creating a unit-test

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

steakhal updated this revision to Diff 284126. steakhal marked 3 inline comments as done. steakhal edited the summary of this revision. steakhal added a comment. - Using `dump`s instead of `reaching` in tests. - Not requiring complete enums anymore //(unlike we did before the patch)//. Repositor

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

steakhal added a comment. In D85528#2203325 , @NoQ wrote: > Aha, ok, sounds like the right thing to do. Like, for Z3 it's actually the > wrong thing to do (you'd prefer to evaluate the cast perfectly by adding > `SymbolCast`) but for pure RangeConstraint

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

steakhal added a comment. In D85528#2205094 , @xazax.hun wrote: > Looks reasonable to me, but I am not very familiar with the impacts of the > additional casts. Do we lose some modeling power when we are using the > regular constraint solver? > > For exa

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

steakhal added a comment. I mean, this shouldn't be an issue. Since we already omitted the 'unnecessary' cast expressions... That decision is the root cause of this, we should have expected that. IMO we do the right thing here. If we want to treat sym and sym2 to refer to the same symbol, we s

[PATCH] D81254: [analyzer] Produce symbolic values for C-array elements

steakhal added a comment. In D81254#2122449 , @ASDenysPetrov wrote: > @NoQ, thanks for the examples. > > I didn't get the first one. How do you get to the "//In reality we don't > know//", if we don't touch `a[index1]`: If `index1` and `index2` has the

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

steakhal updated this revision to Diff 286209. steakhal marked an inline comment as done. steakhal added a comment. Add an extra `RUN` line without //refutation//. The expected result is the same as with refutation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

steakhal added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84316/new/ https://reviews.llvm.org/D84316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

steakhal abandoned this revision. steakhal added a comment. I no longer think that we should support Symbol to Symbol comparisons. It would introduce certain anomalies, like //Why could the CM reason about this and that comparisons wile could not in others//. As of now, it's clear that we can no

[PATCH] D84979: [analyzer][NFC] Refine CStringLength modeling API

steakhal updated this revision to Diff 286212. steakhal marked 2 inline comments as done. steakhal edited the summary of this revision. steakhal added a comment. Herald added a subscriber: mgorny. Fixed Artem's inline comments: - `cstring::getCStringLength` now takes `StateRef` by value - `cstrin

[PATCH] D84979: [analyzer][NFC] Refine CStringLength modeling API

steakhal added a comment. In D84979#2190996 , @balazske wrote: > Really I am still not totally familiar how the checkers work and if it is > good to have these function names. Yea, naming things is pretty hard. I'm open to have a less verbose one - espe

[PATCH] D84736: [analyzer] Handle pointer difference of ElementRegion and SymbolicRegion

steakhal added inline comments. Comment at: clang/test/Analysis/pointer-arithmetic.c:88 + clang_analyzer_dump_int(p - pn); // expected-warning-re {{0 - (reg_${{[0-9]+}})}} + clang_analyzer_dump_int((p + 1) - q); // expected-warning {{Unknown}} // FIXME: Might point to

[PATCH] D84736: [analyzer] Handle pointer difference of ElementRegion and SymbolicRegion

steakhal added inline comments. Comment at: clang/test/Analysis/pointer-arithmetic.c:88 + clang_analyzer_dump_int(p - pn); // expected-warning-re {{0 - (reg_${{[0-9]+}})}} + clang_analyzer_dump_int((p + 1) - q); // expected-warning {{Unknown}} // FIXME: Might point to

[PATCH] D84736: [analyzer] Handle pointer difference of ElementRegion and SymbolicRegion

steakhal updated this revision to Diff 286318. steakhal marked 4 inline comments as done. steakhal retitled this revision from "[analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion" to "[analyzer] Handle pointer difference of ElementRegion and SymbolicRegion". steakhal ed

[PATCH] D132142: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:799 + /// we actually know their types. + QualType getApproximatedType() const { +return sym->getType()->getPointeeType(); martong wrote: > I thin

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

steakhal added a comment. Consider adding a test case demonstrating if the checker recognizes and suppresses the check if there are multiple variables referring to the memory being reallocated. If the check does not handle that case, we should probably state this limitation explicitly. And mark

[PATCH] D132142: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions

This revision was automatically updated to reflect the committed changes. steakhal marked 3 inline comments as done. Closed by commit rGf8643a9b31c4: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D132142?vs

[PATCH] D132143: [analyzer] LazyCompoundVals should be always bound as default bindings

This revision was automatically updated to reflect the committed changes. Closed by commit rGafcd862b2e0a: [analyzer] LazyCompoundVals should be always bound as default bindings (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D132109: [analyzer] Dump the environment entry kind as well

This revision was automatically updated to reflect the committed changes. Closed by commit rG7cddf9cad18a: [analyzer] Dump the environment entry kind as well (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132109/new/ https://r

[PATCH] D133851: [analyzer] Initialize ShouldEmitErrorsOnInvalidConfigValue analyzer option

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

[PATCH] D133851: [analyzer] Initialize ShouldEmitErrorsOnInvalidConfigValue analyzer option

This revision was automatically updated to reflect the committed changes. Closed by commit rGb8e1da050673: [analyzer] Initialize ShouldEmitErrorsOnInvalidConfigValue analyzer option (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D64087: [clang] Correct source locations for instantiations of out-of-line defaulted special member functions. (PR25683)

steakhal added a comment. This patch appears to introduce a bug in some source ranges. Reported the regression at https://github.com/llvm/llvm-project/issues/71161. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64087/new/ https://reviews.llvm.org/D

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

steakhal added a comment. Actually, there is one more bug there. If the string has an embedded null-terminator; a wrong length will be returned. Please add tests to demonstrate the wrong behavior. Try different target triples for having irregular char sizes and pin that. Repository: rG LLVM

[PATCH] D128535: [analyzer] Improve loads from reinterpret-cast fields

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1897 + QualType Ty = SubReg->getValueType(); + if (BaseTy->isScalarType() && Ty->isScalarType()) { +if (Ctx.getTypeSizeInChars(BaseTy) >= Ctx.getTypeSizeInChars(Ty)) {

[PATCH] D127874: [analyzer] Reimplement UnreachableCodeChecker using worklists

steakhal added a comment. In D127874#3628112 , @martong wrote: >> I don't think any of the issues mentioned in this patch relates to strongly >> connected components, thus I don't think I can answer to this question. > > In your example above, repeated h

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

steakhal added a comment. Sorry about my delayed response. I was busy. I've left a couple comments inline. Nothing serious. Thanks for the patch! Comment at: clang/docs/ReleaseNotes.rst:593 + +- clang-extdef-mapping now accepts .ast files as input. This is faster than to + rec

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

steakhal added inline comments. Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:149 + if (!CI) +CI = new CompilerInstance(); + thieta wrote: > steakhal wrote: > > What takes the ownership of `CI`? When is it deleted? > I don't think anyone

[PATCH] D122244: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

steakhal added a comment. In D122244#3404727 , @Szelethus wrote: > LGTM! You did check whether a missing doc field will actually trigger this > error, right? Yup, it works as expected! ninja: Entering directory `build/release' [1/375] Building Chec

[PATCH] D122244: [analyzer] Turn missing tablegen doc entry of a checker into fatal error

steakhal updated this revision to Diff 417862. steakhal added a comment. reword error message CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122244/new/ https://reviews.llvm.org/D122244 Files: clang/utils/TableGen/ClangSACheckersEmitter.cpp Index: clang/utils/TableGen/ClangSAChecker

  1   2   3   4   5   6   7   8   9   10   >