v17rx: signed overflow in DDS phase differences, V.17 training fails at -O2 unless built with -fwrapv

[MaximeLussier]

Environment

• spandsp master @ 8f1e164 (3.1.1). 0.0.6 shows the same thing.
• GCC 13.3 (Ubuntu 24.04), x86_64, CFLAGS=-O2. No -fwrapv.
• Used through Asterisk 22 res_fax_spandsp (audio/G.711 fax). Reproduced offline by feeding the recorded G.711 audio to fax_rx().

Symptom

V.17 receive fails intermittently: "Short training failed", "Training failed (sequence failed)", then PPR/CTC loops, and finally the call ends with an error. The same library built at -O0, or at -O2 -fwrapv, decodes the same recordings fine.

Cause

In process_half_baud() several phase differences are computed as int32_t subtractions of wrapping DDS phases. They overflow in normal operation:

v17rx.c:682   if ((uint32_t) (angle - s->last_angles[0]) < (uint32_t) DDS_PHASE(180.0f))
v17rx.c:697   phase_step = angle - DDS_PHASE(180.0f + 18.433f);
v17rx.c:731   ang = angle - s->last_angles[i & 1];
v17rx.c:770   phase_step = angle - DDS_PHASE(18.433f);
v17rx.c:939   ang = angle - s->last_angles[s->training_count & 1];

UBSan (-fsanitize=signed-integer-overflow) on one real call:

v17rx.c:697:32: runtime error: signed integer overflow: 279895616 - -1927569408 cannot be represented in type 'int'
v17rx.c:770:32: runtime error: signed integer overflow: -2021760640 - 219914256 cannot be represented in type 'int'
v17rx.c:939:13: runtime error: signed integer overflow: 280882688 - -1927569408 cannot be represented in type 'int'

At -O2 the compiler relies on "no overflow" when it folds the ang > DDS_PHASE(90) || ang < DDS_PHASE(-90) tests. As a result, the phase reversal checks give wrong answers.

Numbers

(118 offline replays of real bench calls, same harness and inputs, only the library build changes)

build	pages decoded	calls completed
-O2 (as shipped)	123	43
-O2 -fwrapv	191	68
-O2 + patch (no -fwrapv)	192	69

The remaining calls in the set are recordings of calls that failed for other reasons, so they are not expected to complete. With the patch, UBSan reports nothing in v17rx.c on the whole set.

Fix

patch:

0001-v17rx-take-DDS-phase-differences-in-unsigned-arithme.patch

It adds a phase_diff() helper that subtracts in uint32_t. Otherwise, it should be build with -fwrapv.
Other modems, v29rx.c and v27ter_rx.c, have similar angle - last_angle code that should be addressed...

NB: Claude Code Opus 5.5 was used to find the issue, write the patch and draft this report, I re-wrote parts of it.

[pfournier]

@coral opened pull request #128 fixing those overflows a few weeks ago.

[MaximeLussier]

@pfournier That's a hell of a coincidence ;) Good thing this gets fixed!