Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/cc26xx_cc13xx: Fix bogus array-bound warning [backport 2023.04] #19509

Merged

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