You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "jlaitine (via GitHub)" <gi...@apache.org> on 2024/03/19 11:07:54 UTC
[PR] Fix user task exception handling [nuttx]
jlaitine opened a new pull request, #11944:
URL: https://github.com/apache/nuttx/pull/11944
## Summary
If an user process causes an illegal exception, we want to just kill the user process, not cause system panic. Add this functionality for the riscv_exception.
The group_killchildren is modified in a way that if the parent task is terminated forcefully, it also terminates all the children the same way; i.e. not via signal. This allows terminating all the children also from within an exception.
Also fix the _assert printout. Currently the assert prints out the information about the task to which we are returning from exception. This is not necessarily the same as the one that causes it.
## Impact
Enable user-side crash on RISC-V platforms, e.g. illegal memory accesses w.o. crashing the whole system.
## Testing
Tested on MPFS RISC-V platform, in CONFIG_BUILD_KERNEL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1533361706
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
Yes, it seems that updating the g_running_task was left out when doing a context switch in syscall for risc-v. I made another PR of this: #11961
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1532298092
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
Maybe, riscv forget to set g_running_task to this_task at the end of exception handle. You could search the place under arch/arm which change g_running_task and compare with riscv and find the difference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1532059701
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
There may be something that I don't quite undertand, but I changed this just because in case of any crash due to exception, I am getting dump of some irrelevant task, and not the one that causes the crash:
#define running_task() \
(up_interrupt_context() ? g_running_tasks[this_cpu()] : this_task())
g_running_tasks[this_cpu()] points to the next running task, and this_task points to the interrupted (current) task.
In case of e.g. page fault, the current code will (potentially) dump out information on some other task, and not the one that caused the fault....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1531980390
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
when assert happen in the interrupt handler, this_task will point to the next running task, not the interrupted task, but mast people expect to dump the later in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #11944:
URL: https://github.com/apache/nuttx/pull/11944
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1532059701
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
There may be something that I don't quite understand, but I changed this just because in case of any crash due to exception, I am getting dump of some irrelevant task, and not the one that causes the crash:
#define running_task() \
(up_interrupt_context() ? g_running_tasks[this_cpu()] : this_task())
g_running_tasks[this_cpu()] points to the next running task, and this_task points to the interrupted (current) task.
In case of e.g. page fault, the current code will (potentially) dump out information on some other task, and not the one that caused the fault....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1532158398
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
> There may be something that I don't quite understand, but I changed this just because in case of any crash due to exception, I am getting dump of some irrelevant task, and not the one that causes the crash:
>
> #define running_task() (up_interrupt_context() ? g_running_tasks[this_cpu()] : this_task())
>
> g_running_tasks[this_cpu()] points to the next running task, and this_task points to the interrupted (current) task.
>
> In case of e.g. page fault, the current code will (potentially) dump out information on some other task, and not the one that caused the fault....
no, g_running_tasks point to the interrupted task, this_task may point to the next running task. For example, if the irq handler wake up a new thread, this_task will point to it immediately. g_running_tasks is added to fix this problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1532060975
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
Added the "#ifdef CONFIG_ARCH_KERNEL_STACK" which was missing to make flat builds compile.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix user task exception handling [nuttx]
Posted by "jlaitine (via GitHub)" <gi...@apache.org>.
jlaitine commented on code in PR #11944:
URL: https://github.com/apache/nuttx/pull/11944#discussion_r1532248181
##########
sched/misc/assert.c:
##########
@@ -508,7 +508,7 @@ static void dump_deadlock(void)
void _assert(FAR const char *filename, int linenum,
FAR const char *msg, FAR void *regs)
{
- FAR struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = this_task();
Review Comment:
Ir clearly doesn't work as you explained, at least not for risc-v / kernel mode, where assertions always dump out wrong task information at exception.
I can, however remove this patch from this PR and just keep it in our own own branches! It probably didn't belong to this PR anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org