Skip to content

AP_Common: effectively wrap malloc to zero memory in SITL under Cygwin - #31036

Merged
tridge merged 2 commits into
ArduPilot:masterfrom
tpwrules:pr/cygwin-zero
Sep 5, 2025
Merged

AP_Common: effectively wrap malloc to zero memory in SITL under Cygwin#31036
tridge merged 2 commits into
ArduPilot:masterfrom
tpwrules:pr/cygwin-zero

Conversation

@tpwrules

@tpwrules tpwrules commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

This currently does not work, in particular resulting in scripting raising an internal error on stop because it doesn't think all the memory is freed because the allocated counter doesn't start out at zero because the zero on allocation behavior is broken.

Cygwin has some support for overriding malloc (https://cygwin.com/faq/faq.html#faq.programming.own-malloc), but when this is attempted some logic in the runtime detects this case then forwards all allocator calls (free/calloc/etc.) to the user provided allocation functions. If only malloc is overridden, the other functions just call themselves recursively until the stack overflows. There is no supported way to use the original Cygwin allocator while overriding malloc, but we do not want to include our own allocator just for Cygwin.

Fortunately, in some sense, there is an unsupported way, and our goal can be achieved by overwriting an internal pointer with our malloc implementation after the Cygwin runtime logic checks and believes that no overwrite has occurred, thereby never enabling the redirection and allowing non-malloc functions to still work. Our version then calls calloc to do the allocation and zeroing. Note that the old wrap of _malloc_r was faulty as that symbol is no longer used by newlib; it's just #defined to malloc these days.

Tested that at least the scripting issue is fixed by failing to replicate it, plus tracing execution with gdb to confirm that our wrapper function is executed and does its job.

The correct solution is to not make ArduPilot call malloc at all. We still cannot safely change the semantics to make it returned zeroed memory. This is quite the hack!

Fixes #29563 , despite my protestations. Also fixes #30688 .

Comment thread libraries/AP_Common/c++.cpp Outdated
Comment thread libraries/AP_Common/c++.cpp Outdated
This currently does not work, in particular resulting in scripting
raising an internal error on stop because it doesn't think all the
memory is freed because the allocated counter doesn't start out at zero
because the zero on allocation behavior is broken.

Cygwin has some support for overriding `malloc`
(https://cygwin.com/faq/faq.html#faq.programming.own-malloc), but when
this is attempted some logic in the runtime detects this case then
forwards all allocator calls (`free`/`calloc`/etc.) to the user provided
allocation functions. If only `malloc` is overridden, the other
functions just call themselves recursively until the stack overflows.
There is no supported way to use the original Cygwin allocator while
overriding `malloc`, but we do not want to include our own allocator
just for Cygwin.

Fortunately, in some sense, there is an unsupported way, and our goal
can be achieved by overwriting an internal pointer with our `malloc`
implementation after the Cygwin runtime logic checks and believes that
no overwrite has occurred, thereby never enabling the redirection and
allowing non-`malloc` functions to still work. Our version then calls
`calloc` to do the allocation and zeroing. Note that the old wrap of
`_malloc_r` was faulty as that symbol is no longer used by newlib; it's
just `#define`d to `malloc` these days.

Tested that at least the scripting issue is fixed by failing to
replicate it, plus tracing execution with `gdb` to confirm that our
wrapper function is executed and does its job.

The correct solution is to not make ArduPilot call `malloc` at all. We
still cannot safely change the semantics to make it returned zeroed
memory. This is quite the hack!
This is now accomplished at runtime, and the wrap of `_malloc_r` never
worked properly anyway.

@robertlong13 robertlong13 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.

Tested the artifacts with Mission Planner. No issues.

@tridge tridge 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.

Thanks!

@tridge
tridge merged commit a6308cb into ArduPilot:master Sep 5, 2025
106 checks passed
@github-project-automation github-project-automation Bot moved this to Pending in 4.6 Backports Sep 5, 2025
@tpwrules
tpwrules deleted the pr/cygwin-zero branch September 5, 2025 08:23
@robertlong13 robertlong13 moved this from Pending to 4.6.3-beta1 in 4.6 Backports Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 4.6.3 / 4.6.3-beta1

Development

Successfully merging this pull request may close these issues.

Scripting crash when do stop and restarting in sitl Internal error from multi-heap allocation

3 participants