[PATCH] teach compute_objsize about placement new (PR 100876)

Bernhard Reutner-Fischer rep.dot.nop@gmail.com
Mon Jun 14 06:04:54 GMT 2021


On 14 June 2021 07:56:38 CEST, Bernhard Reutner-Fischer <rep.dot.nop@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



More information about the Gcc-patches mailing list