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)
(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.
--
Marc Glisse
