public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/114594] New: Issues seen with -Wanalyzer-malloc-leak on htop/XUtils.c: String_split
@ 2024-04-04 21:39 dmalcolm at gcc dot gnu.org
2024-04-04 21:51 ` [Bug analyzer/114594] " dmalcolm at gcc dot gnu.org
0 siblings, 1 reply; 2+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2024-04-04 21:39 UTC (permalink / raw)
To: gcc-bugs
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".
^ permalink raw reply [flat|nested] 2+ messages in thread
* [Bug analyzer/114594] Issues seen with -Wanalyzer-malloc-leak on htop/XUtils.c: String_split
2024-04-04 21:39 [Bug analyzer/114594] New: Issues seen with -Wanalyzer-malloc-leak on htop/XUtils.c: String_split dmalcolm at gcc dot gnu.org
@ 2024-04-04 21:51 ` dmalcolm at gcc dot gnu.org
0 siblings, 0 replies; 2+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2024-04-04 21:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114594
--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
The "leak" was fixed in htop by
https://github.com/htop-dev/htop/commit/62c2d820add3dadea7569af051d2afd804f08432
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-04 21:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-04 21:39 [Bug analyzer/114594] New: Issues seen with -Wanalyzer-malloc-leak on htop/XUtils.c: String_split dmalcolm at gcc dot gnu.org
2024-04-04 21:51 ` [Bug analyzer/114594] " dmalcolm at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).