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: operator new returns nonzero


On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse <marc.glisse@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.glisse@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


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