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.
>
