Skip to content

arch/xtensa: add gdbstub support #16095

Closed
chirping78 wants to merge 6 commits into
apache:masterfrom
chirping78:gdbstub
Closed

arch/xtensa: add gdbstub support #16095
chirping78 wants to merge 6 commits into
apache:masterfrom
chirping78:gdbstub

Conversation

@chirping78

Copy link
Copy Markdown
Contributor

Summary

As follow up to #15907, add gdbstub support to xtensa arch.

Impact

No impact to users, only enhanced the development tools.

Testing

Please note that two USB-to-UART converters are used here.

One is the on-board USB-to-UART, which is used for program the flash, and nsh access;
another is an external USB-to-UART converter, connected to GPIO17/GPIO18, or U1TXD/U1RXD.

  • build and flash
./tools/configure.sh -E esp32s3-devkit:gdbstub
make -j flash ESPTOOL_PORT=/dev/ttyUSB0
  • nsh access as normal
minicom -D /dev/ttyUSB0 -b 115200
  • launch gdb to debug
xtensa-esp32s3-elf-gdb nuttx
(gdb) target remote /dev/ttyUSB1
(gdb) b cmd_ls
(gdb) c

Now you can run ls command in the nsh to trigger the breakpoint, and test these gdb functions

  1. Stopping and Continuing:
    b
    c
    n
    s
    ni
    si
    finish

  2. Examining the Stack:
    bt
    info frame
    info locals

  3. Examining Code:
    disass

  4. Examining Data:
    p
    x
    info reg

@github-actions github-actions Bot added Arch: xtensa Issues related to the Xtensa architecture Area: Drivers Drivers issues Area: OS Components OS Components issues Board: xtensa Size: M The size of the change in this PR is medium labels Mar 30, 2025
@nuttxpr

nuttxpr commented Mar 30, 2025

Copy link
Copy Markdown

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although some sections could be more thoroughly documented.

Strengths:

  • Clear Summary: The summary explains the "why," "what," and "how" of the change and links a related issue.
  • Limited Impact: The PR states a clear lack of impact on users, build process, hardware, documentation, security, and compatibility. This simplifies review.
  • Testing Provided: Testing instructions and example logs are included, which is crucial. The explanation of the dual UART setup is helpful.

Areas for Potential Improvement:

  • Impact (While "No Impact" is stated, justification could be stronger): While claiming "no impact" is valid, briefly explaining why there's no user impact (e.g., "gdbstub is a developer tool, not part of the end-user experience") strengthens the claim.
  • Testing (More detail could be helpful): While logs are provided, adding more specific examples of expected output for the gdb commands would be beneficial. For example, show the output of p some_variable or x &some_variable to demonstrate data examination. This helps reviewers validate the functionality. Also specify which build host was used.
  • Testing (Consider additional scenarios): Testing under load or with different configurations could further validate the robustness of the gdbstub implementation.

Overall: The PR is well-structured and provides the necessary information. Adding the minor improvements suggested above would make it even stronger.

@lupyuen

lupyuen commented Mar 30, 2025

Copy link
Copy Markdown
Member

Please remember to fill in the Commit Description. Thanks :-)

anjiahao1 and others added 2 commits March 31, 2025 11:01
Or the gdb on host machine cannot connect to target, the
command such as "target remote /dev/ttyUSB1" will timeout.

Signed-off-by: anjiahao <anjiahao@xiaomi.com>
After PR#14672, there is no use of XCHAL_SYSCALL_LEVEL.

Signed-off-by: chenxiaoyi <chenxiaoyi@xiaomi.com>
Comment thread arch/xtensa/src/common/xtensa_int_handlers.S
Comment thread arch/xtensa/src/common/xtensa_debug.c
Comment thread arch/xtensa/src/common/xtensa_debug.c Outdated
Comment thread arch/xtensa/src/common/xtensa_debug.c Outdated
Comment thread libs/libc/gdbstub/lib_gdbstub.c
@chirping78 chirping78 force-pushed the gdbstub branch 2 times, most recently from 3defe51 to b4ac459 Compare March 31, 2025 09:06

@jerpelea jerpelea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update commit titile for
b4ac459
to
boards/xtensa/esp32s3: add gdbstub config

@tmedicci

Copy link
Copy Markdown
Contributor

Hi @chirping78 ,

If you don't mind, can you please run make host_info and update the PR's description with it? Particularly, I'm interested on Toolchain versions used for testing it. You can borrow any sensitive information.

@chirping78

Copy link
Copy Markdown
Contributor Author

Hi @chirping78 ,

If you don't mind, can you please run make host_info and update the PR's description with it? Particularly, I'm interested on Toolchain versions used for testing it. You can borrow any sensitive information.

@tmedicci cannot run make host_info here, it output some errors:

$ make host_info
file sysinfo.h not exists
DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.
Traceback (most recent call last):
  File "/source/NuttX/nuttx/tools/host_info_dump.py", line 575, in <module>
    header = generate_header(args)
  File "/source/NuttX/nuttx/tools/host_info_dump.py", line 469, in generate_header
    vendor_output, vendor_build_output = get_vendor_info(info["NUTTX_CONFIG"])
  File "/source/NuttX/nuttx/tools/espressif/chip_info.py", line 358, in get_vendor_info
    info["ESPRESSIF_ESPTOOL"].split("-")[1]
IndexError: list index out of range
make: *** [tools/Unix.mk:644: host_info] Error 1

About the toolchain version, I have not updated it after last time you told me to use the same version as in CI's docker.


static const uint16_t g_reg_offs[] =
{
/* In the same order as gdb command "maintenance print remote-registers" */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it need to be changed?

Wouldn't it break #15141? Can you please double-check it?

@sdc-g (FYI)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tmedicci Thanks for your reminder!

Hi @chirping78

May I know the gdb version you are using?
Could you please try to verify based on coredump test case here. Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sdc-g the gdb version is 14.2_20240403.
I have also posted the host_info in the thread as requested by @tmedicci, you can check other tools version there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuNeo
Could you help check above question?
It seems that libs/libc/gdbstub/lib_gdbstub.c and sched/misc/coredump.c both are using g_tcbinfo.reg_off.p, but with different interpretation?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will break coredump if the register layout is different in GDB and coredump.
It does also happen on x86-64 platform. This can be solved by including more register info in g_reginfo(g_reg_offs).

What's worse is that different GDB expects different register layout. For example, xt-gdb and esp32s3-gdb have different register layout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will break coredump if the register layout is different in GDB and coredump. It does also happen on x86-64 platform. This can be solved by including more register info in g_reginfo(g_reg_offs).

What's worse is that different GDB expects different register layout. For example, xt-gdb and esp32s3-gdb have different register layout.

So for current issue, we can expand the struct g_reg_offs to same size as before, but in order with gdb remote register?

For xt-gdb and esp32s3-gdb, this may be resolved by chip selection CONFIG_ARCH_CHIP_XXX in future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or for current issue, use CONFIG_COREDUMP and CONFIG_SERIAL_GDBSTUB to define two different structure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see. The conflict between coredump and gdbstub can't be resolved simply.
This needs structure updating.
I will close this PR for now, and add back after the structure is updated.

@tmedicci

Copy link
Copy Markdown
Contributor

Hi @chirping78 ,
If you don't mind, can you please run make host_info and update the PR's description with it? Particularly, I'm interested on Toolchain versions used for testing it. You can borrow any sensitive information.

@tmedicci cannot run make host_info here, it output some errors:

$ make host_info
file sysinfo.h not exists
DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.
Traceback (most recent call last):
  File "/source/NuttX/nuttx/tools/host_info_dump.py", line 575, in <module>
    header = generate_header(args)
  File "/source/NuttX/nuttx/tools/host_info_dump.py", line 469, in generate_header
    vendor_output, vendor_build_output = get_vendor_info(info["NUTTX_CONFIG"])
  File "/source/NuttX/nuttx/tools/espressif/chip_info.py", line 358, in get_vendor_info
    info["ESPRESSIF_ESPTOOL"].split("-")[1]
IndexError: list index out of range
make: *** [tools/Unix.mk:644: host_info] Error 1

About the toolchain version, I have not updated it after last time you told me to use the same version as in CI's docker.

By the error message, it seems you're using Python 2.7. Can you double-check, please?

Comment thread libs/libc/gdbstub/lib_gdbstub.c Outdated
Comment thread arch/xtensa/src/common/xtensa_debug.c Outdated
Comment thread libs/libc/gdbstub/lib_gdbstub.c Outdated
Comment thread libs/libc/gdbstub/lib_gdbstub.c Outdated
@chirping78

Copy link
Copy Markdown
Contributor Author

By the error message, it seems you're using Python 2.7. Can you double-check, please?

@tmedicci pls check the host_info:

Host system OS:
  Ubuntu 20.04.6 LTS Linux xiaoyich-OptiPlex-7050 5.4.0-190-generic #210-Ubuntu SMP Fri Jul 5 17:03:38 UTC 2024 x86_64 x86_64

Host system PATH:
  /opt/espressif/esp_idf_tools/tools/xtensa-esp-elf-gdb/14.2_20240403/xtensa-esp-elf-gdb/bin
  /source/buildfarm/vela-5.0-test-tools/openocd-esp32/bin
  /source/buildfarm/vela-5.0-test-tools/qemu/bin
  /source/buildfarm/vela-5.0-test-tools/xtensa-esp32-elf/bin
  /source/buildfarm/vela-5.0-test-tools/xtensa-esp32s2-elf/bin
  /source/buildfarm/vela-5.0-test-tools/xtensa-esp32s3-elf/bin
  /home/xiaoyich/.vscode-server/cli/servers/Stable-f1a4fb101478ce6ec82fe9627c43efbf9e98c813/server/bin/remote-cli
  /home/xiaoyich/.local/bin
  /home/xiaoyich/bin
  /home/xiaoyich/.cargo/bin
  /usr/local/sbin
  /usr/local/bin
  /usr/sbin
  /usr/bin
  /sbin
  /bin
  /usr/games
  /usr/local/games
  /snap/bin
  /home/xiaoyich/bin/depot_tools
														
  /opt/android-studio/bin
  /home/xiaoyich/bin/depot_tools
														
  /opt/android-studio/bin


Espressif specific information:

Toolchain version:
  esp32: Bootloader image not found
  esp32s2: Bootloader image not found
  esp32s3: Bootloader image not found
  esp32c2: Bootloader image not found
  esp32c3: Bootloader image not found
  esp32c6: Bootloader image not found
  esp32h2: Bootloader image not found


Toolchain version:
  clang: clang version 10.0.0-4ubuntu1
  gcc: gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
  xtensa-esp32-elf-gcc: xtensa-esp32-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
  xtensa-esp32s2-elf-gcc: xtensa-esp32s2-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
  xtensa-esp32s3-elf-gcc: xtensa-esp32s3-elf-gcc (crosstool-NG esp-12.2.0_20230208) 12.2.0
  riscv32-esp-elf-gcc: Not found
  riscv64-unknown-elf-gcc: Not found


Esptool version:
  4.8.dev4

HAL version:
  Not found

CHIP ID:
  ESP32-S3 has no Chip ID. Reading MAC instead.

Flash ID:
  Manufacturer: c8
  Device: 4017


Security information:
  Flags: 0x00000000 (0b0)
  Key Purposes: (0 0 0 0 0 0 12)
  Chip ID: 9
  API Version: 0
  Secure Boot: Disabled
  Flash Encryption: Disabled
  SPI Boot Crypt Count (SPI_BOOT_CRYPT_CNT): 0x0


Flash status:
  0x0200

MAC address:
  24:ec:4a:09:7a:d8

It's found that some memory ranges such as IROM can't be read by bytes,
so change the memory reading to be 4-byte aligment.

Signed-off-by: chenxiaoyi <chenxiaoyi@xiaomi.com>
This will make gdb command 'g' get correct response.

Signed-off-by: chenxiaoyi <chenxiaoyi@xiaomi.com>
Add the single step breakpoint handling.

Signed-off-by: chenxiaoyi <chenxiaoyi@xiaomi.com>
build, flash, and run:

```
./tools/configure.sh -E esp32s3-devkit:gdbstub
make -j flash ESPTOOL_PORT=/dev/ttyUSB0
minicom -D /dev/ttyUSB0 -b 115200
```

launch gdb to debug:

```
xtensa-esp32s3-elf-gdb nuttx
(gdb) target remote /dev/ttyUSB1
(gdb) b cmd_ls
(gdb) c
```

Signed-off-by: chenxiaoyi <chenxiaoyi@xiaomi.com>
@chirping78

Copy link
Copy Markdown
Contributor Author

Currently the structure of g_reg_offs is too simple to accommodate two different requirements, coredump and gdbstub.
see: #16095 (comment)
So close this PR for now, and will resume after the structure is updated.

@chirping78 chirping78 closed this Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: xtensa Issues related to the Xtensa architecture Area: Drivers Drivers issues Area: OS Components OS Components issues Board: xtensa Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants