Fix int8 SVDF activation-state zero-point overflow (BUG=#2721)

[94xhn]

BUG=#2721

Problem

In tensorflow/lite/micro/kernels/svdf_common.cc's int8 feature-matmul
step (EvalIntegerSvdfReference<int8_t>), the activation_state
tensor's zero-point is added to the requantized dot product after
it has already been saturated to the int8 range:

dot_prod = MultiplyByQuantizedMultiplier(
    dot_prod, data.effective_scale_1_a, data.effective_scale_1_b);
dot_prod = std::min(std::max(output_min, dot_prod), output_max);  // clamp first
// The int16 version of the op assumes a zero_point of 0.  This
// code accounts for the potentially non-zero zero_point for the int8
// version of the op.
*result_in_batch = data.activation_state_zero_point + dot_prod;   // then add zero-point (narrowed into int8_t*)

Because the addition happens after the clamp and the destination is a
1-byte int8_t*, the sum zero_point + dot_prod can exceed the int8
range and silently wrap around (mod 256) instead of saturating when
it is narrowed into the persistent activation_state buffer. This
corrupts the SVDF's recurrent state for any model whose
activation_state tensor has a non-zero zero-point (an entirely legal,
non-pathological quantization parameter ? see
data->activation_state_zero_point = activation_state->params.zero_point;
a few lines below in the same file).

This exact defect is described in #2721.

Fix

Reorder to add the zero-point before the saturating clamp, matching
the pattern already used ~70 lines later in the same file's Rescale
step (requantize -> add zero-point -> clamp -> store):

dot_prod = MultiplyByQuantizedMultiplier(
    dot_prod, data.effective_scale_1_a, data.effective_scale_1_b);
// The int16 version of the op assumes a zero_point of 0.  This
// code accounts for the potentially non-zero zero_point for the int8
// version of the op. The zero-point must be added BEFORE the
// saturating clamp below (not after), otherwise the sum can exceed
// the int8 range and silently wrap around when narrowed into the
// persistent activation_state buffer.
dot_prod = data.activation_state_zero_point + dot_prod;
dot_prod = std::min(std::max(output_min, dot_prod), output_max);
*result_in_batch = dot_prod;

This is a 4-line functional change. The int16 activation-state
instantiation of the same template is unaffected, since its zero-point
is conventionally 0 (per the function's own comment) ? the reorder is
a no-op there.

Test

Added SvdfInt8ActivationStateZeroPointOverflowRegressionTest to
svdf_test.cc, a minimal (1x1x1x1) end-to-end SVDF invocation with a
non-zero activation_state zero-point (30), chosen together with the
other quantization scales (all exactly 1.0, so
MultiplyByQuantizedMultiplier is an exact identity transform) so the
correct expected output can be derived by hand precisely:

• Feature matmul: dot_prod = 120. Correct/fixed:
  clamp(30 + 120, -128, 127) = 127. Buggy: int8_t(30 + clamp(120)) = int8_t(150) wraps to -106.

• This propagates through the Time/Reduce/Rescale steps to a correct
  expected output of 97, vs. a buggy actual output of -128 ? well
  outside the test's tolerance of 1.

Note: this test builds its own tensors rather than reusing the existing
TestIntegerSVDF() helper, because that helper always constructs the
activation_state tensor with a hardcoded zero-point of 0
(irrespective of the activation_state_zero_point argument passed to
it), so it never actually exercised this code path ? which is
presumably part of why this bug went unnoticed.

I verified the buggy vs. fixed arithmetic independently in a standalone
C++ program (exactly reproducing the buggy statement sequence read
directly from this file) before making this change, confirming the
wraparound for several realistic in-range zero-point/dot-product
combinations, and confirming the fix does not change behavior when
zero_point == 0 or when no saturation is needed.

I don't have bazel/make available in my current environment to run
the full TFLM test suite (test_x86_default.sh), so I was unable to
locally execute the added test through the actual build; I did run
clang-format -i -style=google and cpplint on both modified files
per CONTRIBUTING.md (cpplint's remaining findings are pre-existing
style patterns already used throughout both files, not introduced by
this change). I'd appreciate a maintainer/CI check to confirm the new
test builds and passes as expected.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.