When compiler optimisations cause memcpy undefined behaviour: Fix a crash trying to save an empty AudioStream by hpvb · Pull Request #100422 · godotengine/godot · GitHub
Posted by jpluimers on 2025/11/25
The libc C standard library memcpy function is very fast, but because of that also very dumb and with a list of circumstances where its behaviour is undefined (a great opportunity to introduce vulnerabilities in your code). The specs of those are in [Wayback/Archive] memcpy, memcpy_s – cppreference.com, including:
The behavior is undefined if either
destorsrcis an invalid or null pointer.
With C compilers becoming smarter and smarter, their optimisations can hide the fact that you call memcpy with parameters causing undefined behaviour.
This is a nice example of that: [Wayback/Archive] Fix a crash trying to save an empty AudioStream by hpvb · Pull Request #100422 · godotengine/godot · GitHub
The change is relatively simple, but hardly shows why the change is there. Only one line got changed in [Wayback/Archive] Fix a crash trying to save an empty AudioStream by hpvb · Pull Request #100422 · godotengine/godot · GitHub
Vector AudioStreamWAV::get_data() const {
Vector pv;
- if (!data.is_empty()) {
+ if (data_bytes) {
pv.resize(data_bytes);
memcpy(pv.ptrw(), data.ptr() + DATA_PAD, data_bytes);
}
Note that the same change was also proposed in a separate pull request, with a shorter explanation [Wayback/Archive] Fix undefined memcpy in AudioStreamWAV::get_data() by jman168 · Pull Request #97790 · godotengine/godot · GitHub:
Fixes #97720. In the
get_data()function of theAudioStreamWAV,memcpyis called on an undefined pointer due to assuming there is data after some padding bytes (which isn’t true for an empty buffer). This was fixed by checking if there is actually any data to copy instead of checking if the buffer (including padding) is not empty (which should never be true).
It has a slightly different single line change which underneath compiles to the same actual code [Wayback/Archive] Fix undefined memcpy in AudioStreamWAV::get_data() by jman168 · Pull Request #97790 · godotengine/godot · GitHub
Vector AudioStreamWAV::get_data() const {
Vector pv;
- if (!data.is_empty()) {
+ if (data_bytes != 0) {
pv.resize(data_bytes);
memcpy(pv.ptrw(), data.ptr() + DATA_PAD, data_bytes);
}
Both fix the issue [Wayback/Archive] The AudioStreamWav unit test seg faults · Issue #97720 · godotengine/godot.
Via [Wayback/Archive] HP van Braam: “Today I learned two important lessons. One of the lessons is that sometimes changing 1 line of code can take 5 hours. The other lesson is enclosed in the following PR: …” – Mastodon
--jeroen
The Wiert Corner - irregular stream of stuff 
Leave a comment