On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
> On Sat, 7 Sep 2013, Marc Glisse wrote:
>
>> this patch teaches the compiler that operator new, when it can throw,
>> isn't allowed to return a null pointer. Note that it doesn't work for
>> placement new (that one may have to be handled in the front-end or the
>> inliner),
>
>
> Placement new is a completely different thing. For one, it is nothrow (so
> the test doesn't apply). But mostly, it is a condition on the argument more
> than the result. Using the nonnull attribute would make sense, but the
> compiler doesn't seem clever enough to use that information. The easiest
> seems to be in the library:
>
> --- o/new 2013-09-07 11:11:17.388756320 +0200
> +++ i/new 2013-09-07 18:00:32.204797291 +0200
> @@ -144,9 +144,9 @@
>
> // Default placement versions of operator new.
> inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
> -{ return __p; }
> +{ if (!__p) __builtin_unreachable(); return __p; }
> inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
> -{ return __p; }
> +{ if (!__p) __builtin_unreachable(); return __p; }
>
> // Default placement versions of operator delete.
> inline void operator delete (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
>
>
>
> Though I am not sure what the policy is for this kind of thing. And that's
> assuming it is indeed undefined to pass a null pointer to it, I don't have a
> good reference for that.
>
>
>
>> and it will not work on user-defined operator new if they are inlined.
>
>
> But then if that operator new is inlined, it may already contain a line like
> if(p==0)throw(); which lets the compiler know all it needs.
I am not sure I like this version of the patch.
I think the appropriate patch should be in the compiler, not
here. C++ has several places where certain parameters cannot
have non-null values. For example, the implicit parameter 'this'
in non-static member functions. This is another instance.
Furthermore, I do think that the compiler should have special nodes
for both standard placement new and the global operator new functions,
as I explained in previous messages.
-- Gaby
>
>
>> I guess it would be possible to teach the inliner to insert an assert_expr
>> or something when inlining such a function, but that's not for now. The code
>> I removed from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and
>> thus useless.
>>
>> I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper
>> testing when trunk bootstraps again.
>>
>> 2013-09-07 Marc Glisse <marc.gli...@inria.fr>
>>
>> PR c++/19476
>> gcc/
>> * fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
>> * tree-vrp.c (gimple_stmt_nonzero_warnv_p,
>> stmt_interesting_for_vrp):
>> Likewise.
>> (vrp_visit_stmt): Remove duplicated code.
>>
>> gcc/testsuite/
>> * g++.dg/tree-ssa/pr19476-1.C: New file.
>> * g++.dg/tree-ssa/pr19476-2.C: Likewise.
>
>
> --
> Marc Glisse
