Skip to content

Masked the core maps passed to the SMP remap solution search against TX_THREAD_SMP_CORE_MASK, so the computed core index is provably within the schedule list and GCC no longer reports a false array bounds error at -O2 - #728

Open
fdesbiens wants to merge 1 commit into
eclipse-threadx:devfrom
fdesbiens:fix/issue-469

Conversation

@fdesbiens

Copy link
Copy Markdown
Contributor

Description

_tx_thread_smp_remap_solution_find derives a core number from the lowest set bit of the core maps it is given, then uses that number to index _tx_thread_smp_schedule_list and the local thread_remap_list, both of which have TX_THREAD_SMP_MAX_CORES entries.

Every map that reaches the function is already restricted to the cores that are present: tx_thread_smp_cores_allowed is masked with TX_THREAD_SMP_CORE_MASK in _tx_thread_create and in _tx_thread_smp_core_exclude, and the available cores map is built from the schedule list. The compiler cannot see that invariant, so when the function is inlined into _tx_thread_system_suspend at -O2, GCC assumes the returned bit number can be as high as 31 and reports an out of bounds array subscript. Under -Werror that fails the build.

This change masks the three incoming maps with TX_THREAD_SMP_CORE_MASK at the top of the function, in both the inline version in common_smp/inc/tx_thread.h and the out of line twin in common_smp/src/tx_thread_smp_utilities.c. The masks are semantic no-ops, so scheduling behaviour is unchanged, but the value range is now visible to the optimizer. The first core queue entry is also initialized, because the narrowed range lets GCC consider the case of an empty queue on entry and it would otherwise emit a -Wmaybe-uninitialized warning in its place.

Fixes #469

Testing

Reproduced the original error with arm-none-eabi-gcc 13.2.1 against the Cortex-A9 SMP port at -O2 with the reporter's warning set, and confirmed it is gone after the change. Swept -O2, -O3 and -Os against 2, 4 and 8 core configurations, with and without TX_THREAD_SMP_DYNAMIC_CORE_MAX, TX_NOT_INTERRUPTABLE and TX_THREAD_SMP_EQUAL_PRIORITY, all clean under -Wall -Wextra -Werror. The SMP regression suite passes 116 of 116.

…TX_THREAD_SMP_CORE_MASK, so the computed core index is provably within the schedule list and GCC no longer reports a false array bounds error at -O2

The remap solution search derives a core number from the lowest set bit of
the supplied core maps and uses it to index _tx_thread_smp_schedule_list and
the local remap list, both of which have TX_THREAD_SMP_MAX_CORES entries.
Every map reaching the function is already restricted to the cores that are
present, because tx_thread_smp_cores_allowed is masked with
TX_THREAD_SMP_CORE_MASK in tx_thread_create and tx_thread_smp_core_exclude.
The compiler cannot see that invariant, however, so when the function is
inlined into _tx_thread_system_suspend at -O2 GCC assumes the bit number can
be as high as 31 and reports an out of bounds array subscript, which fails
the build under -Werror.

Masking the three incoming maps with TX_THREAD_SMP_CORE_MASK makes the
invariant explicit. The masks are semantic no-ops, so scheduling behaviour is
unchanged. The first core queue entry is now initialized as well, since the
narrowed value range lets the compiler consider an empty queue on entry.

Fixes eclipse-threadx#469

Assisted-by: Copilot (Opus 5) <noreply@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant