'defer' (n3199) concerns
Alejandro Colomar
alx@kernel.org
Tue Jan 21 15:10:53 GMT 2025
Hi all,
After some private discussion with JeanHeyd, I'd like to *partially*
withdraw my concerns against 'defer', as long as it ends up being as
simple as [[gnu::cleanup()]] (or reasonably similar).
The -fanalyzer is currently not smart enough to warn about strsep(3),
but that's a known bug, and will hopefully be fixed in time for defer.
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118500>.
With an inline definition of strsep(3), the -fanalyzer is indeed able to
warn, both with normal code, and with [[gnu::cleanup()]]. See below.
However, and this is a big however, with [[gnu::cleanup()]], it's a bit
less safe, since we need -O3 to trigger (-O2 is not enough), while the
diagnostic triggers at any optinization level for normal code. So, I'm
still a bit skeptic about it, but it could be worse.
alx@devuan:~/tmp/gcc$ cat malloc.c
#include <stdlib.h>
#include <string.h>
int foo(const void *);
char *my_strsep(char **sp, const char *delim);
void freep(void *p);
void
f(void)
{
[[gnu::cleanup(freep)]] char *p = NULL;
p = strdup("asd;:qwe");
my_strsep(&p, ":");
return;
}
void
g(void)
{
char *p;
p = strdup("qwe:;asd");
my_strsep(&p, ";");
free(p);
return;
}
char *my_strsep(char **sp, const char *delim)
{
char *s;
s = *sp;
if (s == NULL)
return NULL;
*sp = strpbrk(s, delim);
return s;
}
void
freep(void *p)
{
free(*(void **) p);
}
Compiled with -O2 or lower, only the normal function triggers a
diagnostic:
alx@devuan:~/tmp/gcc$ gcc-15 -Wall -Wextra -fanalyzer -O2 -S malloc.c
malloc.c: In function ‘my_strsep’:
malloc.c:39:13: warning: leak of ‘s’ [CWE-401] [-Wanalyzer-malloc-leak]
39 | *sp = strpbrk(s, delim);
| ~~~~^~~~~~~~~~~~~~~~~~~
‘g’: events 1-3
│
│ 20 | g(void)
│ | ^
│ | |
│ | (1) entry to ‘g’
│......
│ 24 | p = strdup("qwe:;asd");
│ | ~~~~~~~~~~~~~~~~~~
│ | |
│ | (2) allocated here
│ 25 | my_strsep(&p, ";");
│ | ~~~~~~~~~~~~~~~~~~
│ | |
│ | (3) calling ‘my_strsep’ from ‘g’
│
└──> ‘my_strsep’: events 4-7
│
│ 31 | char *my_strsep(char **sp, const char *delim)
│ | ^~~~~~~~~
│ | |
│ | (4) entry to ‘my_strsep’
│......
│ 36 | if (s == NULL)
│ | ~
│ | |
│ | (5) assuming ‘s’ is non-NULL
│ | (6) following ‘false’ branch (when ‘s’ is non-NULL)... ─>─┐
│ | │
│......
│ | │
│ |┌─────────────────────────────────────────────────────────────────────┘
│ 39 |│ *sp = strpbrk(s, delim);
│ |│ ~~~~~~~~~~~~~~~~~
│ |│ |
│ |└─────────────>(7) ...to here
│
‘my_strsep’: event 8
│
│ 39 | *sp = strpbrk(s, delim);
│ | ~~~~^~~~~~~~~~~~~~~~~~~
│ | |
│ | (8) ⚠️ ‘s’ leaks here; was allocated at (2)
│
At -O3, both trigger a diagnostic:
alx@devuan:~/tmp/gcc$ gcc-15 -Wall -Wextra -fanalyzer -O3 -S malloc.c
malloc.c: In function ‘f’:
malloc.c:16:9: warning: leak of ‘<unknown>’ [CWE-401] [-Wanalyzer-malloc-leak]
16 | return;
| ^~~~~~
‘f’: events 1-3
│
│ 9 | f(void)
│ | ^
│ | |
│ | (1) entry to ‘f’
│......
│ 13 | p = strdup("asd;:qwe");
│ | ~~~~~~~~~~~~~~~~~~
│ | |
│ | (2) allocated here
│ 14 | my_strsep(&p, ":");
│ | ~
│ | |
│ | (3) inlined call to ‘my_strsep’ from ‘f’
│
└──> ‘my_strsep’: events 4-7
│
│ 36 | if (s == NULL)
│ | ^
│ | |
│ | (4) assuming ‘strdup("asd;:qwe")’ is non-NULL
│ | (5) following ‘false’ branch... ─>─┐
│ | │
│......
│ | │
│ |┌──────────────────────────────────────────────┘
│ 39 |│ *sp = strpbrk(s, delim);
│ |│ ~~~~~~~~~~~~~~~~~
│ |│ |
│ |└─────────────>(6) ...to here
│ | (7) when ‘__builtin_strchr’ returns non-NULL
│
<──────┘
│
‘f’: event 8
│
│ 16 | return;
│ | ^~~~~~
│ | |
│ | (8) ⚠️ ‘<unknown>’ leaks here; was allocated at (2)
│
malloc.c: In function ‘g’:
malloc.c:28:9: warning: leak of ‘<unknown>’ [CWE-401] [-Wanalyzer-malloc-leak]
28 | return;
| ^~~~~~
‘g’: events 1-3
│
│ 20 | g(void)
│ | ^
│ | |
│ | (1) entry to ‘g’
│......
│ 24 | p = strdup("qwe:;asd");
│ | ~~~~~~~~~~~~~~~~~~
│ | |
│ | (2) allocated here
│ 25 | my_strsep(&p, ";");
│ | ~
│ | |
│ | (3) inlined call to ‘my_strsep’ from ‘g’
│
└──> ‘my_strsep’: events 4-7
│
│ 36 | if (s == NULL)
│ | ^
│ | |
│ | (4) assuming ‘strdup("qwe:;asd")’ is non-NULL
│ | (5) following ‘false’ branch... ─>─┐
│ | │
│......
│ | │
│ |┌──────────────────────────────────────────────┘
│ 39 |│ *sp = strpbrk(s, delim);
│ |│ ~~~~~~~~~~~~~~~~~
│ |│ |
│ |└─────────────>(6) ...to here
│ | (7) when ‘__builtin_strchr’ returns non-NULL
│
<──────┘
│
‘g’: event 8
│
│ 28 | return;
│ | ^~~~~~
│ | |
│ | (8) ⚠️ ‘<unknown>’ leaks here; was allocated at (2)
│
In function ‘my_strsep’,
inlined from ‘g’ at malloc.c:25:2:
malloc.c:39:15: warning: leak of ‘strdup("qwe:;asd")’ [CWE-401] [-Wanalyzer-malloc-leak]
39 | *sp = strpbrk(s, delim);
| ^~~~~~~~~~~~~~~~~
‘g’: events 1-3
│
│ 20 | g(void)
│ | ^
│ | |
│ | (1) entry to ‘g’
│......
│ 24 | p = strdup("qwe:;asd");
│ | ~~~~~~~~~~~~~~~~~~
│ | |
│ | (2) allocated here
│ 25 | my_strsep(&p, ";");
│ | ~
│ | |
│ | (3) inlined call to ‘my_strsep’ from ‘g’
│
└──> ‘my_strsep’: events 4-8
│
│ 36 | if (s == NULL)
│ | ^
│ | |
│ | (4) assuming ‘strdup("qwe:;asd")’ is non-NULL
│ | (5) following ‘false’ branch... ─>─┐
│ | │
│......
│ | │
│ |┌──────────────────────────────────────────────┘
│ 39 |│ *sp = strpbrk(s, delim);
│ |│ ~~~~~~~~~~~~~~~~~
│ |│ |
│ |└─────────────>(6) ...to here
│ | (7) when ‘__builtin_strchr’ returns NULL
│ | (8) ⚠️ ‘strdup("qwe:;asd")’ leaks here; was allocated at (2)
│
In function ‘my_strsep’,
inlined from ‘f’ at malloc.c:14:2:
malloc.c:39:15: warning: leak of ‘strdup("asd;:qwe")’ [CWE-401] [-Wanalyzer-malloc-leak]
39 | *sp = strpbrk(s, delim);
| ^~~~~~~~~~~~~~~~~
‘f’: events 1-3
│
│ 9 | f(void)
│ | ^
│ | |
│ | (1) entry to ‘f’
│......
│ 13 | p = strdup("asd;:qwe");
│ | ~~~~~~~~~~~~~~~~~~
│ | |
│ | (2) allocated here
│ 14 | my_strsep(&p, ":");
│ | ~
│ | |
│ | (3) inlined call to ‘my_strsep’ from ‘f’
│
└──> ‘my_strsep’: events 4-8
│
│ 36 | if (s == NULL)
│ | ^
│ | |
│ | (4) assuming ‘strdup("asd;:qwe")’ is non-NULL
│ | (5) following ‘false’ branch... ─>─┐
│ | │
│......
│ | │
│ |┌──────────────────────────────────────────────┘
│ 39 |│ *sp = strpbrk(s, delim);
│ |│ ~~~~~~~~~~~~~~~~~
│ |│ |
│ |└─────────────>(6) ...to here
│ | (7) when ‘__builtin_strchr’ returns NULL
│ | (8) ⚠️ ‘strdup("asd;:qwe")’ leaks here; was allocated at (2)
│
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20250121/e12cd0ea/attachment.sig>
More information about the Gcc
mailing list