On Thu, 2023-03-30 at 00:50 +0200, Benjamin Priour wrote: > Hi David, > > I've been playing around with sm-malloc on C++ samples.
Note that the analyzer doesn't properly work yet on C++; see: https://gcc.gnu.org/bugzilla/showdependencytree.cgi?id=97110 I'm hoping to address much of this in GCC 14. Other notes inline below... > I've noticed double delete don't go under -Wanalyzer-double-free but > within > -Wanalyzer-use-after-free scope. > > With the reproducer ... > > struct A {}; > > > > int main() { > > A* a = new A(); > > delete a; > > delete a; > > return 0; > > } > > ... the analyzer diagnoses: > build/gcc/xg++ -Bbuild/gcc -S -fanalyzer > -Wanalyzer-mismatching-deallocation -Wanalyzer-use-after-free > -Wanalyzer-double-free -Wanalyzer-malloc-leak double_delete_test.cpp > double_delete_test.cpp: In function ‘int main()’: > double_delete_test.cpp:7:1: warning: use after ‘delete’ of ‘a’ [CWE- > 416] > [-Wanalyzer-use-after-free] > 7 | } > | ^ > ‘int main()’: events 1-8 > | > | 3 | A* a = new A(); > | | ^ > | | | > | | (1) allocated here > | | (2) assuming ‘operator new(8)’ is non- > NULL > | 4 | delete a; > | | ~~~~~~~~ > | | | | > | | | (4) ...to here > | | | (5) deleted here > | | (3) following ‘true’ branch... > | 5 | delete a; > | | ~~~~~~~~ > | | | | > | | | (7) ...to here > | | (6) following ‘true’ branch... > | 6 | return 0; > | 7 | } > | | ~ > | | | > | | (8) use after ‘delete’ of ‘a’; deleted at (5) > | > double_delete_test.cpp:3:18: warning: dereference of possibly-NULL > ‘operator new(8)’ [CWE-690] [-Wanalyzer-possible-null-dereference] > 3 | A* a = new A(); > | ^ > ‘int main()’: events 1-2 > | > | 3 | A* a = new A(); > | | ^ > | | | > | | (1) this call could return NULL > | | (2) ‘operator new(8)’ could be NULL: > unchecked value from (1) > | > > > Debugging read out two things: > - During second call of 'on_deallocator_call' on delete, the state > would be > 'stop' instead of the expected 'freed' > - The call to set_next_state transitions malloc instead of delete > from > 'nonnull' to 'freed'. > I'm still trying to come up with why these two behaviors happens. > > By the way, in the first call to 'on_deallocator_call' on delete, the > state > is set to 'nonnull', > which conforms to C++ behavior for new. However, a > -Wanalyzer-possible-null-dereference is emitted at the expression > 'new'. > I haven't yet figured out why, but I'm looking into it. I'm guessing that the CFG and thus the supergraph contains edges for handling exceptions, and the analyzer currently doesn't know anything about these, and mishandled them: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97111 You might want to have a look at the supergraph (via -fdump-analyzer- supergraph); I recently added more debugging notes here: https://gcc.gnu.org/onlinedocs/gccint/Debugging-the-Analyzer.html If that's the case, then -fno-exceptions might affect the behavior. > > > Another observation was in the distinction between delete and free in > the > case of mixing them. > With 'a' being allocated with malloc: > > A* a = (A*) malloc(sizeof(A)); > > free(a); > > delete a; // -Wanalyzer-use-after-free, OK, might expect warning > > for > double free though ? > > but with allocation through new and inversion of free/delete > > A* a = new A(); > > delete a; > > free(a); // No diagnostics at all from the analyzer, got > -Wmismatched-new-delete from the front-end though. > > I believe this might come from a similar issue as above, but I still > have > to investigate on that front. > > I just noticed another unexpected behavior. Let's consider > > struct A {int x; int y;}; > > void* foo() { A* a = (A*) __builtin_malloc(sizeof(A)); return a; } > > int main() { > > A* a2 = (A*) __builtin_malloc(sizeof(A)); > > foo(); > > return 0; > > } > > Then a -Wanalyzer-malloc-leak is correctly ensued for allocation in > foo(), > but none is emitted for the leak in main(), although I checked the > exploded > graph it is correctly marked as unchecked(free). > > Should I file those on bugzilla ? We already know that "C++ doesn't work yet". Minimal examples of problems with C++ support might be worth filing, to isolate specific issues. If you do, please add them to the tracker bug (by adding "analyzer-c++" to the "Blocks" field of the new bug(s)) > > > Also I have taken interest in PR106388 - Support for use-after-move > in > -fanalyzer -. > The prerequisite PR106386 - Reuse libstdc++ assertions - would also > be of > great help in extending the support of -Wanalyzer-out-of-bounds, > as we could detect out-of-bounds on vectors through > __glibcxx_requires_subscript used in the definition of operator[]. There's a bunch of other C++-enablement work (as per the tracker bug above) that I think would need fixing before PR106388 is reasonable to tackle. > > I already thought about a few ideas to implement that, but I'll > develop > them more and try to come up with some proof of concept before > sending them > to you. > Hopefully by tomorrow I'll update on this, I'll preferably hop to bed > now > haha. > Do you think this could make a suitable GSoC subject ? I think working on the C++ enablement prerequisites in the analyzer would make more sense. I'd planned to do this myself for GCC 14, but there are plenty of other tasks I could work on if you want to tackle C++ support as a GSoC project for GCC 14. A good C++ project might be: enable enough C++ support in the analyzer for it to be able to analyze itself. This could be quite a large, difficult project, though it sidesteps having to support exception- handling, since we build ourselves with exception-handling disabled. Hope this is helpful Dave
