NoQ added a comment.

> this would be a great flow-sensitive check for the clang static analyzer

You could indeed use static analyzer's path sensitivity to get rid of these 
obvious false positives where the multiplication is performed pretty much 
immediately after a branch (or assert) that avoids overflow, where "pretty much 
immediately" requires the static analyzer to encounter the overflow check while 
interpreting the function that contains the potential overflow. It's still not 
going to be good enough to be a good on-by-default checker because such checks 
may be performed either before the function is invoked, or in a nested function 
call for which the body is not immediately available in the translation unit. A 
good on-by-default checker could be achieved if taint analysis is added to the 
mixture (i.e., only check for overflows of values that are known to definitely 
be arbitrary) but that'd limit the power of the checker dramatically because 
such knowledge is rarely available (you may add annotations for that but that's 
probably a lot of work in your code).

Warnings with false positives may still be useful when they define a clear 
coding convention that you want the users to follow. In your case such 
convention seems to be "always use integer types that are wide enough to avoid 
overflows" and it says nothing about overflow checks being made, so i guess you 
don't absolutely need to bother with flow/path sensitivity. I think the answer 
should be data-driven. Does your codebase already contain overflow checks that 
cause false positives all over the place? If so, static analyzer to the rescue. 
If most of your multiplications are unchecked and you already rely on the 
integer type to be wide enough then you're probably good as-is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93822/new/

https://reviews.llvm.org/D93822

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

Reply via email to