[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[Artem Dergachev via Phabricator via cfe-commits]

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