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

cpu/esp32: fix heap definition for ESP32-S2 and ESP32-S3 #19956

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Sep 29, 2023

Contribution description

For ESP32-S2 and ESP32-S3 the symbol _heap_end must not be used as _eheap for the newlibc malloc and function sbrk.

_heap_end is used by the ESP-IDF heap implementation esp-idf-heap and points to the highest possible address (0x40000000) that could be used for the heap in ESP-IDF. It doesn't point to the top address of the unused SRAM area that can be used in newlibc malloc and function sbrk. Instead, the origin and the length of dram0_0_seg must be used to calculate the end of the heap _eheap.

The problem only occurs for the newlibc malloc when the sbrk function is used but not for the ESP-IDF heap implementation esp_idf_heap.

Testing procedure

Use any ESP32-S2 or ESP32-S3 board and flash tests/sys/malloc, e.g.

CFLAGS='-DCHUNK_SIZE=16384' USEMODULE='stdio_uart' BOARD=esp32s3-pros3 make -j8 -C tests/sys/malloc flash

Without the PR the nm command will give the wrong address

nm -s tests/sys/malloc/bin/esp32s3-pros3/tests_malloc.elf | grep _eheap
40000000 A _eheap

The test will stuck, i.e. the allocation of memory stops when the top of unused SRAM is reached and the board restarts when the watchdog timer expires. With the PR it should work as expected

Help: Press s to start test, r to print it is ready
START
main(): This is RIOT! (Version: 2023.10-devel-309-g4669e)
calloc(zu, zu) = 0x10000000
CHUNK_SIZE: 16384
NUMBER_OF_TESTS: 3
Allocated 16384 Bytes at 0x3fc8c4b0, total 16384
...
Allocated 16384 Bytes at 0x3fcec6f0, total 409792
ESP-ROM:esp32s3-20210327
Build:Mar 27 2021
rst:0x7 (TG0WDT_SYS_RST),boot:0x8 (SPI_FAST_FLASH_BOOT)
Saved PC:0x403763e3

With this PR the nm command should give a address in unused SRAM address space

nm -s tests/sys/malloc/bin/esp32s3-pros3/tests_malloc.elf | grep _eheap
3fcca000 A _eheap

and the test should pass.

Issues/PRs references

For ESP32-S2 and ESP32-S3 the symbol `_heap_end` must not be used as `_eheap` for dynamic memory allocation, because it points to the highest possible address that could be used for the heap, but not to the top address of the unused SRAM area. Instead, the origin and length of `dram0_0_seg` must be used to calculate the end of the heap.
@github-actions github-actions bot added Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Sep 29, 2023
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 29, 2023
@gschorcht gschorcht requested a review from benpicco September 29, 2023 06:28
@gschorcht gschorcht added this to the Release 2023.10 milestone Sep 29, 2023
@riot-ci
Copy link

riot-ci commented Sep 29, 2023

Murdock results

✔️ PASSED

3a40e20 cpu/esp32: fix ld scripts for heap

Success Failures Total Runtime
7936 0 7937 14m:26s

Artifacts

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 29, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 149cee4 into RIOT-OS:master Sep 29, 2023
@gschorcht gschorcht deleted the cpu/esp32/fix_heap branch December 6, 2023 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants