Skip to content

feat: Add -Wwraparound to check for index out of bounds. - #289

Open
iphydf wants to merge 1 commit into
TokTok:masterfrom
iphydf:wraparound
Open

iphydf wants to merge 1 commit into
TokTok:masterfrom
iphydf:wraparound

Conversation

@iphydf

@iphydf iphydf commented Oct 25, 2026 •

Copy link
Copy Markdown
Member
error: `sequnum - q->bottom` can wrap around to values from 4294901761 to 4294967295, because it is the difference of 16-bit values but wraps around at 32 bits here [-Wwraparound]
   --> c-toxcore/toxav/groupav.c:107:20
    |
 107|     if ((uint32_t)(sequnum - q->bottom) > q->size) {
    |                    ^^^^^^^^^^^^^^^^^^^
error: `sequnum - q->bottom` can wrap around to values from 4294901761 to 4294967295, because it is the difference of 16-bit values but wraps around at 32 bits here [-Wwraparound]
   --> c-toxcore/toxav/groupav.c:101:24
    |
 101|         if ((uint32_t)(sequnum - q->bottom) > (1 << 15)) {
    |                        ^^^^^^^^^^^^^^^^^^^
error: `q->top - q->bottom` can wrap around to values from 4294901761 to 4294967295, because it is the difference of 16-bit values but wraps around at 32 bits here [-Wwraparound]
   --> c-toxcore/toxav/groupav.c:153:20
    |
 153|     if ((uint32_t)(q->top - q->bottom) > q->capacity) {
    |                    ^^^^^^^^^^^^^^^^^^
error: `friend_con->tcp_relay_counter - (i + 1)` can wrap around to values from 4294967272 to 4294967295, and 4294967296 is not a multiple of 24, so `% 24` then gives a different index than it would without the wrap [-Wwraparound]
   --> c-toxcore/toxcore/friend_connection.c:275:33
    |
 275|         const uint16_t index = (friend_con->tcp_relay_counter - (i + 1)) % FRIEND_MAX_STORED_TCP_RELAYS;
    |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: `(uint16_t) (gconn->send_array_start - 1)` can wrap around to 65535, but `gconn->send_message_id % 2048` is always between 0 and 2047, so the comparison is always false when it wraps [-Wwraparound]
   --> c-toxcore/toxcore/group_connection.c:140:44
    |
 140|     if ((gconn->send_message_id % GCC_BUFFER_SIZE) == (uint16_t)(gconn->send_array_start - 1)) {
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

(reason for misplaced annotation is preproc, we're using C instead of Cimple here. Cimple wouldn't have that issue, but language-c does)


This change is Reviewable

@iphydf iphydf added this to the v0.0.9 milestone Oct 25, 2026
@github-actions github-actions Bot added the enhancement New feature for the user, not a new feature for build script label Oct 25, 2026
@iphydf
iphydf marked this pull request as ready for review October 25, 2026 20:57
@iphydf
iphydf force-pushed the wraparound branch 5 times, most recently from 405e99a to 1493339 Compare October 26, 2026 11:24

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature for the user, not a new feature for build script

Development

Successfully merging this pull request may close these issues.

1 participant