On 14 June 2021 07:56:38 CEST, Bernhard Reutner-Fischer <rep.dot....@gmail.com>
wrote:
>On 14 June 2021 01:45:36 CEST, Jeff Law via Gcc-patches
><gcc-patches@gcc.gnu.org> wrote:
>>
>>
>>On 6/2/2021 3:40 PM, Martin Sebor via Gcc-patches wrote:
>>> The two forms of placement operator new defined in <new> return
>their
>>> pointer argument and may not be displaced by user-defined functions.
>>> But because they are ordinary (not built-in) functions this property
>>> isn't reflected in their declarations alone, and there's no user-
>>> level attribute to annotate them with. When they are inlined
>>> the property is transparent in the IL but when they are not (without
>>> inlining such as -O0), calls to the operators appear in the IL and
>>> cause -Wmismatched-new-delete to try to match them with the
>functions
>>> called to deallocate memory. When the pointer to the memory was
>>> obtained from a function that matches the deallocator but not
>>> the placement new, the warning falsely triggers.
>>>
>>> The attached patch solves this by detecting calls to placement new
>>> and treating them the same as those to other pass-through calls
>(such
>>> as memset). In addition, it also teaches -Wfree-nonheap-object
>about
>>> placement delete, for a similar reason as above. Finally, it also
>>> adds a test for attribute fn spec indicating a function returns its
>>> argument. It's not necessary for the fix (I had initially though
>>> placement new might have the attribute) but it seems appropriate
>>> to check.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> gcc-100876.diff
>>>
>>> PR c++/100876 - -Wmismatched-new-delete should understand placement
>>new when it's not inlined
>>>
>>> gcc/ChangeLog:
>>>
>>> PR c++/100876
>>> * builtins.c (gimple_call_return_array): Check for attribute fn
>>spec.
>>> Handle calls to placement new.
>>> (ndecl_dealloc_argno): Avoid placement delete.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR c++/100876
>>> * g++.dg/warn/Wmismatched-new-delete-4.C: New test.
>>> * g++.dg/warn/Wmismatched-new-delete-5.C: New test.
>>> * g++.dg/warn/Wstringop-overflow-7.C: New test.
>>> * g++.dg/warn/Wfree-nonheap-object-6.C: New test.
>>> * g++.dg/analyzer/placement-new.C: Prune out expected warning.
>>>
>>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>>> index af1fe49bb48..fb0717a0248 100644
>>> --- a/gcc/builtins.c
>>> +++ b/gcc/builtins.c
>>> @@ -5159,11 +5159,43 @@ static tree
>>> gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>>> range_query *rvals)
>>> {
>>> - if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
>>> - || gimple_call_num_args (stmt) < 1)
Err, the if was removed, so right.
thanks,
>>> + {
>>> + /* Check for attribute fn spec to see if the function returns
>>one
>>> + of its arguments. */
>>> + attr_fnspec fnspec = gimple_call_fnspec (as_a <gcall *>(stmt));
>>> + unsigned int argno;
>>> + if (fnspec.returns_arg (&argno))
>>> + {
>>> + offrng[0] = offrng[1] = 0;
>>> + return gimple_call_arg (stmt, argno);
>>> + }
>>> + }
>>> +
>>> + if (gimple_call_num_args (stmt) < 1)
>>> return NULL_TREE;
>>Nit. You've got an unnecessary {} at the outer level of this hunk.
>
>if (unsigned int = 1 && fnspec.returns_arg (&argno))
>
>doesn't look too appealing though, I'd leave the curly braces, no?
>
>cheers,
>>
>>OK with the nit fixed.
>>
>>THanks,
>>Jeff
