tools/openocd: Fix handling of OPENOCD_CMD_RESET_HALT #19506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue.
maribu
added
Type: bug
benpicco
approved these changes
Apr 25, 2023
|
bors merge |
1 similar comment
|
bors merge |
|
Already running a review |
bors bot
added a commit
that referenced
this pull request
Apr 25, 2023
18620: core: add core_mutex_debug to aid debugging deadlocks r=benpicco a=maribu
### Contribution description
Adding `USEMODULE += core_mutex_debug` to your `Makefile` results in
on log messages such as
[mutex] waiting for thread 1 (pc = 0x800024d)
being added whenever `mutex_lock()` blocks. This makes tracing down
deadlocks easier.
### Testing procedure
Run e.g.
```sh
USEMODULE=core_mutex_debug BOARD=nucleo-f767zi make -C tests/mutex_cancel flash test
```
which should provide output such as
```
Welcome to pyterm!
Type '/exit' to exit.
READY
s
[mutex] waiting for thread 1 (pc = 0x8000f35)
START
main(): This is RIOT! (Version: 2022.10-devel-841-g5cc02-core/mutex/debug)
Test Application for mutex_cancel / mutex_lock_cancelable
=========================================================
Test without cancellation: OK
Test early cancellation: OK
Verify no side effects on subsequent calls: [mutex] waiting for thread 1 (pc = 0x800024d)
OK
Test late cancellation: [mutex] waiting for thread 1 (pc = 0x0)
OK
TEST PASSED
```
```sh
$ arm-none-eabi-addr2line -a 0x800024d -e tests/mutex_cancel/bin/nucleo-f767zi/tests_mutex_cancel.elf
0x0800024d
/home/maribu/Repos/software/RIOT/tests/mutex_cancel/main.c:51
```
### Issues/PRs references
Depends on and includes #18619
19296: nanocoap: allow to define CoAP resources as XFA r=benpicco a=benpicco
19504: cpu/cc26xx_cc13xx: Fix bogus array-bound warning r=benpicco a=maribu
### 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
19506: tools/openocd: Fix handling of OPENOCD_CMD_RESET_HALT r=maribu a=maribu
### Contribution description
The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue.
### Testing procedure
Flashing of e.g. the `cc2650-launchpad` with upstream OpenOCD should work again.
### Issues/PRs references
The change was added to #19050 after testing the PR and before merging. I'm not sure if the fix never worked because of this, or if behavior of `target-export-variables` or GNU Make changed.
19507: cpu/cc26x0_cc13x0: Drop feature cortexm_mpu r=benpicco a=maribu
### Contribution description
At least the CC2650 doesn't have an MPU, I assume this is also true for the rest of the family.
The CC2652 does have an MPU according to the datasheet. So I keep the feature there in place.
### Testing procedure
E.g.
```
make BOARD=cc2650-launchpad -C tests/mpu_noexec_ram flash test
```
fails. (Note: A successful test run would also crash but with a mem manage handler rather than a hardfault due to an invalid instruction on the stack being executed.)
It would be nice to also test the same for a `cc2652-launchpad`, for which the MPU should work.
### Issues/PRs references
None
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
|
Build failed (retrying...): |
|
bors r- |
|
Canceled. |
|
bors merge |
|
Build succeeded: |
|
Backport provided in #19510 |
bors bot
added a commit
that referenced
this pull request
Apr 28, 2023
19505: Rust: Update dependencies [backport 2023.04] r=maribu a=maribu # Backport of #19495 ### Contribution description This updates both the RIOT-specific and generic dependencies of Rust examples and modules. It also follows a deprecation from the G unit renaming originally done in #19292. ### Testing procedure * Green CI should do ### Issues/PRs references Copying from one of the commits with some sed: * riot-sys: * RIOT-OS/rust-riot-sys#26 * RIOT-OS/rust-riot-sys#31 * RIOT-OS/rust-riot-sys#30 * RIOT-OS/rust-riot-sys#28 * RIOT-OS/rust-riot-sys#27 * RIOT-OS/rust-riot-sys#25 * RIOT-OS/rust-riot-sys#23 * RIOT-OS/rust-riot-sys#22 * RIOT-OS/rust-riot-sys#21 * riot-wrappers: * RIOT-OS/rust-riot-wrappers#36 * RIOT-OS/rust-riot-wrappers#50 * RIOT-OS/rust-riot-wrappers#48 * RIOT-OS/rust-riot-wrappers#47 * RIOT-OS/rust-riot-wrappers#44 * RIOT-OS/rust-riot-wrappers#45 * RIOT-OS/rust-riot-wrappers#43 * (later, when the mistake became apparent) RIOT-OS/rust-riot-wrappers#55 ### How to do similar PRs Updating the RIOT-related dependencies (which here also updated bindgen because the dependency of riot-sys changed): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml --package riot-sys --package riot-wrappers; done ``` Updating everything (should never really be needed, b/c if something has a concrete dependency change it'd say so, but dependencies generally get better over time): ``` $ for x in **/Cargo.lock; do cargo update --manifest-path=${x%.lock}.toml; done ``` Creating the commit message: ``` $ git log --first-parent --format='%s (%b)' 9c29faf55d4c14d2d7f55f8df5059c52af4e5317..e4973a6ee88427f702dac41b3dce4fd6b6b9689e | sed 's/Merges: //' | sed 's/^/ * /' ``` git shortlog unfortunately doesn't show the merges the way I prefer linking them. The versions in the command line can be taken from `git diff --text` and looking for the riot-sys or riot-wrappers line, respectively. 19509: cpu/cc26xx_cc13xx: Fix bogus array-bound warning [backport 2023.04] r=maribu a=maribu # 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 19510: tools/openocd: Fix handling of OPENOCD_CMD_RESET_HALT [backport 2023.04] r=maribu a=maribu # Backport of #19506 ### Contribution description The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue. ### Testing procedure Flashing of e.g. the `cc2650-launchpad` with upstream OpenOCD should work again. ### Issues/PRs references The change was added to #19050 after testing the PR and before merging. I'm not sure if the fix never worked because of this, or if behavior of `target-export-variables` or GNU Make changed. Co-authored-by: chrysn <chrysn@fsfe.org> Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
The OPENOCD_CMD_RESET_HALT was not longer correctly passed to the script. This fixes the issue.
Testing procedure
Flashing of e.g. the
cc2650-launchpadwith upstream OpenOCD should work again.Issues/PRs references
The change was added to #19050 after testing the PR and before merging. I'm not sure if the fix never worked because of this, or if behavior of
target-export-variablesor GNU Make changed.