os/mm/mm_heap : use DEBUGASSERT_INFO to print heap related info#7120
os/mm/mm_heap : use DEBUGASSERT_INFO to print heap related info#7120rish-sg wants to merge 1 commit intoSamsung:masterfrom
Conversation
rish-sg
changed the title
ewoodev
left a comment
ewoodev
left a comment
There was a problem hiding this comment.
please check the CI build error
It seems this error is related to DEBUGASSERT_INFO not defined for NXFUSE_HOST_BUILD This requires similar fix of adding a NULL definition for above Macro like below commit: |
sunghan-chang
left a comment
sunghan-chang
left a comment
There was a problem hiding this comment.
Need to check ci failures
|
|
||
| DEBUGASSERT(heap->mm_holder == my_pid); | ||
| DEBUGASSERT_INFO(heap->mm_holder == my_pid, | ||
| "heap=0x%p, holder=%d, pid=%d, cnt=%d", |
There was a problem hiding this comment.
It's difficult to understand the message without checking this code.
How about
"heap=0x%p, mm sem holder pid=%d, running pid=%d, mm sem holder cnt=%d"?
There was a problem hiding this comment.
The size (CONFIG_STDIO_BUFFER_SIZE) of the buffer which stores this formatted string is different for different configs
and majorly it is 64 bytes
and it is 256 bytes for loadable configs of rtl8730e
So, if we would keep a long formatted string, it could be truncated in the output unless the size of the buffer is not modified.
for example the output of the string you suggested in
flat_dev_ddr (rtl8730e):
===========================================================
Assertion details
===========================================================
Assertion failed CPU0 at file: hello_main.c line 100 task: hello pid: 26
print_assert_detail: heap=0x0x6017547c, mm_sem_holder_pid=123, running_pid=26, mm_se
print_assert_detail: Assert location (PC) : 0x0e030283
loadable_ext_ddr (rtl8730e):
===========================================================
Assertion details
===========================================================
Assertion failed CPU0 at file: hello_main.c line 100 task: hello pid: 29
print_assert_detail: heap=0x0x63a93f0c, mm_sem_holder_pid=123, running_pid=29, mm_sem_holder_cnt=2
print_assert_detail: Assert location (PC) : 0x0e272ab3
I have modified the string by the way would like to know your thoughts.
rish-sg
force-pushed
the
mm_sem
branch
5 times, most recently
from
d413e27 to
8c24a67
Compare
This patch replaces DEBUGASSERT with DEBUGASSERT_INFO in 'mm_givesemaphore' function to print heap, heap->mm_holder, my_pid and heap->mm_counts_held as debugging information. For this change we had to make a syscall as DEBUGASSERT_INFO prints the string (given to it as an argument) via a global char array called 'assert_info_str' in print_assert_detail() called from up_assert(). So this string is defined in kernel space and was being accessed from userspace. Therefore, we made a syscall called copy_string_from_userspace which does this task. Sample logs: security level: 0 =========================================================== Assertion details =========================================================== Assertion failed CPU0 at file: hello_main.c line 100 task: hello pid: 29 print_assert_detail: heap=0x63a93f0c, mm_sem_holder_pid=123, running_pid=29, mm_sem_holder_cnt=2 print_assert_detail: Assert location (PC) : 0x0e272ab3 check_assert_location: Code asserted in normal thread! =========================================================== Signed-off-by: ris.singh <ris.singh@samsung.com>
| "clock_settime", "time.h", "", "int", "clockid_t", "const struct timespec*" | ||
| "close", "unistd.h", "CONFIG_NSOCKET_DESCRIPTORS > 0 || CONFIG_NFILE_DESCRIPTORS > 0", "int", "int" | ||
| "closedir", "dirent.h", "CONFIG_NFILE_DESCRIPTORS > 0", "int", "FAR DIR*" | ||
| "copy_string_from_userspace", "assert.h", "", "void", "const char*", "..." |
There was a problem hiding this comment.
we don't prefer to change Syscall.
it will break compatibility.
Can you find other method?
There was a problem hiding this comment.
@rish-sg How about using a global variable and giving the address through the up_userspace.c ? Then, kernel can access the address directly and can print. We must not call syscall API in the assert. Assert means that SW is in abnormal state. We can't get guarantee of other module working.
4 participants
This patch replaces DEBUGASSERT with DEBUGASSERT_INFO in 'mm_givesemaphore'
function to print heap, heap->mm_holder, my_pid and heap->mm_counts_held as debugging information.
For this change we had to make a syscall as DEBUGASSERT_INFO
prints the string (given to it as an argument) via a global
char array called 'assert_info_str' in print_assert_detail()
called from up_assert(). So this string is defined in kernel
space and was being accessed from userspace. Therefore, we
made a syscall called copy_string_from_userspace which does this task.
Sample logs:
Signed-off-by: ris.singh ris.singh@samsung.com