On Wed, Nov 29, 2017 at 08:56:44AM -0700, Martin Sebor wrote: > On 11/29/2017 01:30 AM, Jakub Jelinek wrote: > > On Tue, Nov 28, 2017 at 09:11:00PM -0700, Martin Sebor wrote: > > > On 11/27/2017 02:22 AM, Dominik Inführ wrote: > > > > Thanks for all the reviews! I’ve revised the patch, the > > > > operator_delete_flag is now stored in tree_decl_with_vis (there already > > > > seem to be some FUNCTION_DECL-flags in there). I’ve also added the > > > > option -fallocation-dce to disable this optimization. It bootstraps and > > > > no regressions on aarch64 and x86_64. > > > > > > > It's great to be able to eliminate pairs of these calls. For > > > unpaired calls, though, I think it would be even more useful to > > > also issue a warning. Otherwise the elimination will mask bugs > > > > ?? I hope you're only talking about allocation where the returned > > pointer can't leak elsewhere, doing allocation in one function > > (e.g. constructor, or whatever other function) and deallocation in some > > other one is so common such a warning would be not just useless, but > > harmful with almost all occurrences being false positives. > > > > Warning on malloc/standard operator new or malloc/realloc-like function > > when the return pointer can't escape the current function is reasonable. > > Yes, warn for leaks, or for calls to delete/free with no matching > new/malloc (when they can be detected). > > From the test case included in the patch, warn on the first two > of the following three functions: > > +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C > @@ -0,0 +1,65 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-cddce-details" } */ > + > +#include <stdlib.h> > + > +void > +new_without_use() { > + int *x = new int; > +} > + > +void > +new_array_without_use() { > + int *x = new int[5]; > +} > + > +void > +new_primitive() { > + int *x = new int; > + delete x; > +} > > An obvious extension to such a checker would then be to also detect > possible invalid deallocations, as in: > > void f (unsigned n) > { > void *p = n < 256 ? alloca (n) : malloc (n); > // ... > free (p); > } > > David Malcolm was working on something like that earlier this year > so he might have some thoughts on this as well.
I also sent https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00491.html a while back, sorry I haven't gotten to improving it yet though its gcc 9 stuff at this point I guess. thanks Trev > > Martin