https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114594
Bug ID: 114594
Summary: Issues seen with -Wanalyzer-malloc-leak on
htop/XUtils.c: String_split
Product: gcc
Version: 14.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: analyzer
Assignee: dmalcolm at gcc dot gnu.org
Reporter: dmalcolm at gcc dot gnu.org
CC: BenBE at geshi dot org
Target Milestone: ---
Created attachment 57881
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57881&action=edit
Reduced reproducer
User "BenBE2" on #gcc on IRC noted some issues with the attached file; see also
at https://godbolt.org/z/vKbhqMq4T
The analyzer reports a leak, arguably falsely:
<source>: In function 'xRealloc':
<source>:32:7: warning: leak of '<unknown>' [CWE-401] [-Wanalyzer-malloc-leak]
32 | free(ptr);
| ^~~~~~~~~
'String_split': events 1-2
|
| 38 | char** String_split(const char* s, char sep, size_t* n) {
| | ^~~~~~~~~~~~
| | |
| | (1) entry to 'String_split'
| 39 | const size_t rate = 10;
| 40 | char** out = xCalloc(rate, sizeof(char*));
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) calling 'xCalloc' from 'String_split'
|
+--> 'xCalloc': event 3
|
| 15 | void* xCalloc(size_t nmemb, size_t size) {
| | ^~~~~~~
| | |
| | (3) entry to 'xCalloc'
|
'xCalloc': event 4
|
| 16 | assert(nmemb > 0);
| | ^~~~~~
| | |
| | (4) following 'true' branch (when 'nmemb != 0')...
|
'xCalloc': event 5
|
| 17 | assert(size > 0);
| | ^~~~~~
| | |
| | (5) ...to here
|
'xCalloc': event 6
|
| 17 | assert(size > 0);
| | ^~~~~~
| | |
| | (6) following 'true' branch (when 'size != 0')...
|
'xCalloc': events 7-11
|
| 18 | if (SIZE_MAX / nmemb < size) {
| | ~ ^
| | | |
| | | (7) ...to here
| | (8) following 'false' branch...
|......
| 21 | void* data = calloc(nmemb, size);
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (9) ...to here
| 22 | if (!data) {
| | ~
| | |
| | (10) following 'false' branch (when 'data' is
non-NULL)...
|......
| 25 | return data;
| | ~~~~
| | |
| | (11) ...to here
|
<------+
|
'String_split': events 12-13
|
| 40 | char** out = xCalloc(rate, sizeof(char*));
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (12) returning to 'String_split' from 'xCalloc'
|......
| 44 | while ((where = strchr(s, sep)) != NULL) {
| | ~~~~~~~~~~~~~~
| | |
| | (13) when 'strchr' returns non-NULL
|
'String_split': events 14-16
|
| 44 | while ((where = strchr(s, sep)) != NULL) {
| | ^
| | |
| | (14) following 'true' branch
(when 'where' is non-NULL)...
| 45 | size_t size = (size_t)(where - s);
| | ~~~~~~~~~~~
| | |
| | (15) ...to here
| 46 | out[ctr] = xStrndup(s, size);
| | ~~~~~~~~~~~~~~~~~
| | |
| | (16) calling 'xStrndup' from 'String_split'
|
+--> 'xStrndup': events 17-21
|
| 67 | char* xStrndup(const char* str, size_t len) {
| | ^~~~~~~~
| | |
| | (17) entry to 'xStrndup'
| 68 | char* data = strndup(str, len);
| | ~~~~~~~~~~~~~~~~~
| | |
| | (18) allocated here
| 69 | if (!data) {
| | ~
| | |
| | (19) assuming 'data' is non-NULL
| | (20) following 'false' branch (when 'data' is
non-NULL)...
|......
| 72 | return data;
| | ~~~~
| | |
| | (21) ...to here
|
<------+
|
'String_split': events 22-25
|
| 44 | while ((where = strchr(s, sep)) != NULL) {
| | ~~~~~~~~~~~~~~
| | |
| | (25) when 'strchr' returns NULL
| 45 | size_t size = (size_t)(where - s);
| 46 | out[ctr] = xStrndup(s, size);
| | ^~~~~~~~~~~~~~~~~
| | |
| | (22) returning to 'String_split' from 'xStrndup'
| 47 | ctr++;
| 48 | if (ctr == blocks) {
| | ~
| | |
| | (23) following 'false' branch (when 'ctr != blocks')...
|......
| 52 | s += size + 1;
| | ~~
| | |
| | (24) ...to here
|
'String_split': events 26-30
|
| 44 | while ((where = strchr(s, sep)) != NULL) {
| | ^
| | |
| | (26) following 'false' branch
(when 'where' is NULL)...
|......
| 54 | if (s[0] != '\0') {
| | ~~~~~
| | | |
| | | (27) ...to here
| | (28) following 'false' branch...
|......
| 58 | out = xRealloc(out, sizeof(char*) * (ctr + 1));
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | |
| | | (29) ...to here
| | (30) calling 'xRealloc' from 'String_split'
|
+--> 'xRealloc': event 31
|
| 28 | void* xRealloc(void* ptr, size_t size) {
| | ^~~~~~~~
| | |
| | (31) entry to 'xRealloc'
|
'xRealloc': event 32
|
| 29 | assert(size > 0);
| | ^~~~~~
| | |
| | (32) following 'true' branch (when 'size != 0')...
|
'xRealloc': events 33-37
|
| 30 | void* data = realloc(ptr, size);
| | ^~~~~~~~~~~~~~~~~~
| | |
| | (33) ...to here
| | (34) when 'realloc' fails
| 31 | if (!data) {
| | ~
| | |
| | (35) following 'true' branch (when 'data' is NULL)...
| 32 | free(ptr);
| | ~~~~~~~~~
| | |
| | (36) ...to here
| | (37) '<unknown>' leaks here; was allocated at (18)
|
If I'm reading things right, at the free at (37) it's freeing a buffer
containing the last pointer to the allocation at (18). But this is on an
error-handling path which is about to call "fail", which the analyzer knows
calls "abort".
So there are at least two issues here:
(a) difficult to read: yet another '<unknown>' in the report, making it hard to
decipher what the leaked thing being reported is.
Probably instead of '<unknown>' it should say 'out[0]'.
Ideally could show a "path of last liveness", showing that:
"ptr" aliases "out", out points-to (buffer allocated at (9), I think), which
points-to (buffer allocated at (18))
hence the free of ptr loses the last paths of liveness of the buffer allocated
at (18).
(b) Maybe we shouldn't bother reporting leaks on paths that are about to
unconditionally exit/abort, similar to the logic for leaks at the end of
"main".
