RPN expression evaluator: EXC/REV are no-ops, AVG is fatal on unknown, result read from stack bottom

[somethingwithproof]

The RPN expression evaluator (thold_functions.php:329-1060) has several defects that make expression-type thresholds return wrong values or abort the poller. Grouping them because they share one test surface: the thold_expression_*_rpn() family is pure, so all of these are reproducible without a database.

Each was confirmed by running the function, not by reading it.

EXC does not exchange (thold_functions.php:637-643)

$v1 = thold_expression_rpn_pop($stack);   // top
$v2 = thold_expression_rpn_pop($stack);   // next
array_push($stack, $v2);
array_push($stack, $v1);

Popping reverses, then pushing in the same order restores it. [1,2] EXC yields [1,2]; RRDtool gives [2,1].

REV does not reverse (thold_functions.php:673-687)

Same shape: the pop loop already reverses, then array_reverse() puts it back. [1,2,3] 3 REV yields [1,2,3] instead of [3,2,1].

An expression using either operator to order operands before a non-commutative operator (-, /, LT) computes the wrong comparison, silently.

AVG is fatal on an unknown sample (thold_functions.php:705)

} else {
    $total += $v;
}

'U' and 'NAN' are pushed as literal strings elsewhere in the evaluator, and reach this arm. On PHP 8 int + "U" throws TypeError: Unsupported operand types, uncaught, in the poller process. Every threshold after it in that run goes unevaluated.

RRDtool's AVG skips unknowns and divides by the count of known values, returning UNKN when all are unknown.

The expression result is taken from the bottom of the stack (thold_functions.php:1060)

return $stack[0];

RRDtool RPN yields the top. 1,2,3,+ returns 1 rather than 5. For an expression that leaves one value this is the same element, so it only bites when the stack is unbalanced — which the EXC/REV bugs above make more likely. An expression ending in POP reads an undefined index.

CURRENT_GRAPH_MAXIMUM_VALUE is unreachable (thold_functions.php:986-987)

$spectypes = ['CURRENT_DATA_SOURCE', 'CURRENT_GRAPH_MINIMUM_VALUE',
    'CURRENT_GRAPH_MINIMUM_VALUE', 'CURRENT_DS_MINIMUM_VALUE', ...];

The minimum appears twice and the maximum is absent, although thold_expression_specialtype_rpn() handles it at :828. The dispatch falls through to Unsupported Field, sets $rpn_error, and the whole expression returns 0. A low threshold on such an expression then alerts permanently.

thold_rpn() modulo by zero is fatal (thold_functions.php:4798)

The CDEF evaluator guards division but not modulo, so % by zero throws DivisionByZeroError. The division guard also returns -1, a plausible-looking number that is then compared against the bounds; the function's own invalid sentinel elsewhere is ''.

$rpn_error is not checked before the popped value is used

thold_expression_rpn_pop() (:329) sets the flag and returns false on underflow, but only _math_rpn checks it. _boolean_rpn, _compare_rpn, _setops_rpn and _stackops_rpn push the false back onto the stack and carry on.

Related, in the same family:

• AND/OR (:460, :469) test $v1 > 0 && $v2 > 0; RRDtool treats any non-zero value as true, so negative operands are inverted.
• IF (:479) tests $v3 == 0, and 'U' == 0 is false on PHP 8, so an unknown condition takes the then branch instead of yielding unknown.
• MIN/MAX (:556-563) check INF before U, inverting RRDtool's precedence.

Suggested approach

These are dense enough, and the functions pure enough, that replacing the evaluator against RRDtool's documented semantics is more tractable than patching each arm. RRDtool's rpn documentation is a usable specification and rrdtool graph output can serve as the oracle.

I have a PR in progress.

[somethingwithproof]

Implementation in #788. All six items above are covered there with tests that fail on develop.

[somethingwithproof]

Phase 1 characterization tests for the evaluator are in #799. They cover the hi/low and time-based arms and pin present behaviour before any restructuring, which is the safety net the RPN work in #788 did not need but the state machine does.