Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overloaded delete not called when using sections garbage collection #132

Open
ilg-ul opened this issue Jun 5, 2024 · 5 comments
Open

overloaded delete not called when using sections garbage collection #132

ilg-ul opened this issue Jun 5, 2024 · 5 comments

Comments

@ilg-ul
Copy link

ilg-ul commented Jun 5, 2024

I have a small test that overloads new/delete, calls them and checks if the overloaded functions were really called:

cd tmp

cat <<__EOF__ >overload-new.cpp

#include <iostream>
#include <stdlib.h>

static int news = 0, deletes = 0;

void *operator new(size_t size) { news++; return malloc(size); }
void operator delete(void *ptr) noexcept { deletes++; free(ptr); }

int main(int argc, const char *argv[]) {
    std::cout << "";
    delete new int;
    std::cout << "news: " << news << ", deletes: " << deletes << std::endl;

    return news == 0 || deletes == 0;
}
__EOF__

g++-14 -c overload-new.cpp -o overload-new.cpp.o
g++-14 overload-new.cpp.o -o overload-new-cpp
./overload-new-cpp

g++-14 -c overload-new.cpp -o gc-overload-new.cpp.o -ffunction-sections -fdata-sections
g++-14 gc-overload-new.cpp.o -o gc-overload-new-cpp -ffunction-sections -fdata-sections -Wl,-dead_strip
./gc-overload-new-cpp

With the default configuration the test passes, both new and delete are accounted as called.

If I use section garbage collection, delete is not called.

However, if I set the standard to c++11, it works as expected.

g++-14 -c overload-new.cpp -o gc-11-overload-new.cpp.o -ffunction-sections -fdata-sections -std=c++11
g++-14 gc-11-overload-new.cpp.o -o gc-11-overload-new-cpp -ffunction-sections -fdata-sections -Wl,-dead_strip
./gc-11-overload-new-cpp

The test is borrowed with permission from Martin Storsjö, the llvm-mingw project.

g++-14 is from Homebrew, but I noticed this behaviour with by GCC 14 & 13 too.

For my tests I'm tempted to add -std=c++11 and hide the issue, but I thought to report it, perhaps for older sources it might be relevant.

@iains
Copy link
Owner

iains commented Jun 5, 2024

thanks for the report.

-ffunction-sections -fdata-sections should make no difference on Darwin (since we are limited to 255 sections max, we cannot allocate one section per function).

What should happen with replaceable new/delete is that the linker chooses the non-weak replacement (provided by the user) in preference to the weak version (provided by libstdc++/libc++)

In principle, -dead_strip in the linker should not make any difference to this.

I think we need to test this on unpatched upstream trunk (on x86_64) and double-check we are emitting things correctly (if were are, then this is a linker bug).

@iains
Copy link
Owner

iains commented Jun 5, 2024

-ffunction-sections -fdata-sections should make no difference on Darwin (since we are limited to 255 sections max, we cannot allocate one section per function).

For the record, in case that seems like a bad thing, this is an ELF-ism.

Mach-O has a different (and internally automatic) mechanism which is triggered by .subsections_by_symbols. In this (so-called 'atom') model, we can split code or data on any linker-visible symbol (thus it is actually more fine-grained than ffunction/data-sections and does not require the user to specify it, it is on by default).

@ilg-ul
Copy link
Author

ilg-ul commented Jun 5, 2024

Thank you for explaining this.

double-check we are emitting things correctly (if were are, then this is a linker bug).

I'm afraid this is a bit too much for me...

It is also curious why the issue is not observed when using C++11.

@iains
Copy link
Owner

iains commented Jun 5, 2024

Thank you for explaining this.

double-check we are emitting things correctly (if were are, then this is a linker bug).

I'm afraid this is a bit too much for me...

that's fine - it is something for me to check - but I want to do that on the upstream code (not this branch) because we should not have made any changes to this behaviour in the patches to support arm64.

It is also curious why the issue is not observed when using C++11.

me too :)

@ilg-ul
Copy link
Author

ilg-ul commented Aug 8, 2024

The problem seems to be fixed on 14.2, all my tests passed, on both x64 and arm64:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants