zaks.anna added a comment.
This generally sounds good. Definitely do submit these changes in small pieces!
See http://llvm.org/docs/DeveloperPolicy.html#incremental-development for
rationale.
https://reviews.llvm.org/D27753
___
cfe-commits mailing
gerazo added a comment.
Before abandoning this patch and rewriting it, I would like to get a thumbs up
for my plans: I will reimplement all functionality included here but without
creating a new checker. Some parts which relate to specific checkers will be
put into the corresponding checkers (l
gerazo added a comment.
> Stepping back a bit, what do you consider "dirty" vs "clean"? It seems that
> you are looking for prove that the values are known to be within the bounds
> of min and max int values. What happens if there is a comparison to an
> unknown symbolic value? Should that be c
zaks.anna added a comment.
> All this leads to the need of several types of taintednesses (string
> taintedness, array taintedness, general bound check taintedness) because the
> cleansing can only take down one type of taintedness at a time. That would be
> the only way for a checker to be abl
gerazo marked an inline comment as done.
gerazo added a comment.
Hmm... I am thinking on this issue for a week now...
I've played with the idea of implementing cleansing rules in
GenericTaintChecker. It would be elegant but unfortunately, I have to think
they are not general. Cleansing of a str
gerazo marked an inline comment as done.
gerazo added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp:184
+Ty = Ctx.IntTy;
+ if (!Ty->isIntegerType() || Ctx.getIntWidth(Ty) <= TooNarrowForBoundCheck)
+return false;
a.sidor
zaks.anna added a comment.
> In this checker, I give warnings for values which are both tainted and were
> also not checked by the programmer. So unlike GenericTaintChecker, I do
> implement the boundedness check here for certain, interesting constructs
> (which is controlled by the critical op
a.sidorin added a comment.
Hello Zoltan. Sorry, I'm a bit busy now. Here are my thoughts about the design.
1. I think we should not add new warnings to GenericTaintChecker. Instead, it
is better to move warnings out of it. After that it will become just a plugin
used by other checkers. Such sp
gerazo added a comment.
Hi, did you have time to check my changes?
https://reviews.llvm.org/D27753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
gerazo marked an inline comment as done.
gerazo added a comment.
So thank you again for the valuable questions.
In this checker, I give warnings for values which are both tainted and were
also not checked by the programmer. So unlike GenericTaintChecker, I do
implement the boundedness check here
gerazo marked 6 inline comments as done.
gerazo added a comment.
Thank you very much for your help. I've added all suggested modifications
including tests covering all checker option settings.
https://reviews.llvm.org/D27753
___
cfe-commits mailing
gerazo updated this revision to Diff 82336.
https://reviews.llvm.org/D27753
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
test/Ana
zaks.anna added a comment.
> Did you checked if same warnings may be emitted by another checkers? For
> example,
> ArrayBoundChecker may warn if index is tainted.
I second that. The GenericTaintChecker also reports uses of tainted values. It
is not clear that we should add a new separate chec
a.sidorin added a comment.
Thank you for your work, Zoltán!
Did you checked if same warnings may be emitted by another checkers? For
example, ArrayBoundChecker may warn if index is tainted.
I also have some comments below.
Comment at: lib/StaticAnalyzer/Checkers/DirtyScalarChe
gerazo created this revision.
gerazo added reviewers: zaks.anna, dcoughlin.
gerazo added a subscriber: cfe-commits.
Herald added a subscriber: mgorny.
Checker for catching tainted value usage without proper bound checking. Uses
GenericTaintChecker which is also in alpha.security.
https://review
15 matches
Mail list logo
