[Bug c++/94671] New: Wrong behavior with operator new overloading when using O2 for optimization

[Bug c++/94671] New: Wrong behavior with operator new overloading when using O2 for optimization

[basv@odd-e.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

            Bug ID: 94671
           Summary: Wrong behavior with operator new overloading when
                    using O2 for optimization
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: basv@odd-e.com
  Target Milestone: ---

This bug is a similar bug as an earlier reported in clang:
https://bugs.llvm.org/show_bug.cgi?id=15541

When having the overloaded new operator defined in other cpp or in a lib, g++
with O2 optimizes a new operation without assignment (or assignment to local or
static), and it will not call the overloaded new at all.

The following code will reproduce the problem.

---------------
test.cpp:
---------------

#include <iostream>

#define CHECK(expect, actual) std::cout << "EXPECT:" << expect <<
"\tACTUAL:"<<actual << std::endl
extern bool newCalled;

void testNewWithoutAssignment() {
        newCalled = false;
        new char;
        CHECK(1, newCalled);
}

static char* somechar;
void testNewWithAssignmentToStatic() {
        newCalled = false;
        somechar = new char;
        CHECK(1, newCalled);
}


char *p;
void testNewWithAssignment() {
        newCalled = false;
        p = new char;
        CHECK(1, newCalled);
}

int main() {
        testNewWithoutAssignment();
        testNewWithAssignmentToStatic();
        testNewWithAssignment();
        return 0;
}

---------------
new.cpp:
---------------

#include <exception>
#include <new>
#include <cstdlib>

extern bool newCalled;
bool newCalled = false;
void* operator new (size_t size)  throw (std::bad_alloc)
{
        newCalled = true;
        return malloc(size);
}

-----------

I know this is an optimization (it doesn't happen with -O0)... but it does
break the C++ standard.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
No, this conforms to the standard. See [expr.new]

> An implementation is allowed to omit a call to a replaceable global allocation
> function (17.6.2.1, 17.6.2.2). When it does so, the storage is instead
> provided by the implementation or provided by extending the allocation
> of another new-expression.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[basv@odd-e.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #2 from Bas Vodde <basv@odd-e.com> ---

Oh wow, does this mean that it is the choice of the compiler to actually call
an overloaded operator new ?

That is interesting. Thanks.

I'd still consider it highly surprising behavior, at least it was for me...

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
The question is whether the standard allows eliding of the side-effect setting
newCalled to true.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[basv@odd-e.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #4 from Bas Vodde <basv@odd-e.com> ---

The newCalled to true in the example was the simplest way to show the behavior.

This bug came up in a open source project called CppuTest. This has the
functionality to detect memory leaks and does so by overloading the operator
new. For each operator new, it keeps accounting information.

When an operator new gets optimized by the compiler, the framework can't keep
track of the accounting information and the delete call will report that
non-allocated memory was deleted.

I assume this is an useful and perfectly legit way of overloading operator
new/delete.

This behavior was caught when running the automated test of the framework,
which failed in the debian build when updating to gcc10:
https://people.debian.org/~doko/logs/gcc10-20200225/cpputest_3.8-7_unstable_gcc10.log

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[basv@odd-e.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #5 from Bas Vodde <basv@odd-e.com> ---

In the case we found this, it mostly uses the overload for accounting and thus
it doesn't cause a serious problem ('just' a test failure).

If you use the overloaded new/delete for providing your own memory management,
then this would potentially cause a serious problems.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, the compiler shouldn't optimize away the allocation and not the
deallocation or vice versa, it needs to either optimize away allocation and all
corresponding deallocations, or none of that.
There were some bugs on the GCC side, but 20200225 snapshot is very old, you
should try something newer, last related fix is from 4 days ago.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> The question is whether the standard allows eliding of the side-effect
> setting newCalled to true.

It does.

That side effect happens inside a replaceable global allocation function. The
compiler is allowed to optimise away calls to replaceable global allocation
functions.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Bas Vodde from comment #4)
> When an operator new gets optimized by the compiler, the framework can't
> keep track of the accounting information and the delete call will report
> that non-allocated memory was deleted.

Obviously memory that wasn't returned from your replacement operator new can't
be passed to your replacement operator delete. If calls to operator new are
combined into fewer calls, or elided completely, then the compiler is also
responsible for combining/eliding the corresponding calls to operator delete.

Read the proposal referenced in the Clang bug you linked to:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3433.html

I don't see any bug with current GCC 10.0.1 snapshots. If I provide a
replacement operator delete then it is only called once, with a pointer that
was returned by the single call to operator new.

[Bug c++/94671] Wrong behavior with operator new overloading when using O2 for optimization

[basv@odd-e.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94671

--- Comment #9 from Bas Vodde <basv@odd-e.com> ---

Hi Jonathan,

You are right, I was drawing much too many conclusions there. I should have
waited with that comment until I slept :) Sorry for that. And it has been a
while since I read the proposal, it has been years since we hit this problem in
clang.

Let me experiment a bit and see why gcc now behaves differently than clang, and
whether that can cause a non-optimized placement delete to called on a replace
operator new.

Otherwise, I still find it odd behavior but less harmful as I wrongly stated in
the previous comment.