[analyzer] Avoid contradicting assumption in tainted div-by-0 error node by necto · Pull Request #144491 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Arseniy Zaostrovnykh (necto)

Changes

This patch corrects the state of the error node generated by the core.DivideZero checker when it detects potential division by zero involving a tainted denominator.

The checker split in
91ac5ed started to introduce a conflicting assumption about the denominator into the error node:
Node with the Bug Report "Division by a tainted value, possibly zero" has an assumption "denominator != 0".

This has been done as a shortcut to continue analysis with the correct assumption after the division - if we proceed, we can only assume the denominator was not zero. However, this assumption is introduced one-node too soon, leading to a self-contradictory error node.

In this patch, I make the error node with assumption of zero denominator fatal, but allow analysis to continue on the second half of the state split with the assumption of non-zero denominator.

CPP-6376

Full diff: https://github.com/llvm/llvm-project/pull/144491.diff

2 Files Affected:

(modified) clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp (+4-4)
(modified) clang/test/Analysis/taint-generic.c (+13)

diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 15d73fb9ca39a..ab90615f63182 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -69,7 +69,7 @@ void DivZeroChecker::reportTaintBug( llvm::ArrayRef TaintedSyms) const { if (!TaintedDivChecker.isEnabled()) return;

if (ExplodedNode *N = C.generateNonFatalErrorNode(StateZero)) {

if (ExplodedNode *N = C.generateErrorNode(StateZero)) { auto R = std::make_unique(TaintedDivChecker, Msg, N); bugreporter::trackExpressionValue(N, getDenomExpr(N), *R); @@ -113,9 +113,9 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B, if ((stateNotZero && stateZero)) { std::vector taintedSyms = getTaintedSymbols(C.getState(), *DV); if (!taintedSyms.empty()) {

 reportTaintBug("Division by a tainted value, possibly zero", stateNotZero,

```
                C, taintedSyms);
```
```
 return;
```

 reportTaintBug("Division by a tainted value, possibly zero", stateZero, C,

```
                taintedSyms);
```

 // Fallthrough to continue analysis in case of non-zero denominator.

} }

diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index 3c520612c5d9b..9d6d2942df4a9 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -412,6 +412,19 @@ int testTaintedDivFP(void) { return 5/x; // x cannot be 0, so no tainted warning either }

+void clang_analyzer_warnIfReached(); + +int testTaintDivZeroNonfatal() {

int x;
scanf("%d", &x);
int y = 5/x; // expected-warning {{Division by a tainted value, possibly zero}}
if (x == 0)
clang_analyzer_warnIfReached();
else
clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
return y; +}
// Zero-sized VLAs. void testTaintedVLASize(void) { int x;