This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Handle new operators with no arguments in DCE.


On 8/5/19 1:57 PM, Marc Glisse wrote:
> On Mon, 5 Aug 2019, Martin Liška wrote:
> 
>> On 8/5/19 9:07 AM, Marc Glisse wrote:
>>> On Mon, 5 Aug 2019, Martin Liška wrote:
>>>
>>>> I'm sending fix for the ICE. The issue is that we can end up
>>>> with a ctor without an argument (when not being used).
>>>
>>> Ah, I didn't realize that after cloning and drastically changing the signature it would still count as operator new/delete. Is getting down to 0 arguments the only bad thing that can happen? Can't we have an operator delete (void*, void*) where the first argument gets optimized out and we end up optimizing as if the second argument was actually the memory being released? Should we do some sanity-checking when propagating the new/delete flags to clones?
>>>
>>
>> It can theoretically happen, but it should be properly handled in the following
>> code:
>>
>>   810            if (is_delete_operator
>>   811                || gimple_call_builtin_p (stmt, BUILT_IN_FREE))
>>   812              {
>>   813                /* It can happen that a user delete operator has the pointer
>>   814                   argument optimized out already.  */
>>   815                if (gimple_call_num_args (stmt) == 0)
>>   816                  continue;
>>   817
>>   818                tree ptr = gimple_call_arg (stmt, 0);
>>   819                gimple *def_stmt;
>>   820                tree def_callee;
>>   821                /* If the pointer we free is defined by an allocation
>>   822                   function do not add the call to the worklist.  */
>>   823                if (TREE_CODE (ptr) == SSA_NAME
>>   824                    && is_gimple_call (def_stmt = SSA_NAME_DEF_STMT (ptr))
>>   825                    && (def_callee = gimple_call_fndecl (def_stmt))
>>   826                    && ((DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
>>   827                         && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_ALIGNED_ALLOC
>>   828                             || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
>>   829                             || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
>>   830                        || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
>>   831                  {
>>   832                    /* Delete operators can have alignment and (or) size as next
>>   833                       arguments.  When being a SSA_NAME, they must be marked
>>   834                       as necessary.  */
>>   835                    if (is_delete_operator && gimple_call_num_args (stmt) >= 2)
>>   836                      for (unsigned i = 1; i < gimple_call_num_args (stmt); i++)
>>   837                        {
>>   838                          tree arg = gimple_call_arg (stmt, i);
>>   839                          if (TREE_CODE (arg) == SSA_NAME)
>>   840                            mark_operand_necessary (arg);
>>   841                        }
>>
>> Where we verify that first argument of delete call is defined as a LHS of a new operator.
> 
> What I am saying is that it may be the wrong operator new.
> 
> Imagine something like:
> 
> struct A {
>   A();
>   __attribute__((malloc)) static void* operator new(unsigned long sz, void*,bool);
>   static void operator delete(void* ptr, void*,bool);
> };
> int main(){
>   int*p=new int;
>   new(p,true) A;
> }
> 
> At the beginning, it has
> 
>     D.2321 = A::operator new (1, p, 1);
> 
> and
> 
>     A::operator delete (D.2321, p, 1);
> 
> Now imagine we have access to the body of the functions, and the last one is transformed into
> 
>     operator delete.clone (p)

You are right. It can really lead to confusion of the DCE.

What we have is DECL_ABSTRACT_ORIGIN(decl) which we can use to indicate operators
that were somehow modified by an IPA optimization. Do you believe it will be sufficient?

Thanks,
Martin

> 
> (the first argument was removed)
> p does come from an operator new, so we do see a matched pair new+delete, but it is the wrong pair.
> 
> I admit that's rather unlikely, but with clones that have a different signature, many assumptions we could make on the functions become unsafe, and I am not clear on the scale of this issue.
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]