[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-11-22 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 477233. ASDenysPetrov added a comment. Completely reworked solution. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-07-15 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Thanks Denys for your continued work on this. These are very good questions that must be answered, we need exactly such thinking to implement this properly. I believe we can advance gradually. In D103096#3642681 ,

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-07-11 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov marked 10 inline comments as done. ASDenysPetrov added a comment. @martong Thank you for your patience. I started moving to a bit another direction that we can improving it iteratively. Just spoiling, in my latest solution a single symbol will be associated to many classes. Here

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-06-30 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D103096#3613059 , @ASDenysPetrov wrote: > @martong Just FYI. I've been working on reworking this solution to using > `EquivalenceClasses` for several weeks. It turned out that this is an > extremely hard task to acomplish.

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-06-27 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov marked 4 inline comments as done. ASDenysPetrov added a comment. @martong Just FYI. I've been working on reworking this solution to using `EquivalenceClasses` for several weeks. It turned out that this is an extremely hard task to acomplish. There a lot of cast cases like:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-05-26 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Denys, I've created a very simple patch that makes the SValBuilder to be able to look up and use a constraint for an operand of a SymbolCast. That change passes 2 of your test cases, thus I made that a parent patch. Comment at:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-05-13 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. In D103096#3502955 , @martong wrote: > Ping Thank you, folk, for taking you time. I'll surely make corresponding changes according ещ your suggestions and notify you then. Sorry, @martong, for the late response. I'm

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-05-10 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-29 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1289-1291 + auto It = llvm::find_if(*CM, [MinBitWidth](CastMap::value_type ) { +return Item.first >= MinBitWidth; + }); There might be a problem

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-29 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Giving it some more thought, the `SymCastMap = Map` should be keyed as well with an equivalence class : `SymCastMap = Map`. This is the only way to use the equivalence info correctly when we process the casts. Comment at:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:889-890 +REGISTER_MAP_FACTORY_WITH_PROGRAMSTATE(CastMap, uint32_t /*bitwidth*/, RangeSet) +REGISTER_MAP_WITH_PROGRAMSTATE(SymCastMap, SymbolRef, CastMap) + These

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Thanks Denys for the update! This is getting really good. I have some concerns though about the `CastMap = Map`. I think we should have `CastMap = Map` instead, and we could get the `RangeSet` from the existing `ConstraintRange` mapping. By storing directly the

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-26 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 425248. ASDenysPetrov marked 4 inline comments as done. ASDenysPetrov edited the summary of this revision. ASDenysPetrov added a comment. @martong thank you for the idea. I've tried to implement it. Could you look at the patch once again, please?

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-25 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2280-2290 +/// Return a symbol which is the best canidate to save it in the constraint +/// map. We should correct symbol because in case of truncation cast we can +/// only

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-22 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov marked 10 inline comments as done. ASDenysPetrov added a comment. Thank you for the review @martong! Your work is not less harder then mine. I'll rework and update the revision ASAP. Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:421-426

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-04-19 Thread Gabor Marton via Phabricator via cfe-commits
martong added a subscriber: vabridgers. martong added a comment. First of all, thanks Denys for working on this, nice work! Here are my concerns and remarks. I think this fixed set of types in `NominalTypeList` is too rigid. I don't like the fact that we have to iterate over all four types

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-03-23 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. Herald added a project: All. Ping. If there is somebody interested in this? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 ___ cfe-commits mailing list

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

2021-11-19 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 388552. ASDenysPetrov added a comment. Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-10-11 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-16 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1908-1912 +// Handle SymbolCast before actual assignment. +std::tie(Sym, NewConstraint) = +modifySymbolAndConstraints(Sym, NewConstraint); +if (!State) +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-16 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 359305. ASDenysPetrov added a comment. Adapted solution to ConstraintAssignor API. Added tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1908-1912 +// Handle SymbolCast before actual assignment. +std::tie(Sym, NewConstraint) = +modifySymbolAndConstraints(Sym, NewConstraint); +if (!State) +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1908-1912 +// Handle SymbolCast before actual assignment. +std::tie(Sym, NewConstraint) = +modifySymbolAndConstraints(Sym, NewConstraint); +if (!State) +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1908-1912 +// Handle SymbolCast before actual assignment. +std::tie(Sym, NewConstraint) = +modifySymbolAndConstraints(Sym, NewConstraint); +if (!State) +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 359007. ASDenysPetrov edited the summary of this revision. ASDenysPetrov added a comment. Improved `ignoreCasts` implementation. Adapted to `ConstraintAssignor`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @vsavchenko > It is already a pattern in other type hierarchies. I just rarely met them. And it was hard to me reading the code searching for implementation all over the places. Comment at:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2877818 , @ASDenysPetrov wrote: > Made `ignoreCast` non-virtual. > P.S. IMO, this change is not something that can be taken as a pattern, > though. It is already a pattern in other type hierarchies. Virtual

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-14 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; vsavchenko wrote: >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-14 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 358685. ASDenysPetrov added a comment. Made`ignoreCast` non-virtual . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-14 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 358669. ASDenysPetrov added a comment. Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; ASDenysPetrov wrote: > vsavchenko

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-14 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; vsavchenko wrote: >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. > // 1. `VisitSymbolCast`. > // Get a range for main `reg_$0` - [-2147483648, 2147483647] > // Cast main range to `short` - [-2147483648, 2147483647] -> [-32768, > 32767]. > // Now we get a valid range for further bifurcation - [-32768, 32767]. That's a great

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 +if (!Opts.ShouldSupportSymbolicIntegerCasts) + return VisitSymExpr(Sym); + vsavchenko wrote: > ASDenysPetrov wrote: > > vsavchenko wrote: > >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. OK, thanks for putting a summary. I now got a good idea why you need both. At the same time, take a look at D105692 . I'm about to land it and I think it's going to be useful for you. CHANGES SINCE LAST ACTION

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 +if (!Opts.ShouldSupportSymbolicIntegerCasts) + return VisitSymExpr(Sym); + ASDenysPetrov wrote: > vsavchenko wrote: > > ASDenysPetrov wrote: > >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; vsavchenko wrote: >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; ASDenysPetrov wrote: > vsavchenko

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. I'll allocate some time to get into your summary, but for now here are my concerns about `SymbolRangeInferrer` and `VisitSymbolCast`. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 +if

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +return Sym; vsavchenko wrote: > Do you

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-13 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 +if (!Opts.ShouldSupportSymbolicIntegerCasts) + return VisitSymExpr(Sym); + vsavchenko wrote: > Why do you use `VisitSymExpr` here? You want

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-12 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @vsavchenko I've updated the summary. I hope, I addressed your question. Currently, this patch has some issues and may not fully reflect the summary. I'll update it soon(not without your suggestions). Thanks. CHANGES SINCE LAST ACTION

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2867441 , @ASDenysPetrov wrote: > @vsavchenko > >> Why did you write it this way!? > > I want the map contains only valid constraints at any time, so we can easely > get them without traversing with all variants

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1241 + // Find the first constraint and exit the loop. + RSPtr = getConstraint(State, S); +} vsavchenko wrote: > Why do you get associated

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @vsavchenko > Why did you write it this way!? I want the map contains only valid constraints at any time, so we can easely get them without traversing with all variants intersecting with each other. I'm gonna move `updateExistingConstraints ` logic to

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2867136 , @ASDenysPetrov wrote: > @vsavchenko > >> I still want to hear a good explanation why is it done this way. Here `c` >> is mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, >> but

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2867104 , @ASDenysPetrov wrote: > Generally, with this patch we kinda have several constraints for each cast of > a single symbol. And we shall care for all of that constraints and timely > update them (if

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @vsavchenko > I still want to hear a good explanation why is it done this way. Here `c` is > mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, but > we also have `(short)x` associated with `[8, 8]`. Why can't > `VisitSymbolCast` look up

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. Generally, with this patch we kinda have several constraints for each cast of a single symbol. And we shall care for all of that constraints and timely update them (if possible). For instance, we have `int x` and met casts of this symbol in code: int x;

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2867021 , @ASDenysPetrov wrote: > In D103096#2866730 , @vsavchenko > wrote: > >> In D103096#2866704 , >> @ASDenysPetrov wrote:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. In D103096#2866730 , @vsavchenko wrote: > In D103096#2866704 , @ASDenysPetrov > wrote: > >> @vsavchenko > > That's not the question I'm asking. Why do you need to set constraints

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2866704 , @ASDenysPetrov wrote: > @vsavchenko That's not the question I'm asking. Why do you need to set constraints for other symbolic expressions, when `SymbolicInferrer` can look them up on its own? Which

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @vsavchenko Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2797-2799 +ProgramStateRef +RangeConstraintManager::updateExistingConstraints(ProgramStateRef State, + SymbolRef

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. Can you please explain why you do the same thing in two different ways? Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296 +SymbolRef Sym = Operand; +while (isa(Sym)) + Sym = cast(Sym)->Operand; +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-09 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 357463. ASDenysPetrov added a comment. Added more descriptive comments. Fixed `RangeConstraintManager::updateExistingConstraints` function. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-08 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 357240. ASDenysPetrov added a comment. Added `SymExpr::ignoreCasts` method. Added descriptive comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 Files:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-06 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2801-2802 + // Get a root symbol in case of SymbolCast. + while (isa(Sym)) +Sym = cast(Sym)->getOperand(); + vsavchenko wrote: > ASDenysPetrov wrote:

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-06 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2801-2802 + // Get a root symbol in case of SymbolCast. + while (isa(Sym)) +Sym = cast(Sym)->getOperand(); + ASDenysPetrov wrote: > vsavchenko wrote: > >

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-06 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. I found some issues. Working on improvement. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2797-2799 +ProgramStateRef +RangeConstraintManager::updateExistingConstraints(ProgramStateRef State, +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-06 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. This is a very complicated patch, I think we'll have to iterate on it quite a lot. Additionally, we have to be sure that this doesn't crash our performance. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1131 +class

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-07-06 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 356668. ASDenysPetrov added a comment. Splitted this revision and moved `SValBuilder` related changes to separate patch D105340 . Added detailed comments. Made `NominalTypeList` //static// and as a result removed the

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-17 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2506-2520 + for (auto It = NTL.findByWidth(C.getIntWidth(T)) - 1; It >= NTL.begin(); + --It) { +SymbolRef S = State->getSymbolManager().getCastSymbol( +

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-17 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D103096#2822965 , @ASDenysPetrov wrote: > In D103096#2821750 , @vsavchenko > wrote: > >> Hey, great work! I think that casts are extremely important, but it looks >> like you

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-16 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. In D103096#2821750 , @vsavchenko wrote: > Hey, great work! I think that casts are extremely important, but it looks > like you mixed so many things into this patch. Let's make one step at a time > a split it into (at

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-16 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko requested changes to this revision. vsavchenko added a comment. This revision now requires changes to proceed. Hey, great work! I think that casts are extremely important, but it looks like you mixed so many things into this patch. Let's make one step at a time a split it into (at

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-16 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. What about this patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-08 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 350579. ASDenysPetrov added a comment. Added a boolean option `handle-integral-cast-for-ranges` under `-analyzer-config` flag. Disabled the feature by default. @Noq, @steakhal How do you think whether it's neccesory to add any changes in

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-04 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. There is so much fancy stuff going on upstream. Awesome to see. I'm trying to catch up ASAP, I'm finally done with my master's thesis. In D103096#2798238 , @NoQ wrote: > Additionally, presence of cast symbols is extremely

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-06-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D103096#2789439 , @ASDenysPetrov wrote: > @NoQ > This solution only intends to make correct calculations whenever cast > occures. We can mark this as //alpha// or add an argument flag to turn cast > reasoning on/off, or we can

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-05-31 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment. @NoQ This solution only intends to make correct calculations whenever cast occures. We can mark this as //alpha// or add an argument flag to turn cast reasoning on/off, or we can even disable any part of this patch with argument settings. > But this still

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-05-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. It sounds like you indeed solved a lot of problems that prevented us from enabling `SymbolCast`. But this still requires //massive// testing, a lot more than a typical constraint solver patch; extraordinary claims require extraordinary evidence. If it works out though, it

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

2021-05-25 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov created this revision. ASDenysPetrov added reviewers: NoQ, vsavchenko, steakhal, martong, dcoughlin, baloghadamsoftware. ASDenysPetrov added a project: clang. Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun.