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

nuttxgdb utils module update #14912

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

XuNeo
Copy link
Contributor

@XuNeo XuNeo commented Nov 23, 2024

Note: Please adhere to Contributing Guidelines.

Summary

This PR mainly update the utils module and minor bug fixes.

  1. Added Backtrace to better handle converting addresses to symbols.
  2. Enhancement to offset_of and container_of, now support string as argumment
  3. added helper sizeof, both str and gdb.Type are accepted.
  4. Added enum function to map C enum to python Enum class
    in C:
    enum color_e {
        RED = 1,
        GREEN = 2,
    };
    in python:
    COLOR = utils.enum("enum color_e", "COLOR")
    print(COLOR.GREEN.value) # --> 2
    RED = COLOR(1)
  1. Added ArrayIterator to access array elements
  2. Added crash log parsing for addr2line command, now you can use addr2line -f crash.log or addr2line -f crash.log -p 123 for specified PID.
  3. Moved profiling commands to profile.py

Impact

No.

Testing

I tested addr2line on qemu arm64.

(gdb) addr2line -f crash.log
Address              Symbol                           Source

Backtrace of 0
0x402cf900           <sched_backtrace+88>             /home/neo/projects/nuttx/nuttx/sched/sched/sched_backtrace.c:106
0x402cd854           <sched_dumpstack+68>             /home/neo/projects/nuttx/nuttx/libs/libc/sched/sched_dumpstack.c:71
0x402c0510           <dump_running_task+420>          /home/neo/projects/nuttx/nuttx/sched/misc/assert.c:667
0x402c0978           <_assert+544>                    /home/neo/projects/nuttx/nuttx/sched/misc/assert.c:718
0x402b35bc           <uart_check_special+132>         /home/neo/projects/nuttx/nuttx/drivers/serial/serial.c:2272
0x402b39fc           <uart_recvchars+520>             /home/neo/projects/nuttx/nuttx/drivers/serial/serial_io.c:282
0x402b1978           <pl011_irq_handler+148>          /home/neo/projects/nuttx/nuttx/drivers/serial/uart_pl011.c:838
0x402bfb70           <irq_dispatch+60>                /home/neo/projects/nuttx/nuttx/sched/irq/irq_dispatch.c:145
0x402b14e0           <arm64_doirq+96>                 /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_doirq.c:75
0x402b0908           <arm64_decodeirq+80>             /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_gicv3.c:808
0x402afd60           <arm64_irq_handler+32>           /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_vectors.S:227
0x402cbcc0           <up_idle+8>                      /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_idle.c:63
0x402af6c4           <arm64_boot_primary_c_routine+16> /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_boot.c:212
0x402cf900           <sched_backtrace+88>             /home/neo/projects/nuttx/nuttx/sched/sched/sched_backtrace.c:106
0x402cd854           <sched_dumpstack+68>             /home/neo/projects/nuttx/nuttx/libs/libc/sched/sched_dumpstack.c:71
0x402c0360           <dump_backtrace+32>              /home/neo/projects/nuttx/nuttx/sched/misc/assert.c:452
0x402c11d4           <nxsched_foreach+160>            /home/neo/projects/nuttx/nuttx/sched/sched/sched_foreach.c:69
0x402c0ad0           <_assert+888>                    /home/neo/projects/nuttx/nuttx/sched/misc/assert.c:790
0x402b35bc           <uart_check_special+132>         /home/neo/projects/nuttx/nuttx/drivers/serial/serial.c:2272
0x402b39fc           <uart_recvchars+520>             /home/neo/projects/nuttx/nuttx/drivers/serial/serial_io.c:282
0x402b1978           <pl011_irq_handler+148>          /home/neo/projects/nuttx/nuttx/drivers/serial/uart_pl011.c:838
0x402bfb70           <irq_dispatch+60>                /home/neo/projects/nuttx/nuttx/sched/irq/irq_dispatch.c:145
0x402b14e0           <arm64_doirq+96>                 /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_doirq.c:75
0x402b0908           <arm64_decodeirq+80>             /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_gicv3.c:808
0x402afd60           <arm64_irq_handler+32>           /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_vectors.S:227
0x402cbcc0           <up_idle+8>                      /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_idle.c:63
0x402af6c4           <arm64_boot_primary_c_routine+16> /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_boot.c:212

Backtrace of 1
0x402cbcc0           <up_idle+8>                      /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_idle.c:63
0x402cbcc0           <up_idle+8>                      /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_idle.c:63

Backtrace of 2
0x402cbcc0           <up_idle+8>                      /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_idle.c:63

Backtrace of 3
0x402cbcc0           <up_idle+8>                      /home/neo/projects/nuttx/nuttx/arch/arm64/src/common/arm64_idle.c:63
(gdb)

XuNeo and others added 19 commits November 23, 2024 16:50
Use the file hash instead to avoid file name conflicts

Signed-off-by: xuxingliang <[email protected]>
Make sure file name is surrounded by \"

Signed-off-by: xuxingliang <[email protected]>
Use json module to save macro info to json file and load directly. It can save 2seconds for x4b projects to load plugin

Signed-off-by: xuxingliang <[email protected]>
After we introduced NxDQueue and NxSQueue, we're using them like `NxDQueue(g_active_connections, "struct socket_conn_s", "node")` and leads to `utils.container_of(ptr, str, str)`, so maybe we need to allow str input for `utils.container_of`

Signed-off-by: Zhe Weng <[email protected]>
We're using `ptr.cast(get_long_type())` a week ago to get the pointer's value, it's alright, but `ptr.address` is not, the `ptr.address` will return **the address of the pointer**.

These values are the address of first element in the queue:
    - `int(g_sigfreeaction["head"])`
    - `g_sigfreeaction["head"].cast(get_long_type())`
    - `g_sigfreeaction["head"].dereference().address`
But:
    `int(g_sigfreeaction["head"].address)` is the address of the "head" member, which equals to the address of `g_sigfreeaction`

It's happening in NxSQueue:
    g_sigfreeaction = gdb.parse_and_eval("g_sigfreeaction")
    print(["%x" % (node) for node in NxSQueue(g_sigfreeaction)])
    print(["%x" % (node) for node in NxSQueue(g_sigfreeaction, "sigactq_t", "flink")])
Without this patch:
    ['f3c0aa10', 'f3c0aa2c', 'f3c0aa48']
    ['55db90a0', 'f3c0aa10', 'f3c0aa2c']
With this patch:
    ['f3c0aa10', 'f3c0aa2c', 'f3c0aa48']
    ['f3c0aa10', 'f3c0aa2c', 'f3c0aa48']

Signed-off-by: Zhe Weng <[email protected]>
Use array iterator where possible.

Signed-off-by: xuxingliang <[email protected]>
Now crash log can be directly pass to addr2line tool to get all backtraces.

E.g.
(gdb) addr2line -f crash.log -p 485
Address              Symbol                           Source

Backtrace of 485
0x402cc0ac           <up_switch_context+84>           /home/work/ssd1/workspace/MiRTOS-X4b-Stable-Build/nuttx/include/arch/syscall.h:179
0x40291276           <nxsig_timedwait+386>            signal/sig_timedwait.c:365
0x4028fc7e           <nxsig_nanosleep+106>            signal/sig_nanosleep.c:141
0x4028fdba           <clock_nanosleep+26>             signal/sig_nanosleep.c:333
0x402c3736           <usleep+62>                      unistd/lib_usleep.c:108
0x415018c0           <cs2p2p_mSecSleep+24>            Src/PPPP_Common.c:1139
0x414fabde           <cs2p2p_Run_send_DRW+258>        Src/PPPP_API.c:5577
0x414fac62           <cs2p2p_PPPP_thread_send_DRW+18> Src/PPPP_API.c:5616
0x40446f62           <pthread_startup+10>             pthread/pthread_create.c:59
0x41094f4a           <pthread_start+106>              pthread/pthread_create.c:139

Signed-off-by: xuxingliang <[email protected]>
1. Support any type of value as pointer address in container_of
2. Support string as type in offset_of
3. Make sure type is a pointer in container_of, and not a pointer in
   offset_of

Signed-off-by: xuxingliang <[email protected]>
Usage:
(gdb) py NX_INITSTATE = utils.enum("enum nx_initstate_e")
(gdb) py print(list(NX_INITSTATE))
[<NX_INITSTATE_E.POWERUP: 0>, <NX_INITSTATE_E.BOOT: 1>, <NX_INITSTATE_E.TASKLISTS: 2>, <NX_INITSTATE_E.MEMORY: 3>, <NX_INITSTATE_E.HARDWARE: 4>, <NX_INITSTATE_E.OSREADY: 5>, <NX_INITSTATE_E.IDLELOOP: 6>, <NX_INITSTATE_E.PANIC: 7>]
(gdb) py print(NX_INITSTATE.POWERUP)
NX_INITSTATE_E.POWERUP
(gdb) py print(NX_INITSTATE.POWERUP.value)
0
(gdb) py print(NX_INITSTATE(1))

Signed-off-by: xuxingliang <[email protected]>
Signed-off-by: Xu Xingliang <[email protected]>
Add simple time command to test time cost of a command.
Usage:
(gdb) time memleak
...
(gdb) Time elapsed: 1.23456s

Signed-off-by: xuxingliang <[email protected]>
GDB provides four “standard” register names sp, pc, fp and ps. Those can be used in most of the cases.

See https://sourceware.org/gdb/current/onlinedocs/gdb.html/Registers.html

Signed-off-by: xuxingliang <[email protected]>
Use a class Bactrace to support convert address to backtrace, format to string, accessing element and iterate though them.
Adjust usage in fs and addr2line to make full use of it.

Signed-off-by: xuxingliang <[email protected]>
GDB is slow to look up symbols, cache the result to speed up commands like memdump etc.

Signed-off-by: xuxingliang <[email protected]>
Let GDB to read from ELF for readonly data.
Disable this feature by nxgcore --no-trust-readonly.

Signed-off-by: xuxingliang <[email protected]>
@github-actions github-actions bot added Area: Tooling Size: L The size of the change in this PR is large labels Nov 23, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 23, 2024

[Experimental Bot, please feedback here]

This PR does not fully meet the NuttX requirements. While it provides a good overview of the changes, it lacks crucial details required for proper review and integration.

Here's a breakdown of what's missing and how to improve it:

Summary:

  • Vague Description: "Update the utils module and minor bug fixes" is too general. Each change listed (Backtrace, offset_of, container_of, sizeof, enum, ArrayIterator, crash log parsing, profiling commands move) deserves its own brief explanation of why it was changed (bug fix, performance improvement, new functionality). For example, why was Backtrace added? What problem did it solve? What was inefficient about the old offset_of and container_of?
  • Missing Context: The enum example is good, but others lack context. How are ArrayIterator, sizeof (str and gdb.Type versions), and the enhanced offset_of/container_of used? Provide brief examples.
  • Missing Issue References: If any of these changes address existing NuttX issues, link them explicitly.

Impact:

  • Incorrect "No": The impact is definitely not "No." These are significant changes to utility functions and debugging tools. This section needs to be carefully filled out.
    • User Impact: Will users need to change their code to use the new utilities? Does the crash log parsing change how users interact with crash logs?
    • Build Impact: Do these changes affect build times or dependencies?
    • Hardware Impact: While the core utils may not have direct hardware impact, the crash log parsing and addr2line changes could. Does this affect how debugging information is generated or processed for different architectures?
    • Documentation Impact: Absolutely requires documentation updates. The new utilities and features need to be documented. Specify what documentation needs to be added/updated.
    • Security Impact: Consider potential security implications. Does the crash log parsing handle sensitive information? Do the other utilities introduce any vulnerabilities? Even if the answer is "NO," explicitly state it and briefly justify it.
    • Compatibility Impact: Do these changes break compatibility with existing NuttX versions or tools?

Testing:

  • Insufficient Testing: Testing only addr2line on qemu arm64 is not enough. Every change needs to be tested. Provide test cases and results for each new utility and enhancement: Backtrace, offset_of, container_of, sizeof, enum, ArrayIterator, and the profiling command move.
  • Missing "Before" Logs: You included "after" logs for addr2line, but where are the "before" logs to demonstrate the change in behavior?
  • Incomplete Build Host/Target Information: Specify the exact OS version, compiler version (including minor version), QEMU version, and NuttX configuration used for testing. This level of detail is essential for reproducing the tests.

In short: This PR needs substantial revision before it can be considered for merging. Provide more detail and justification for each change, thoroughly assess the impact, and include comprehensive testing results for all modifications. Follow the template meticulously and address every point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Tooling Size: L The size of the change in this PR is large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants