Issue is overloading operator new and delete in framework with -02 or above optimisation

Question

we am facing issue with libc++ thats comes bundle with xcode 9.2

Scenario:

We have a framework which is overloading operator new and delete. The definition of these operator new and delete is kept hidden inside the dll with the apple guideline defined here: https://developer.apple.com/library/content/technotes/tn2185/_index.html#//apple_ref/doc/uid/DTS10004200-CH1-SECTION13

Also we have an application which is linking against this framework and overloading its own operator new and delete.

Problem:

Now, issue is that if a string(std::string) is created inside framework, it is calling application side operator new for memory allocation but at the time of destruction, it is calling framework side operator delete. This may lead to memory corruption due to different heap implementation on application side and framework side and is definately an issue.

This issue is observed only on release build of framework when -o2 or above optimization level is used. If we passed -fno-inline flag to compiler, this issue is not observed. Also this issue is not observed with xcode 8.2.

Upon further investigating this issue, I find that the destructor of basic_string is made inline in current version of libc++ which is not the case with libc++ that bundles with xcode 8.2. There is some discussion about this on clang forum : https://reviews.llvm.org/D24599

My guess is that because of inlining during run time linking, destructor of basic_string is referencing framework side operator delete to release the allocated memory but I need confirmation about my theory.

If this is the case, then should we build our framework with -fno-inline flag? Is there any major performance hit, if we use this flag or is there any other approach that we should consider?

Defination of overloaded operator new and delete:

void * operator new ( size_t len ) throw ( std::bad_alloc ){...}

void * operator new( std::size_t len, const std::nothrow_t & _nothrow ) throw (){...}

void * operator new[] ( size_t len ) throw ( std::bad_alloc ){...}

void operator delete ( void * ptr ) throw(){...}

void operator delete ( void * ptr, const std::nothrow_t & _nothrow ) throw (){...}

void operator delete[] ( void * ptr ) throw(){...}

looking for help

I will provide more info, if needed

Answer 1

From the Technical Note:

Your replacements will be in effect application wide. Even code in other linkage units (shared libraries) will call your custom new and delete. Throughout the application (all linkage units), there should be only one definition for the replaced new and delete. This will ensure that if memory ownership is transferred across a shared library boundary, it will be deleted correctly.

You have two definitions of new/delete in your application. This is already Bad(tm).

In general, shared libraries should not override these operators, unless that is the shared library's only job. Otherwise it becomes likely that an application will link to more than one definition of overridden new/delete. In rare circumstances a shared library may find it convenient to have private definitions of these operators. This is done by linking with the -unexported_symbols_list filename flag and placing the following symbols in the unexport file.

Can you double check if the library has really followed that rule - i.e. not exported the symbols?

In doing so, the author must ensure that memory ownership is not transferred into, or out of, this shared library. Note that memory ownership transfer can happen in subtle ways such as passing reference counted objects (e.g. std::string), throwing exceptions which contain a heap allocated message (e.g. std::runtime_error) or having a resource-allocating constructor inlined, with the corresponding destructor not inlined (or vice-versa).

I'd advise against using the -fno-inline flag as a "quick-fix", as that might only hide the current, obvious issue (multiple new/delete definitions) while still allowing memory corruption further down the line. If you have no way to get rid of the duplicate defintions (which would be be the far better choice IMO), you can only try ensure that the headers never contain anything that allocates memory while being able to be inlined - but even that I'd regard as both difficult & risky.