On Wed, Nov 29, 2017 at 8:15 AM, David Malcolm <dmalc...@redhat.com> wrote:
> On Wed, 2017-11-29 at 08:56 -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 was experimenting with optimizing away matching malloc/free pairs,
> moving the allocation to either the stack, or to a thread-local
> obstack, under certain conditions, or to hoist allocations out of
> loops.
>
> I didn't get any significant wins, but much of this was due to my lack
> of experience with the middle-end, and being drawn back to front-
> end/diagnostic improvements.
>
> Issues I ran into included:
>
> * wrapper functions like "Perl_malloc" which require LTO for the
> optimization to be able to see the allocations
>
> * 435.gromacs has this "interesting" function:
It is still in the upstream sources too:
https://sourcecodebrowser.com/gromacs/3.3.1/smalloc_8h.html#a776f070172af6850dda887c7358fa630
Thanks,
Andrew Pinski
>
> unsigned maxavail(void)
> {
> char *ptr;
> unsigned low,high,size;
>
> low=0;
> high=256e6;
> while ((high-low) > 4) {
> size=(high+low)/2;
> if ((ptr=malloc((size_t)size))==NULL)
> high=size;
> else {
> free(ptr);
> low=size;
> }
> }
> return low;
> }
>
> i.e. a loop which attempts a binary search of malloc calls to try to
> find a threshold at which they fail.
>
>
> Dave
