baloghadamsoftware added inline comments.
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:330
+ nonloc::ConcreteInt(Max), SVB.getConditionType());
+ if (auto DV = IsCappedFromAbove.getAs()) {
+if (State->assume(*DV, false))
geo
baloghadamsoftware updated this revision to Diff 138954.
baloghadamsoftware added a comment.
Updated according to the comments.
https://reviews.llvm.org/D41938
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/Si
baloghadamsoftware marked 12 inline comments as done.
baloghadamsoftware added inline comments.
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:326
+
+ llvm::APSInt Max = AT.getMaxValue() >> 2; // Divide by 4.
+ SVal IsCappedFromAbove =
george.karpenk
baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, dcoughlin.
Herald added subscribers: a.sidorin, szepet.
This patch is a "light" version of https://reviews.llvm.org/D35109:
Since the range-based constraint manager (default) is weak in handling
comparisons where
NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.
There are still performance regressions coming in, and this time it doesn't
look like it's my fault: https://bugs.llvm.org/show_bug.cgi?id=38208
I suspect that this might be because we aren't enforcing complexity thresholds
over a
baloghadamsoftware added a comment.
In https://reviews.llvm.org/D41938#1167313, @NoQ wrote:
> There are still performance regressions coming in, and this time it doesn't
> look like it's my fault: https://bugs.llvm.org/show_bug.cgi?id=38208
>
> I suspect that this might be because we aren't enfo
baloghadamsoftware added a comment.
In https://reviews.llvm.org/D41938#1167639, @baloghadamsoftware wrote:
> The flag is off by default. Except the rearrangement of additive operations.
> Should we put it also behind the flag?
I did it as a temporary quick fix: https://reviews.llvm.org/D49536.
NoQ added a comment.
I think it'd be fine to do the rearrangement for additive ops without the
option check, as we discussed. I.e., rearrange when it's either an additive op,
or both the flag is set and the values are overflow-clamped. And remove the
clamp checks (which are presumably heavy) fo
baloghadamsoftware updated this revision to Diff 130403.
baloghadamsoftware added a comment.
Updated according to the comments.
https://reviews.llvm.org/D41938
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/Si
george.karpenkov added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:597
+ // than or equal to the quarter of the maximum value of that type.
+ bool shouldAggressivelySimplifyRelationalComparison();
+
High level comment: can y
baloghadamsoftware added a comment.
Thank you for your comments. Since the original author of this particular code
is Artem, I think he will answer your questions.
https://reviews.llvm.org/D41938
___
cfe-commits mailing list
cfe-commits@lists.llvm.
NoQ added a comment.
George's style comments are usually super spot-on. Please feel free to improve
my code. Also it was only written as a proof-of-concept because i failed to
explain my approach with natural language, so it definitely needs polishing.
I'd let you know when i disagree with anyt
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
This looks good with a super tiny nit regarding comments about signed integers.
George, are you happy with the changes? (:
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOp
baloghadamsoftware updated this revision to Diff 140061.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.
Comment fixed.
https://reviews.llvm.org/D41938
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:675-677
+ /// is on the right. This is only done if both concrete integers are greater
+ /// than or equal to the quart
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.
George or Devin, please accept it or give me some feedback if not. Since this
patch affects the core infrastructure I think it is wise to merge it only if at
least two of you have accepted it. Artem is one,
george.karpenkov accepted this revision.
george.karpenkov added a comment.
LGTM. Sorry for the delay, I thought that a single acceptance was sufficient.
https://reviews.llvm.org/D41938
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329780: [Analyzer] SValBuilder Comparison Rearrangement
(with Restrictions and Analyzer… (authored by baloghadamsoftware, committed by
).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
NoQ added a comment.
We've found a crash on our internal buildbot, would you like to have a look?:
**`$ cat repro.c`**
int foo(int x, int y) {
short a = x - 1U;
return a - y;
}
**`$ clang -cc1 -analyze -analyzer-checker=core repro.c`**
Assertion failed: (APSIntType(LInt) == BV.ge
19 matches
Mail list logo
