[Bug middle-end/54202] Overeager warning about freeing non-heap objects
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54202
Charles Blake changed:
What|Removed |Added
CC||charlechaud at gmail dot com
--- Comment #11 from Charles Blake ---
Here is another example (no external includes/etc.):
typedef long unsigned int size_t;
extern void *malloc (size_t __size) __attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__malloc__))
__attribute__ ((__alloc_size__ (1))) __attribute__
((__warn_unused_result__));
extern void free (void *__ptr) __attribute__ ((__nothrow__ , __leaf__));
typedef struct { long count, flags; } Foo;
Foo *G(Foo *x, long flags, int unused) {
if (x) { // originally a file descriptor
x->count = 0; flags = 0;
} else {
if (!(x = (Foo *)malloc(sizeof *x))) return (Foo *)0;
flags = 1;
}
x->flags = flags;
return x;
}
Foo *F(Foo *x, long flags) {
if (x) { // originally a pathname path
x->count = 0; flags = 0;
} else {
if (!(x = (Foo *)malloc(sizeof *x))) return (Foo *)0;
flags = 1;
}
x->flags = flags;
return G(x, flags, 123);
}
void release(Foo *x) {
if (x && x->flags) free(x);
}
int main(void) {
Foo x;
if (F(, 0))
release();
return 0;
}
In the above, variations due to compiler optimization levels make it even more
confusing: -O1 does not warn, -O2 does warn, -O3 again does not warn (on both
gcc-12.3 and gcc-13.2 on Gentoo Linux, anyway). O3 optimizes the whole main
away. clang never warns on this at any -O level, nor with clang --analyze.
What is particularly bad about this warning is that AFAICT there is no way to
massage the code to reliably silence it while also preserving the intended
effect. Now it's even on by default. This actively discourages an otherwise
clean arrangement in the C programming language when working with anyone else
who takes warns too seriously. Any such warning should have "may be" language
not language that sounds like the compiler has "proved an actuality".
Might the same argument apply to many warnings? Maybe. Two words is not so
bad, though. Other warns may have workarounds like annotations to silence them
reliably without sacrificing functionality. Maybe I'm missing one here?
[Bug middle-end/54202] Overeager warning about freeing non-heap objects
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54202 --- Comment #10 from Martin Sebor --- (In reply to Serdar Sanli from comment #9) > A simpler example not involving any globals, causing Wfree-nonheap-object > warning since GCC11 This is actually a bug in the example: it's invalid to decrement a pointer to the beginning of an object as done in Foo::allocate.
[Bug middle-end/54202] Overeager warning about freeing non-heap objects
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54202
Serdar Sanli changed:
What|Removed |Added
CC||mserdarsanli at gmail dot com
--- Comment #9 from Serdar Sanli ---
A simpler example not involving any globals, causing Wfree-nonheap-object
warning since GCC11
https://godbolt.org/z/MYn1zrjE4
===
struct Foo {
void allocate(int n);
void deallocate();
int *_data;
};
void Foo::allocate(int n) {
_data = new int[n] - 1;
}
void Foo::deallocate() {
delete[] (_data+1);
}
[Bug middle-end/54202] Overeager warning about freeing non-heap objects
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54202
Giuseppe D'Angelo changed:
What|Removed |Added
CC||dangelog at gmail dot com
--- Comment #8 from Giuseppe D'Angelo ---
The original testcase by Thiago still fails with GCC 11.
Unfortunately, GCC 11 decided to turn -Wfree-nonheap-object on by default (!),
resulting in false positives for Qt users (containers in Qt use this pattern).
For instance
https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qlinkedlist.h?h=5.15
struct QT_DEPRECATED_VERSION_5_15 QLinkedListData
{
QLinkedListData *n, *p;
QtPrivate::RefCount ref; // set to -1 for shared_null
int size;
uint sharable : 1;
Q_CORE_EXPORT static const QLinkedListData shared_null;
};
template
inline QLinkedList::~QLinkedList()
{
if (!d->ref.deref())
freeData(d); // never called on shared_null because of the check
}
template
void QLinkedList::freeData(QLinkedListData *x)
{
// ...
delete x; // -Wfree-nonheap-object here
}
[Bug middle-end/54202] Overeager warning about freeing non-heap objects
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54202
Martin Sebor changed:
What|Removed |Added
CC||msebor at gcc dot gnu.org
Component|c |middle-end
--- Comment #7 from Martin Sebor ---
All late warnings are susceptible to the same problem. The only way to tell
from the IL that the free call isn't unconditional is from the CFG. While that
could be used to issue either a "maybe" kind of a warning or a definitive one
per function, it would just turn most instances of these warnings into the
"maybe" kind. Which is how all warnings need to be viewed regardless.
What might help more than rewording every warning to say "this may be
undefined" is printing the chain of conditions that need to be satisfied in
order for the warning to trigger. The infrastructure to do this is all there,
it's just a matter of taking advantage of it: when a warning is issued, walk
the shortest path from the entry to the basic block with the problem statement
and print each conditional along the way. With that, we should end up with
output very close to that of the analyzer:
pr54202.c:17:9: warning: ‘free’ of ‘_null’ which points to memory not on
the heap [CWE-590] [-Wanalyzer-free-of-non-heap]
17 | free(d);
| ^~~
‘f’: events 1-3
|
| 16 | if (d->refcount == 0)
| |^
| ||
| |(1) following ‘true’ branch...
| 17 | free(d);
| | ~~~
| | |
| | (2) ...to here
| | (3) call to ‘free’ here
|
Removing basic block 5
f ()
{
int _2;
[local count: 1073741824]:
_2 = shared_null.refcount;
if (_2 == 0) <<< condition
goto ; [33.00%]
else
goto ; [67.00%]
[local count: 354334800]:
free (_null); [tail call] <<< conditional invalid call
[local count: 1073741824]:
return;
}
