Bug 68210 - nothrow operator fails to call default new
Summary: nothrow operator fails to call default new
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 9.0
Assignee: Jonathan Wakely
URL:
Keywords:
: 65290 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-11-04 16:04 UTC by Martin Sebor
Modified: 2022-07-29 16:41 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-11-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2015-11-04 16:04:48 UTC
A recent discussion about operator new (https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00192.html) revealed that the libstdc++ implementation of the nothrow overload of the operator doesn't conform to the requirement to "return a pointer obtained as if acquired from the (possibly replaced) ordinary version."  The test case below illustrates some of the problems.

The requirement quoted above implies that a conforming implementation of the nothrow overload must call the ordinary form like so:

void* operator new (size_t n, const nothrow_t&) {
    try {
        return operator new (n);
    }
    catch (...) {
        return 0;
    }
}


$ cat t.cpp && ~/bin/gcc-5.1.0/bin/g++ -Wall t.cpp && ./a.out 
#include <new>
#include <assert.h>
#include <stdlib.h>

struct MyBadAlloc: std::bad_alloc { };

static bool new_fail;
static bool bad_alloc_thrown;
static unsigned new_called;
static unsigned new_handler_called;

static void new_handler ()
{
    if (new_handler_called++)
        throw MyBadAlloc ();
}

void* operator new (size_t n)
{
    static size_t cntr;

    ++new_called;

    for ( ; ; ) {
        if (void *p = new_fail ? 0 : malloc (n + sizeof n)) {
            *static_cast<size_t*>(p) = ++cntr;
            return static_cast<size_t*>(p) + 1;
        }

        if (std::new_handler h = std::set_new_handler (0)) {
            std::set_new_handler (h);
            h ();
        }
        else {
            bad_alloc_thrown = true;
            throw MyBadAlloc ();
        }
    }
}

void operator delete (void *p)
{
    if (p)
        free (static_cast<size_t*>(p) - 1);
}

int main ()
{
    new_called = 0;
    void *p = operator new (1, std::nothrow);

    assert (p != 0);
    assert (1 == new_called);

    std::set_new_handler (new_handler);
    new_fail = true;

    try {
        p = operator new (1, std::nothrow);
    }
    catch (...) {
        assert (!"nothrow operator new threw");
    }

    assert (0 == p);
    assert (2 == new_handler_called);
    assert (bad_alloc_thrown);
}
a.out: t.cpp:53: int main(): Assertion `1 == new_called' failed.
Aborted (core dumped)
Comment 1 Jonathan Wakely 2016-03-22 17:52:01 UTC
*** Bug 65290 has been marked as a duplicate of this bug. ***
Comment 2 Jonathan Wakely 2016-03-22 18:06:45 UTC
The dup PR 65290 pointed out the requirements were changed by http://cplusplus.github.io/LWG/lwg-defects.html#206 -- we still implement the C++03 rules.

We should fix this. However, the C++11 requirement means that the nothrow version of new has to pay for the throw/catch overhead. If we could tell when these functions have not been replaced we could avoid that overhead.

Also we certainly don't want to conform to the new requirement when libstdc++ is built with -fno-exceptions, because allocation failure would abort in operator new(size_t) and so the nothrow version never gets a chance to handle the exception and return null.

We will also need to change operator new[](size_t, nothrow_t) which currently calls operator new(size_t, nothrow_t), rather than operator new[](size_t) as required. If a user only replaces operator new[](size_t) then their replacement won't be called by operator new[](size_t, nothrow_t). Again, what we implement is the C++03 rule.
Comment 3 Jakub Jelinek 2017-05-02 15:56:53 UTC
GCC 7.1 has been released.
Comment 4 Jonathan Wakely 2018-08-10 19:30:50 UTC
(In reply to Martin Sebor from comment #0)
>     assert (0 == p);
>     assert (2 == new_handler_called);
>     assert (bad_alloc_thrown);

I think this test case is wrong. The new handler is set, so we never get to:

        else {
            bad_alloc_thrown = true;
            throw MyBadAlloc ();
        }

So bad_alloc_throw is never set.

I'm using this instead, which passes with my fixed libstdc++ and with libc++:

int main ()
{
    new_called = 0;
    void *p = operator new (1, std::nothrow);

    VERIFY (p != 0);
    VERIFY (1 == new_called);

    std::set_new_handler (new_handler);
    new_fail = true;

    try {
        p = operator new (1, std::nothrow);
    }
    catch (...) {
        VERIFY (!"nothrow operator new threw");
    }

    VERIFY (0 == p);
    VERIFY (2 == new_handler_called);
    VERIFY (!bad_alloc_thrown);

    std::set_new_handler (0);
    new_fail = true;
    new_handler_called = 0;

    try {
        p = operator new (1, std::nothrow);
    }
    catch (...) {
        VERIFY (!"nothrow operator new threw");
    }

    VERIFY (0 == p);
    VERIFY (0 == new_handler_called);
    VERIFY (bad_alloc_thrown);
}
Comment 5 Jonathan Wakely 2018-08-10 19:35:41 UTC
Or maybe:

int main ()
{
    void *p = operator new (1, std::nothrow);

    VERIFY (p != 0);
    VERIFY (1 == new_called);
    VERIFY (0 == new_handler_called);
    VERIFY (!bad_alloc_thrown);
    operator delete(p);

    new_fail = true;
    p = operator new (1, std::nothrow);

    VERIFY (0 == p);
    VERIFY (2 == new_called);
    VERIFY (0 == new_handler_called);
    VERIFY (bad_alloc_thrown);

    new_fail = true;
    bad_alloc_thrown = false;
    std::set_new_handler (new_handler);
    p = operator new (1, std::nothrow);

    VERIFY (0 == p);
    VERIFY (3 == new_called);
    VERIFY (2 == new_handler_called);
    VERIFY (!bad_alloc_thrown);
}
Comment 6 Jonathan Wakely 2018-08-10 20:21:03 UTC
Author: redi
Date: Fri Aug 10 20:20:27 2018
New Revision: 263478

URL: https://gcc.gnu.org/viewcvs?rev=263478&root=gcc&view=rev
Log:
PR libstdc++/68210 adjust operator new and delete for LWG 206

Ensure that nothrow versions of new and delete call the ordinary
versions of new or delete, instead of calling malloc or free directly.

These files are all compiled with -std=gnu++14 so can use noexcept and
nullptr to make the code more readable.

	PR libstdc++/68210
	* doc/xml/manual/intro.xml: Document LWG 206 change.
	* libsupc++/del_op.cc: Replace _GLIBCXX_USE_NOEXCEPT with noexcept.
	* libsupc++/del_opa.cc: Likewise.
	* libsupc++/del_opant.cc: Likewise.
	* libsupc++/del_opnt.cc: Likewise. Call operator delete(ptr) instead
	of free(ptr).
	* libsupc++/del_ops.cc: Replace _GLIBCXX_USE_NOEXCEPT with noexcept.
	* libsupc++/del_opsa.cc: Likewise.
	* libsupc++/del_opva.cc: Likewise.
	* libsupc++/del_opvant.cc: Likewise.
	* libsupc++/del_opvnt.cc: Likewise. Call operator delete[](ptr)
	instead of operator delete(ptr).
	* libsupc++/del_opvs.cc: Replace _GLIBCXX_USE_NOEXCEPT with noexcept.
	* libsupc++/del_opvsa.cc: Likewise.
	* libsupc++/new_op.cc: Use __builtin_expect in check for zero size.
	* libsupc++/new_opa.cc: Use nullptr instead of literal 0.
	* libsupc++/new_opant.cc: Likewise. Replace _GLIBCXX_USE_NOEXCEPT
	with noexcept.
	* libsupc++/new_opnt.cc: Likewise. Call operator new(sz) instead of
	malloc(sz).
	* libsupc++/new_opvant.cc: Use nullptr and noexcept.
	* libsupc++/new_opvnt.cc: Likewise. Call operator new[](sz) instead of
	operator new(sz, nothrow).
	* testsuite/18_support/new_nothrow.cc: New test.

Added:
    trunk/libstdc++-v3/testsuite/18_support/new_nothrow.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/doc/xml/manual/intro.xml
    trunk/libstdc++-v3/libsupc++/del_op.cc
    trunk/libstdc++-v3/libsupc++/del_opa.cc
    trunk/libstdc++-v3/libsupc++/del_opant.cc
    trunk/libstdc++-v3/libsupc++/del_opnt.cc
    trunk/libstdc++-v3/libsupc++/del_ops.cc
    trunk/libstdc++-v3/libsupc++/del_opsa.cc
    trunk/libstdc++-v3/libsupc++/del_opva.cc
    trunk/libstdc++-v3/libsupc++/del_opvant.cc
    trunk/libstdc++-v3/libsupc++/del_opvnt.cc
    trunk/libstdc++-v3/libsupc++/del_opvs.cc
    trunk/libstdc++-v3/libsupc++/del_opvsa.cc
    trunk/libstdc++-v3/libsupc++/new_op.cc
    trunk/libstdc++-v3/libsupc++/new_opa.cc
    trunk/libstdc++-v3/libsupc++/new_opant.cc
    trunk/libstdc++-v3/libsupc++/new_opnt.cc
    trunk/libstdc++-v3/libsupc++/new_opvant.cc
    trunk/libstdc++-v3/libsupc++/new_opvnt.cc
Comment 7 Jonathan Wakely 2018-08-10 21:48:38 UTC
Fixed for gcc 9.
Comment 8 Matthijs Kooijman 2022-07-29 16:27:00 UTC
Note that in comment:2, Jonathan Wakely pointed out a caveat:

> Also we certainly don't want to conform to the new requirement when
> libstdc++ is built with -fno-exceptions, because allocation failure
> would abort in operator new(size_t) and so the nothrow version never
> gets a chance to handle the exception and return null.

But this was not taken into account when implementing the fix for this issue, meaning nothrow operators are now effectively useless with -fno-exceptions (and there is thus no way to handle allocation failure other than aborting in that case).

I created a new bug report about this here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106477