This revision was automatically updated to reflect the committed changes.
Closed by commit rL278941: [analyzer] Add a checker for loss of sign or
precision in integral casts. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D13126?vs=67817&id=68372#toc
Repository:
r
NoQ accepted this revision.
NoQ added a reviewer: NoQ.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good!
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:67
@@ +66,3 @@
+ const Stmt *Parent = PM.getParent(Cast);
+ if (!Parent)
+
danielmarjamaki marked 3 inline comments as done.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:67
@@ +66,3 @@
+ const Stmt *Parent = PM.getParent(Cast);
+ if (!Parent)
+return;
I get assertion failure then when running this test:
Analysis/
danielmarjamaki updated this revision to Diff 67817.
danielmarjamaki added a comment.
fixed review comments
https://reviews.llvm.org/D13126
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
NoQ added a comment.
Wow, i've completely forgot about this one. Sorry about that... I think this
checker is good to have in the current shape, and probably incrementally
developed. It'd probably move out of alpha whenever it's tweaked to somehow
make sure it finds enough real bugs among intent
danielmarjamaki updated this revision to Diff 67470.
danielmarjamaki added a comment.
Make sure patch can be applied in latest trunk.
Ping.
https://reviews.llvm.org/D13126
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyz
danielmarjamaki added a comment.
In http://reviews.llvm.org/D13126#381772, @zaks.anna wrote:
> Could you add the reduced false positives to the tests file?
>
> > As far as I see the diagnostics are showing the proper path now..
>
>
> What do you mean? Does this refer to supplying more information
danielmarjamaki updated this revision to Diff 52029.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
Minor fixes of review comments. Renamed functions to "isNegative" and
"isGreaterEqual" and return bool. Updated comment. Added testcases for false
positives I h
zaks.anna added a comment.
Could you add the reduced false positives to the tests file?
> As far as I see the diagnostics are showing the proper path now..
What do you mean? Does this refer to supplying more information the the path
about why the error occurs?
Comment at: li
danielmarjamaki marked 3 inline comments as done.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:12
@@ +11,3 @@
+//
+// ConversionChecker generates a subset of the warnings that are reported by
+// Wconversion. It is designed to be an alternative to Wconversion.
---
danielmarjamaki updated this revision to Diff 51402.
danielmarjamaki added a comment.
Minor fixes of review comments.
- Improved comments about the checker in the source code.
- Improved capitalization and punctuation in error messages.
- Renamed "canBe..." functions and changed their return type
danielmarjamaki added a comment.
Here are some false positives I have marked... let me know if you want more...
I have marked this as a FP:
URL:
ftp://ftp.se.debian.org/debian/pool/main/m/m17n-lib/m17n-lib_1.7.0.orig.tar.gz
File: m17n-lib-1.7.0/src/mtext.c
Line 1946
Code:
int mtext_set_cha
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
-
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val)
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val)
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
---
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val)
danielmarjamaki added a comment.
In http://reviews.llvm.org/D13126#379306, @zaks.anna wrote:
> Why is there such a large jump in the number of warnings reported in the last
> patch iteration?
> It went from "1678 projects where scanned. In total I got 124 warnings" to
> "In 2215 projects it fo
zaks.anna added a comment.
Why is there such a large jump in the number of warnings reported in the last
patch iteration?
It went from "1678 projects where scanned. In total I got 124 warnings" to "In
2215 projects it found 875 warnings." Did the number of warnings in the initial
1678 projects
danielmarjamaki added a comment.
ping
http://reviews.llvm.org/D13126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
http://reviews.llvm.org/D13126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
http://reviews.llvm.org/D13126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
In http://reviews.llvm.org/D13126#335929, @danielmarjamaki wrote:
> For information, I am testing this patch right now.. it will take a while 1-2
> days.
I have run my latest patch..
In 2215 projects it found 875 warnings.
For comparison the -Wconversion gene
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
For information, I am testing this patch right now.. it will take a while 1-2
days.
http://reviews.llvm.org/D13126
___
cfe-commits mailing list
cfe-commits@lists.llv
danielmarjamaki updated this revision to Diff 45972.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
fixed review comments, readded 'loss of sign' checking
http://reviews.llvm.org/D13126
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/
danielmarjamaki marked 7 inline comments as done.
danielmarjamaki added a comment.
http://reviews.llvm.org/D13126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki updated this revision to Diff 43997.
danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.
Refactorings thanks to review comments
http://reviews.llvm.org/D13126
Files:
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/Checker
danielmarjamaki marked 4 inline comments as done.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:41
@@ +40,3 @@
+const Stmt *Parent = PM.getParent(Cast);
+if (!Parent)
+ return;
a.sidorin wrote:
> Parent should always exist for an impli
a.sidorin added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:44
@@ +43,3 @@
+
+const BinaryOperator *B = dyn_cast(Parent);
+if (!B)
Note that InitExprs of DeclStmts are not binary operators. So you will not get
a warning
a.sidorin added a subscriber: a.sidorin.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:41
@@ +40,3 @@
+const Stmt *Parent = PM.getParent(Cast);
+if (!Parent)
+ return;
Parent should always exist for an implicit cast. May be it's better
danielmarjamaki added a comment.
Thanks a lot for those comments. I'll try your suggestions. I will try to
upload some samples where I think the ProgramState is wrong.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:78
@@ +77,3 @@
+
+// Can E value be greater or e
NoQ added a comment.
Hmm, just noticed the related work on casts in http://reviews.llvm.org/D12901,
which seems to be directly related to my hand-waving above. It might
accidentally be useful for reducing FPs of this checker as well.
http://reviews.llvm.org/D13126
__
NoQ added a comment.
I've got a few minor code comments.
I really wish to have a look at false positives on which
> the value analysis fails and then there is not much my checker could do
either in a form of FIXME tests, or as preprocessed code samples, because i'm
currently digging the topic
danielmarjamaki added a comment.
ping. I would like to have a review on this.
http://reviews.llvm.org/D13126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
> I classified 91 warnings as TP. 14 as FP. and then there were 19 that I
> failed to triage. I for instance failed to triage code implemented in headers
> when I don't know what values function arguments will have.
Sorry I calculated wrong.
I classified 91 wa
danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.
I have looked all warnings that I got. 1678 projects where scanned. In total I
got 124 warnings.
I classified 91 warnings as TP. 14 as FP. and then there were 19 that I failed
to triage. I for instance failed to
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
In http://reviews.llvm.org/D13126#302647, @dcoughlin wrote:
> In http://reviews.llvm.org/D13126#302328, @danielmarjamaki wrote:
>
> > When scanning 692 projects with this checker I got 56 warnings. I've
> > triage
dcoughlin added a subscriber: dcoughlin.
dcoughlin added a comment.
In http://reviews.llvm.org/D13126#302328, @danielmarjamaki wrote:
> When scanning 692 projects with this checker I got 56 warnings. I've triaged
> 21 random warnings of these so far and saw 20 TP and 1 FP.
>
> When I have triage
danielmarjamaki updated this revision to Diff 41858.
danielmarjamaki added a comment.
Another diff.
This time I have stripped the checker to make it simpler to review, triage, etc.
It will now only warn about loss of precision in assignments when RHS is a
variable.
For me, triaging these resul
NoQ added a subscriber: NoQ.
NoQ added a comment.
In http://reviews.llvm.org/D13126#291763, @danielmarjamaki wrote:
> I have problems with the "default" handling of expressions.
>
> I want to warn about loss of precision for such code:
>
> unsigned int x = 256;
> unsigned char c;
> c = x;
>
danielmarjamaki added a comment.
ping..
the latest patch has an alternative approach, where the checker track values of
symbols.
REGISTER_MAP_WITH_PROGRAMSTATE(DeclVal, const ValueDecl *, SVal)
The reason I don't use normal ProgramState values is that these symbol values
are truncated.
I w
danielmarjamaki updated this revision to Diff 40505.
danielmarjamaki added a comment.
I have problems with the "default" handling of expressions.
I want to warn about loss of precision for such code:
unsigned int x = 256;
unsigned char c;
c = x;
The analyser tells me that the RHS value is
danielmarjamaki added a comment.
In http://reviews.llvm.org/D13126#270193, @danielmarjamaki wrote:
> > It might be more useful if you could print the paths on which the errors
> > occurred (this could be done for text output with -analyzer-output=text)
>
>
> Sounds good. Is it possible to use it
danielmarjamaki added a comment.
> It might be more useful if you could print the paths on which the errors
> occurred (this could be done for text output with -analyzer-output=text)
Sounds good. Is it possible to use it with scan-build?
Comment at: lib/StaticAnalyzer/Checker
zaks.anna added a comment.
This checker produces a lot of warnings! Have you analyzed how many are false
positives? Have you tried reporting these warnings?
It's hard to make use of the results you posted from the debian packages. For
most of them, I cannot tell if they are valid reports or fal
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: zaks.anna.
danielmarjamaki added a subscriber: cfe-commits.
This is a new static analyzer checker that warns when there is loss of sign and
loss of precision.
It is similar in spirit to Wsign-compare/Wsign-conversion etc. B
46 matches
Mail list logo
