Re: operator new returns nonzero

[Marc Glisse <marc dot glisse at inria dot fr>]

On Sat, 7 Sep 2013, Gabriel Dos Reis wrote:

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

The 2 patches are really independent, one (in the compiler) for regular 
operator new, and one (in the library) for the placement version.

I mostly like this second patch because it is so easy ;-)
But I will be happy if someone posts a better solution.

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

Indeed, I didn't check how gcc handles "this" being non-0.

> Furthermore, I do think that the compiler should have special nodes
> for both standard placement new and the global operator new functions,

That's one way to do it. Since this is the first time I look at those, I 
don't really see the advantage compared to the current status, but I 
trust you. What would you do with this special-node placement new? Keep it 
as is until after vrp so we can use the !=0 property and then expand it to 
its first argument? Or expand it early to the equivalent of the library 
code I wrote above?

> as I explained in previous messages.

I couldn't find them, sorry if they contained information that makes this 
post irrelevant.

--
Marc Glisse