AP_MultiHeap: initialize only if heap allocation succeeded

[srmainwaring]

Ensure new_heap is not null before accessing, and initialise current heap usage.

Motivation

• On ESP32 S3 the heap allocator is failing when attempting to create a new Lua state.
• The variable current_heap_usage is not initialised to zero.

[tpwrules]

What exactly is failing? That malloc call to initialize the heap?

Moving the magic init is correct (poor review on my part, whoops) and probably needs a backport.

But Theoretically™ malloc returns zero-initialized memory on ArduPilot so the clear of the used size is not necessary. Have you seen that be non-zero?

[srmainwaring]

But Theoretically™ malloc returns zero-initialized memory on ArduPilot so the clear of the used size is not necessary. Have you seen that be non-zero?

Yes. The non-zero is the reason the allocator is failing on ESP32. The first pass through the allocator it fails at

ardupilot/libraries/AP_MultiHeap/MultiHeap_malloc.cpp

Line 80 in 5f8a655

if (heapp->current_heap_usage + size > heapp->max_heap_size) {

Can post a full stack trace if evidence required - but I've seen this fail on ESP32 classic and the S3.

[tpwrules]

The failure matches my experience, and it makes sense that the heap malloc itself is not failing because then it should crash with a null pointer dereference.

But that's bad, it means the wrapping is not working. Let me see if there's an obvious cause.

[srmainwaring]

I don't see anything in https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Common/c%2B%2B.cpp that would wrap malloc for ESP32 to ensure memory is initialised to zero. Where is that meant to occur for ESP32?

[tpwrules]

It is supposed to be done here but I suspect that doesn't get passed through to the linker because that is done by ESP-IDF:

ardupilot/Tools/ardupilotwaf/boards.py

Line 1064 in 5f8a655

env.LINKFLAGS += ['-Wl,--wrap,malloc']

Adding target_link_options(${elf_file} PRIVATE "-Wl,--wrap,malloc") after the idf_build_executable in CMakeLists.txt fixes it, or at least I can see __wrap_malloc in the binary now.

We really need to convince the team to switch to a different function name, this has been repeatedly shown to not be safe.

[tpwrules]

I have adjusted to include this fix, please test that it works properly.

[srmainwaring]

I have adjusted to include this fix, please test that it works properly.

Thanks - looks good. Re-tested a hello world script on ESP32 S3.

[davidbuzz]

this PR should perhaps have also had the prefix AP_HAL_ESP32 now that its been determined to be a missing --wrap on malloc in the CMake linker that was exposing the issue on esp32, but its a great find, and brings the esp32 builds closer to how chibios does its pre-zero-ing so its a good-thing.