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.

Sign up for GitHub

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

Jump to bottom

sim: do not exit simulator on up_assert (just let the task exit) #3205

Merged
merged 1 commit into from
Mar 29, 2021
Merged

sim: do not exit simulator on up_assert (just let the task exit) #3205

merged 1 commit into from
Mar 29, 2021

Conversation

protobits
Copy link
Contributor

Summary

up_assert was exiting simulator which means that using assert from an application would never print the error and just exit the task (as it should).

Impact

Fix assert() on sim

Testing

Custom config based on sim:nsh. I can see the assert message now and only the app exits

@protobits protobits added the Type: Bug Something isn't working label Mar 27, 2021
@patacongo
Copy link
Contributor

patacongo commented Mar 27, 2021 via email

@patacongo
Copy link
Contributor

patacongo commented Mar 27, 2021 via email •
edited
Loading

@protobits
Copy link
Contributor Author

Thanks! That's definitely helpful.

@protobits
Copy link
Contributor Author

@patacongo can you review again? I simply used board_power_off in the end since it was already doing the same simulation abort.

@patacongo
Copy link
Contributor

patacongo commented Mar 28, 2021 •
edited
Loading

@patacongo can you review again? I simply used board_power_off in the end since it was already doing the same simulation abort.

The only issue that I see is that board_power_off is only built if CONFIG_BOARDCTL_POWEROFF is enabled. I would expect CI to fail since CONFIG_BOARDCTL_POWEROFF is only enabled in about half of the sim configurations:

$ find . -name defconfig | xargs grep -l BOARDCTL_POWEROFF | wc -
     30      30     846 -
$ find . -name defconfig | xargs grep -L BOARDCTL_POWEROFF | wc -
     21      21     562 -

Perhaps you could prototype in up_internal.h and make it build unconditionally.

@protobits
Copy link
Contributor Author

@patacongo can you review again? I simply used board_power_off in the end since it was already doing the same simulation abort.

The only issue that I see is that board_power_off is only build if CONFIG_BOARDCTL_POWEROFF is enabled. I would expect CI to fail since CONFIG_BOARDCTL_POWEROFF is only enabled in about half of the sim configurations:

$ find . -name defconfig | xargs grep -l BOARDCTL_POWEROFF | wc -
     30      30     846 -
$ find . -name defconfig | xargs grep -L BOARDCTL_POWEROFF | wc -
     21      21     562 -

Your prediction is correct.
I will then add the host_abort() you suggested and use it to implement the poweroff command as well.

@protobits
sim: only abort sim in assertion if in interrupt context/IDLE task
1869a6c 
This fixes the problem that an assertion in sim build aborted NuttX
even when the assertion was generated from userspace (in which case
simpy the task needs to exit). This required moving the relevant code
into the sim blob.
@xiaoxiang781216
Copy link
Contributor

@patacongo do you think the patch is good now?

@patacongo patacongo merged commit 1b8a690 into apache:master Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patacongo patacongo patacongo approved these changes

@xiaoxiang781216 xiaoxiang781216 xiaoxiang781216 approved these changes

Assignees
No one assigned
Labels
Type: Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@protobits @patacongo @xiaoxiang781216