[clang] [StaticAnalyzer] DivZeroChecker: possibly zero non-tainted state should not transition to non-zero state directly.

[Ustchcs_Bugfinder]

Summary

We are trying to write checkers to catch possibly zero state. core.DivideZero transitioning to non-zero state directly leaves no space for other checkers to do anything else any more.

[martong]

We are trying to write checkers to catch possibly zero state.

IMO, that should be handled with taint analysis, i.e. when the value's provenance is untrusted we should warn. I don't see any other cases when we'd like to warn about a possible 0 denominator because that would cause false positives.

... core.DivideZero transitioning to non-zero state directly leaves no space for other checkers to do anything else any more.

To meaningfully continue the path sensitive analysis, we must assume that the value cannot be 0, otherwise the program would be illformed (undefined behaviour), thus it would not make any sense to continue the analysis on the path where the denom can also be 0.
Actually, this is a recurring pattern we do all over the a static analyzer in many checkers, checkout e.g. the DereferenceChecker and you will see the same pattern.

clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp

void DivZeroChecker::checkPreStmt(const BinaryOperator *B,
 
   bool TaintedD = isTainted(C.getState(), *DV);
   if ((stateNotZero && stateZero && TaintedD)) {
     reportBug("Division by a tainted value, possibly zero", stateZero, C,
               std::make_unique<taint::TaintBugVisitor>(*DV));
     return;
   }
 
+  if (stateZero) {
+    return;
+  }
+
   // If we get here, then the denom should not be zero. We abandon the implicit
   // zero denom case for now.
   C.addTransition(stateNotZero);
 }
 
 void ento::registerDivZeroChecker(CheckerManager &mgr) {
   mgr.registerChecker<DivZeroChecker>();
 }
 
 bool ento::shouldRegisterDivZeroChecker(const CheckerManager &mgr) {
   return true;
 }