On Wed, 4 Aug 2021, 02:15 Modi Mo, <mod...@fb.com> wrote:
> On 8/3/21, 4:57 PM, "Jonathan Wakely" <jwakely....@gmail.com> wrote:
>
> > On Tue, 3 Aug 2021, 23:10 Modi Mo, wrote:
> > >
> > > On 8/3/21, 1:51 PM, "Jonathan Wakely" <jwakely....@gmail.com> wrote:
> > > >
> > > > Could you explain this sentence in the commit message:
> > > > "Note that the definition of global new is user-replaceable so
> users
> > > > should ensure that the one used follows these semantics."
> > >
> > > AFAICT based on the C++ standard, the user can replace the definition
> of operator new with anything they want.
> >
> >
> > No, they have to meet certain requirements.
> >
> > If any operator new is potentially throwing (i.e. not declared
> > noexcept) and it returns null, you get UB. A potentially throwing
> > operator new *must* return non-null, or throw something they can be
> > caught by catch(bad_alloc const&), or not return (e.g. terminate).
> > If any operator new is noexcept, then it reports failure by returning
> > null instead of throwing.
>
> Ah right, I found this out when implementing which is the main motivation
> behind using "throw()" as noexcept will categorize all ::new as
> ::new(std::nothrow) which isn't what we're after in this implementation.
>
This seems like a clang-specific implementation detail. I think GCC treats
throw() as exactly equivalent to noexcept.
But maybe __attribute__((nothrow,return_nonnull)) would do it for GCC.
> > So if the user replaces the global operator new(size_t), then their
> > replacement must never return null, because it's declared without
> > noexcept.
> >
> >
> > >
> > > In the current implementation we're using an un-enforced "throw()"
> annotation so it's up to the implementation of ::new (in libstdc++/jemalloc
> etc.) to comply properly or risk UB.
> >
> > That's what I was afraid of. The implementation in libstdc++ cannot
> > know the user passed -fnew-infallible so has no way of knowing whether
> > it's allowed to throw, or if it has to terminate on failure. So it
> > will throw in failure, which will then be undefined.
> >
> > So it seems the user must either provide a replacement operator new
> > which terminates, or simply ensure that their program never exceeds
> > the system's available memory. In the latter case, the
> > -fnew-infallible option is basically a promise to the compiler that
> > the system has "infinite" memory (or enough for the program's needs,
> > if not actually infinite).
>
> Yeah, there's a disconnect between declaration (what -fnew-infallible
> does) and definition (libstdc++). I don't know of a way to enforce this
> automatically without creating a shim like you suggested.
>
> > > Marking ::new as noexcept is more rigid of an enforcement but I
> believe it's implementation defined as to how exception specifications are
> merged
> >
> > I think it's undefined if two declarations of the same function have
> > different exception specifications. I might be wrong.
>
> The main reason I didn't do this is as you identified above, marking it
> noexcept from a standard viewpoint changes ::new to : :new(std::nothrow).
>
> > Making it noexcept would currently mean that the compiler must assume
> > it can return null, and so adds extra checks to a new-expression so
> > that if operator new returns null no object is constructed. But if
> > it's noexcept *and* return_nonnull I guess that check can be elided.
>
> I think doing this could work though I feel like there'll be some rough
> edges that have to be filed down.
>
> > > > What happens if operator new does throw? Is that just treated
> like any
> > > > other noexcept function that throws, i.e. std::terminate()? Or is
> it
> > > > undefined behaviour?
> > >
> > > It gets treated like the older "throw()" annotation where it's UB,
> there's no enforcement.
> >
> >
> > It's not UB for a throw() function to throw. In C++03 it would call
> > std::unexpected() which by default calls std::terminate(). So throw()
> > is effectively the same as noexcept.
>
> What's getting altered ATM is the implicit declaration of "new" that
> doesn't actually require the header to be copied in:
>
> These implicit declarations introduce only the function names operator
> new, operator new[], opera- tor delete, and operator delete[]. [Note: The
> implicit declarations do not introduce the names std, std::size_t,
> std::align_val_t, or any other names that the library uses to declare these
> names.
> Thus, a new-expression, delete-expression, or function call that refers to
> one of these functions without importing or including the header <new>
> (17.6.1) is well-formed.
>
Ah right, yes, it's not enough to adjust <new>, so it needs to be a
compiler flag.
> The definition though doesn't have to abide by this and if it doesn't then
> UB gets triggered.
>
> > > > And what if you use ::new(std::nothrow) and that fails, returning
> > > > null. Is that undefined?
> > >
> > > This change doesn't affect ::new(std::nothrow) ATM. Nathan Sidwell
> suggested that we may want to change semantics for non-throwing as well to
> be truly "infallible" but we don't have a use-case for this scenario as of
> yet.
> >
> > Ah, I see. So it only affects operator new(size_t)?
> > And operator new(size_t, align_val_t) too?
> > And the corresponding operator new[] forms?
>
> Yep.
>
> > > > It seems to me that the former case (a potentially-throwing
> allocation
> > > > function) could be turned into a std::terminate call fairly easily
> > > > without losing the benefits of this option (you still know that no
> > > > call to operator new will propagate exceptions).
> > >
> > > Could you elaborate? That would be great if there's a way to do this
> without requiring enforcement on the allocator definition.
> >
> > Well I meant adding enforcement when calling operator new. The
> > compiler could treat every call to operator new as though it's done
> > by:
> >
> > void* __call_new(size_t n) noexcept ( return operator new(n); }
> >
> > so that an exception will call std::terminate. But that would add some
> > overhead (more for Clang than for GCC, due to how noexcept is
> > enforced), and that overhead is presumably unnecessary if the
> > intention of the option is to make a promise that operator new won't
> > ever throw.
>
> The extra overhead is pretty meaningful as when inlined you'll get
> terminate landing pads at each site and when not inlined you get an extra
> call.
>
These landing pads are not present in GCC, as the EH tables are used
instead.
> > > > But for the latter case the program *must* replace operator new
> to ensure that the
> > > > nothrow form never returns null, otherwise you violate the promise
> > > > that the returns_nonnull attribute makes, and have undefined
> > > > behaivour. Is that right?
> > >
> > > Yes good insight. That's another tricky aspect to implementing this
> for non-throwing new.
> >
> > If you just treat -fnew-infallible as a promise that operator new
> > never fails, then ISTM that it's OK to treat failure as undefined (you
> > said failure was impossible, if it fails, that's your fault). And if
> > you replace operator new, you can keep your promise by making it
> > terminate on failure.
>
> Agreed, from a design standpoint operator new failing with
> -fnew-infallible is just UB.
>
> > But if some enforcement is wanted (maybe via
> > -fnew-infallible-trust-but-verify ;-) then calls to non-throwing new
> > could be enforced as if called by:
> >
> > void* __call_new_nt(std::size_t n) noexcept {
> > if (auto p = operator new(n, std::nothrow)) return p;
> > std::terminate();
> > }
> >
> > Anyway, all that aside, I find the idea interesting even if it's just
> > an unchecked promise by the user that new won't fail.
>
> :D
>
> > I wonder how far we could get just by changing the <new> header, so
> > that a -D option could be used instead of -fnew-infallible:
> >
> > --- a/libstdc++-v3/libsupc++/new
> > +++ b/libstdc++-v3/libsupc++/new
> > @@ -123,8 +123,14 @@ namespace std
> > * Placement new and delete signatures (take a memory address argument,
> > * does nothing) may not be replaced by a user's program.
> > */
> > +#ifdef _GLIBCXX_NEW_INFALLIBLE
> > +_GLIBCXX_NODISCARD __attribute__((return_nonnull))
> > + void* operator new(std::size_t) noexcept
> > + __attribute__((__externally_visible__));
> > +#else
> > _GLIBCXX_NODISCARD void* operator new(std::size_t) _GLIBCXX_THROW
> > (std::bad_alloc)
> > __attribute__((__externally_visible__));
> > +#endif
> > _GLIBCXX_NODISCARD void* operator new[](std::size_t) _GLIBCXX_THROW
> > (std::bad_alloc)
> > __attribute__((__externally_visible__));
> > void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
> >
> >
> > (and similarly for the new[] and aligned new overloads)
>
> I tried this initially but hit exception spec conflicts against the
> implicit definitions that the Clang FE generates which are unchanged.
> The implicit declarations are also relevant in more cases where <new>
> isn't included.
>
Understood.
On the subject of adding a libstdc++ -D option in this area:
>
> libstdc++-v3/include/ext/new_allocator.h
>
> // NB: __n is permitted to be 0. The C++ standard says nothing
> // about what the return value is when __n == 0.
> _GLIBCXX_NODISCARD _Tp*
> allocate(size_type __n, const void* = static_cast<const void*>(0))
> {
> #if __cplusplus >= 201103L
> // _GLIBCXX_RESOLVE_LIB_DEFECTS
> // 3308. std::allocator<void>().allocate(n)
> static_assert(sizeof(_Tp) != 0, "cannot allocate incomplete
> types");
> #endif
>
> if (__builtin_expect(__n > this->_M_max_size(), false))
> {
> // _GLIBCXX_RESOLVE_LIB_DEFECTS
> // 3190. allocator::allocate sometimes returns too little
> storage
> if (__n > (std::size_t(-1) / sizeof(_Tp)))
> std::__throw_bad_array_new_length();
> std::__throw_bad_alloc();
> }
>
> With the goal of reducing exception propagation, the exception here from
> exceeding _M_max_size() which is 2^64 bits on 64-bit systems should
It should not be more than 2^63 on typical systems:
#if __PTRDIFF_MAX__ < __SIZE_MAX__
return std::size_t(__PTRDIFF_MAX__) / sizeof(_Tp);
#else
return std::size_t(-1) / sizeof(_Tp);
#endif
never happen but does pessimize propagation if it can't be inlined/proven
> untaken. We're looking at a -D option "_GLIBCXX_RESTRICT_EXCEPT" (name TBD)
> to mark functions like these as noexcept. With ::new being infallible a lot
> of these functions (like _S_check_init_len in stl_vector.h as well) have
> these very unlikely corner cases that now become the exceptional roots and
> could be optimized optimistically now.
>
I'm interested in this too, let me know when you want to pursue this
(probably on the libstdc++ mailing list).
