Skip to content

Commit

Permalink
Make memory leak in xRealloc explicit
Browse files Browse the repository at this point in the history
There is an edge case in xRealloc upon failure to realloc memory
that manifests when the memory area pointed to by ptr contains pointers.
As the structure of ptr is opaque to xRealloc there's no chance for
properly releasing the memory for those indirect pointers.

The best we could do is release ptr itself, leaving behind an
indirect leak of those pointers inside the memory pointed to by ptr.
This memory leak is picked up by analyzers like deepcode and
GCC 14's -fanalyzer, which causes a lengthy, hard-to-parse diagnostic.
Leaving the memory allocated and failing on this code path is
properly ignored though (at least with GCC 14).

A better solution thus is to keep ptr untouched and instead leak the
whole block which avoids this diagnostic on indirectly leaked memory
which is free'd by bailing shortly after anyway.

This is not a fix, neither was the code we had before.
This commit mainly documents an edge case and tries to avoid
some hard-to-understand (but unavoidable) leak by making it
more blatantly obvious in the used tooling.
  • Loading branch information
BenBE committed Apr 4, 2024
1 parent ef6658c commit 62c2d82
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions XUtils.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,16 @@ void* xCalloc(size_t nmemb, size_t size) {

void* xRealloc(void* ptr, size_t size) {
assert(size > 0);
void* data = realloc(ptr, size); // deepcode ignore MemoryLeakOnRealloc: this goes to fail()
void* data = size ? realloc(ptr, size) : NULL; // deepcode ignore MemoryLeakOnRealloc: this goes to fail()
if (!data) {
free(ptr);
/* free'ing ptr here causes an indirect memory leak if pointers
* are held as part of an potential array referenced in ptr.
* In GCC 14 -fanalyzer recognizes this leak, but fails to
* ignore it given that this path ends in a noreturn function.
* Thus to avoid this confusing diagnostic we opt to leave
* that pointer alone instead.
*/
// free(ptr);
fail();
}
return data;
Expand Down

0 comments on commit 62c2d82

Please sign in to comment.