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 for certain, interesting constructs (which 
is controlled by the critical option). GenericTaintChecker focuses purely on 
taintedness, almost like a service for other checkers. I've added a new rule to 
it, improving the taintedness logic, but I felt mixing the bound checking logic 
into it would make the two ideas inseparable.
I've also checked others using tainted values. ArrayBoundCheckerV2 also works 
with array indexing and modifies its behaviour on finding tainted values. 
However, the main difference is that ArrayBoundCheckerV2 uses region 
information to check bounds which may or may not be present, while this checker 
can give a warning whenever any information from the constraint manager does 
not prove that any bound checking were performed on the value before (and 
potentially works on many other constructs where region information shouldn't 
be there at all). Not having correct region information with tainted values is 
typical when reading data from unknown sources. This dirty approach is better 
in this regard. ArrayBoundCheckerV2 on the other hand can give warnings solely 
based on region information. Yes, it can happen that both will give a warning 
as well for the same construct. Do you think it is distracting? Should I remove 
the array indexing checks from this checker (still it gives warning for pointer 
arithmetic as well)?



================
Comment at: test/Analysis/dirty-scalar-unbounded.cpp:1
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify 
-analyzer-config alpha.security.DirtyScalar:criticalOnly=false %s
+
----------------
a.sidorin wrote:
> 1. It will be good to have tests for option set to true.
> 2. Is there any test that makes usage of 'RecurseLimit` variable?
Now both settings are covered. For RecurseLimit, I've added a named constant 
and better explanation. This is only a practical limit to not let the analysis 
dive too deep into complex loop expressions. The limit currently set should be 
adequate, so it would not make sense to set it programmatically.


https://reviews.llvm.org/D27753



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to