feat(esp32): enable semihosting exit in QEMU#1549
Conversation
Summary of ChangesHello @luoliwoshang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces crucial enhancements for emulating ESP32 and ESP32-C3 targets in QEMU by enabling clean program exits via semihosting. It involves updating the underlying newlib library, adjusting QEMU launch parameters, and refining firmware image generation to correctly support both Xtensa and RISC-V architectures. Additionally, a new testing framework has been added to ensure the reliability and correctness of emulator interactions, providing a more robust development and testing environment for embedded applications. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enables semihosting for ESP32 (Xtensa) and ESP32-C3 (RISC-V) targets in QEMU, allowing programs to exit cleanly. This is achieved by updating the newlib build configurations, adding the -semihosting flag to QEMU commands, and introducing a new test framework to verify the emulator's behavior. The refactoring of esp32c3.json to inherit from a new esp32c3-basic.json is a good improvement for maintainability.
The PR title and summary currently focus on ESP32 (Xtensa). It would be beneficial to update them to also reflect the significant changes made for ESP32-C3 (RISC-V) to provide a more complete picture of the PR's scope.
I have a couple of minor suggestions for improvement in the new test files.
Code Review SummaryThis PR successfully enables ESP32 semihosting exit functionality for clean QEMU emulation. The implementation is solid with good test coverage. I've identified a few areas for improvement: Key Findings:
Priority Recommendations:
The changes are well-structured and the new test framework provides excellent coverage for embedded targets. Great work! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1549 +/- ##
==========================================
+ Coverage 91.35% 91.65% +0.29%
==========================================
Files 47 47
Lines 12681 13135 +454
==========================================
+ Hits 11585 12039 +454
Misses 906 906
Partials 190 190 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a43eb49 to
bc1014a
Compare
Switch ESP32 libcrt0 build flag from QEMU_SEMIHOSTING to QEMU_SEMIHOSTING_EXIT_ONLY so only _exit uses semihosting while regular syscall behavior (like _write) stays non-semihosting.
f6d39c9 to
e114e99
Compare
Upgrade newlib-esp32 source from esp-4.3.0_20250211-patch5 to esp-4.3.0_20250211-patch6, including URL/version/resource directory and related unit test expectations.
|
Good refactoring to generalize the ESP test script for both targets, and the Key concerns: (1) CI only installs |
fe61012 to
b669815
Compare
| echo "" | ||
| echo "=== Test 4: Verify QEMU output ===" | ||
|
|
||
| # Ignore emulator boot logs and validate the last non-empty line. | ||
| RUN_OUT=$(llgo run -a -target=esp32c3-basic -emulator . 2>&1) | ||
| LAST_LINE=$(printf "%s\n" "$RUN_OUT" | tr -d '\r' | awk 'NF{line=$0} END{print line}') | ||
| if [ "$LAST_LINE" = "Hello World" ]; then | ||
| echo "✓ PASS: QEMU output ends with Hello World" | ||
| else | ||
| echo "✗ FAIL: QEMU output mismatch" | ||
| echo "Last line: $LAST_LINE" | ||
| echo "" | ||
| echo "Full output:" | ||
| echo "$RUN_OUT" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
NOTE:verify at _demo/embed/test-esp-serial-startup.sh
b669815 to
ca4a824
Compare
Summary
This PR enables reliable QEMU exit for the existing
esp32target (Xtensa) by switching to newlib's exit-only semihosting path.Goal: keep normal ESP32 runtime behavior for non-exit syscalls, but let
llgo run -target=esp32 -emulatorterminate QEMU cleanly for test automation.Motivation
Before this change, ESP32 emulator runs could end via an illegal-instruction panic path without a deterministic semihosting exit signal, which made emulator-based comparison tests unstable.
At the same time, enabling full semihosting is not desired here because it can reroute regular I/O behavior. For ESP32 we only need semihosting on
_exit.What Changed
1) newlib integration for Xtensa (ESP32)
esp-4.3.0_20250211-patch5esp-4.3.0_20250211-patch6In LLGO's Xtensa newlib build config:
libgloss/xtensa/sim-call.S(semihost call entry support).-DQEMU_SEMIHOSTING_EXIT_ONLYfor Xtensa libgloss/syscalls build.libgloss/libnosys/_exit.cin the build.-DGETREENT_PROVIDEDin Xtensa newlib compile flags so newlib's own reent path is used instead of relying on external stubs.2) ESP32 emulator command
targets/esp32.jsonto run QEMU with semihosting enabled:-semihostingtoqemu-system-xtensaarguments.3) CI and smoke test improvements
test_esp32c3_startup.shwith a generalized script:_demo/embed/test-esp-serial-startup.sh__libc_init_array,.init_arrayin.rodata, BIN segment check)qemu-system-riscv32qemu-system-xtensaNewlib Behavior Clarification
With patch6 +
QEMU_SEMIHOSTING_EXIT_ONLYin Xtensa:_exituses semihosting to notify QEMU and terminate emulator runs cleanly.So this is intentionally not full semihosting; it is exit-only semihosting.
Validation
Local checks performed:
llgo run -a -target=esp32 -emulator .bash _demo/embed/test-esp-serial-startup.shIf local cache still points to an older newlib bundle, clear and rebuild once:
Scope / Compatibility
esp32target.