Skip to content

Conversation

maribu
Copy link
Member

@maribu maribu commented Apr 25, 2023

Backport of #19504

Contribution description

GCC 12 create a bogus array out of bounds warning as it assumes that because there is special handling for uart == 0 and uart == 1, uart can indeed be 1. There is an assert(uart < UART_NUMOF) above that would blow up prior to any out of bounds access.

In any case, optimizing out the special handling of uart == 1 for when UART_NUMOF == 1 likely improves the generated code and fixes the warning.

/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:88:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
   88 |     ctx[uart].rx_cb = rx_cb;
      |     ~~~^~~~~~
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
   52 | static uart_isr_ctx_t ctx[UART_NUMOF];
      |                       ^~~
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:89:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
   89 |     ctx[uart].arg = arg;
      |     ~~~^~~~~~
/home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
   52 | static uart_isr_ctx_t ctx[UART_NUMOF];
      |                       ^~~

Testing procedure

The actual change is a pretty obvious one-liner, so that code review and a green CI should be sufficient. If not, running any UART example app without regression should do.

Issues/PRs references

None

GCC 12 create a bogus array out of bounds warning as it assumes that
because there is special handling for `uart == 0` and `uart == 1`,
`uart` can indeed be `1`. There is an `assert(uart < UART_NUMOF)` above
that would blow up prior to any out of bounds access.

In any case, optimizing out the special handling of `uart == 1` for
when `UART_NUMOF == 1` likely improves the generated code and fixes
the warning.

    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:88:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
       88 |     ctx[uart].rx_cb = rx_cb;
          |     ~~~^~~~~~
    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
       52 | static uart_isr_ctx_t ctx[UART_NUMOF];
          |                       ^~~
    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:89:8: error: array subscript 1 is above array bounds of 'uart_isr_ctx_t[1]' [-Werror=array-bounds]
       89 |     ctx[uart].arg = arg;
          |     ~~~^~~~~~
    /home/maribu/Repos/software/RIOT/cc2650/cpu/cc26xx_cc13xx/periph/uart.c:52:23: note: while referencing 'ctx'
       52 | static uart_isr_ctx_t ctx[UART_NUMOF];
          |                       ^~~

(cherry picked from commit d6499fa)
@maribu maribu requested a review from smlng as a code owner April 25, 2023 20:12
@maribu maribu added Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: ARM Platform: This PR/issue effects ARM-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Apr 25, 2023
@riot-ci
Copy link

riot-ci commented Apr 25, 2023

Murdock results

✔️ PASSED

59e8458 cpu/cc26xx_cc13xx: Fix bogus array-bound warning

Success Failures Total Runtime
6882 0 6882 10m:13s

Artifacts

@MrKevinWeiss
Copy link
Contributor

OK, we will add this in but not create a new RC, if there is something else that comes up that is worth doing a point release it will be added.

Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@MrKevinWeiss
Copy link
Contributor

bors merge

@MrKevinWeiss
Copy link
Contributor

bors cancel

@bors
Copy link
Contributor

bors bot commented Apr 26, 2023

Canceled.

@MrKevinWeiss
Copy link
Contributor

This should be merged into the release branch after the release is finished.

@MrKevinWeiss
Copy link
Contributor

Now that the release is done, bors merge

@maribu
Copy link
Member Author

maribu commented Apr 28, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 28, 2023

Build succeeded:

@bors bors bot merged commit f10a5b4 into RIOT-OS:2023.04-branch Apr 28, 2023
@maribu maribu deleted the backport/2023.04/cc2650-compilation-fixes branch April 23, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: ARM Platform: This PR/issue effects ARM-based platforms Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants