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/>
signature.asc
Description: PGP signature
