[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 Eric Gallager changed: What|Removed |Added CC||rep.dot.nop at gmail dot com --- Comment #6 from Eric Gallager --- Recent patchset for fixing problems in GCC based on this same principle: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613116.html
[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 Eric Gallager changed: What|Removed |Added CC||dmalcolm at gcc dot gnu.org --- Comment #5 from Eric Gallager --- One thing to consider now that GCC has a static analyzer is that putting the kind of useless "if" in question before a "free" can make the analyzer shut up about possible double-free warnings, so if this were to become its own diagnostic, there might be separate diagnostics that prompt users to do opposite things...
[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 Andrew Pinski changed: What|Removed |Added Severity|normal |enhancement
[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 jim at meyering dot net changed: What|Removed |Added CC||jim at meyering dot net --- Comment #4 from jim at meyering dot net --- Thanks for considering the addition. IME, the vast majority (probably "all I've seen") of such "useless" if stmts have been attempts to avoid what used to be UB/segfault on old systems, or simply due to people not realizing that free(NULL) is now known to be ok on all reasonable portability targets.
[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 Eric Gallager changed: What|Removed |Added CC||list+gcc-bugzilla@meyering. ||net --- Comment #3 from Eric Gallager --- cc-ing Jim Meyering, the author of the original gnulib script, to see if he has any input
[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Last reconfirmed||2017-08-18
CC||msebor at gcc dot gnu.org
Ever confirmed|0 |1
--- Comment #2 from Martin Sebor ---
I think a warning like this might be useful, although I probably wouldn't want
to enable under -Wall or -Wextra. (GCC could also use branch probabilities to
decide whether or not to warn.)
That said, I would expect GCC to get rid of the tests with
-fdelete-null-pointer-checks. As it is, it eliminates the call to free when
the pointer is known to be null but not the test itself (see below). That
seems quite suboptimal to me since in the absence of any other information a
pointer being passed to free is far more likely to be non-null than not. Clang
and ICC both optimize the function below into an unconditional jump to free.
So while I think implementing the optimization is the better solution here, the
warning could also help improve the efficiency of the emitted code in its
absence.
$ cat b.c && gcc -O2 -S -Wall -Wextra -fdump-tree-optimized=/dev/stdout b.c
void free (void*);
void g (void *p)
{
if (p)// test retained
free (p);
else
free (p); // eliminated
}
;; Function g (g, funcdef_no=0, decl_uid=1817, cgraph_uid=0, symbol_order=0)
Removing basic block 5
g (void * p)
{
[100.00%] [count: INV]:
if (p_2(D) != 0B)
goto ; [53.47%] [count: INV]
else
goto ; [46.53%] [count: INV]
[53.47%] [count: INV]:
free (p_2(D)); [tail call]
[100.00%] [count: INV]:
return;
}
[Bug c/80528] reimplement gnulib's "useless-if-before-free" script as a compiler warning
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #1 from Jakub Jelinek --- Well, if the user knows that the pointer is usually NULL, then if (p) free (p); is a significant optimization over free (p). So as code size optimization, removing it is fine, but otherwise, it isn't at all clear.
