[libc++] regression: new/delete symbol overrides broken on macOS

[tycho]

This is referring to commit 8418955.

In a shared library which statically links libc++ (ANGLE's libEGL in this case), the symbols for new and new[] are, as of the above commit, suddenly exposed as global, but the corresponding delete and delete[] operators are not.

Before the above commit:

$ nm -g -C --defined-only contrib/angle/angle/out/macOS-Debug-arm64/libEGL.dylib | grep -e new -e delete

After:

$ nm -g -C --defined-only contrib/angle/angle/out/macOS-Debug-arm64/libEGL.dylib | grep -e new -e delete
00000000001e9460 T operator new[](unsigned long)
00000000001e9740 T operator new[](unsigned long, std::align_val_t)
00000000001e9328 T operator new(unsigned long)
00000000001e95e0 T operator new(unsigned long, std::align_val_t)

This causes applications like mine with custom allocators (mimalloc in this case) to provide the implementations for the operator new symbols, but not the delete symbols, which inevitably causes a crash within the shared library when it tries to free memory.

[ShabbyX]

Attn @nico

[tycho]

I think it's this line specifically that suddenly made them visible globally. Strange that this change was only done to new/new[]...

[ldionne]

@petrhosek Could you have a look?

[petrhosek]

@petrhosek Could you have a look?

Yes, I was already looking at the macOS implementation in the context of #122983 so I can look into this.

@tycho Do you have instructions for building libEGL.dylib so I can reproduce this locally?

[tycho]

ANGLE's build directions:

https://chromium.googlesource.com/angle/angle/+/main/doc/DevSetup.md

The TL;DR is something like:

• clone ANGLE
• clone depot_tools
• add depot_tools to PATH
• enter ANGLE source tree, gclient sync
• mkdir -p out/debug; cd out/debug
• make an args.gn file
• gn gen .
• ninja libEGL libGLESv2

and you should get a libEGL + libGLESv2. Their version of libc++ comes from the third_party/libc++/src directory, which gets autorolled from Chromium periodically.

This is the args.gn I am using for arm64 debug builds of ANGLE at the moment, and what I used for the repro in the OP:

target_cpu = "arm64"
angle_enable_gl = true
angle_enable_gl_desktop_backend = true
angle_enable_d3d11 = false
angle_enable_d3d9 = false
angle_enable_glsl = true
angle_enable_metal = true
angle_enable_null = false
angle_enable_swiftshader = false
angle_enable_trace = false
angle_enable_annotator_run_time_checks = true
angle_enable_vulkan_validation_layers = true
angle_enable_vulkan_gpu_trace_events = true
angle_enable_vulkan = true
angle_enable_wgpu = false
angle_expose_non_conformant_extensions_and_versions = true
angle_enable_custom_vulkan_cmd_buffers = false
angle_enable_overlay = true
angle_enable_share_context_lock = false
angle_enable_global_mutex_recursion = true
angle_has_astc_encoder = false
is_component_build = false
enable_stripping = false
dcheck_always_on = true
angle_use_wayland = false
is_debug = true
use_custom_libcxx = true
is_clang = true
use_thin_lto = false
treat_warnings_as_errors = false
angle_use_custom_libvulkan = true
is_asan = false
is_tsan = false

# macOS-specific configuration
mac_deployment_target = "11.0"
angle_shared_libvulkan = true
enable_dsyms = true

# Support ccache wrapper for faster repeated builds
cc_wrapper = "/opt/homebrew/bin/ccache"

[tycho]

This allows me to build and run with libEGL+libGLESv2 without crashing:

libc++.patch

It basically makes all the delete operators visible (and overridable) too. But naturally it doesn't have the extra checks that the new operator ones have to notice partially overridden symbols.

But that did lead me to notice that not all of the new operators are exposed. libc++ is making these visible:

operator new[](unsigned long)
operator new[](unsigned long, std::align_val_t)
operator new(unsigned long)
operator new(unsigned long, std::align_val_t)

But it misses the nothrow versions, for some reason (the following are covered by mimalloc, but libc++ doesn't expose them?):

operator new[](unsigned long, std::nothrow_t const&)
operator new[](unsigned long, std::align_val_t, std::nothrow_t const&)
operator new(unsigned long, std::nothrow_t const&)
operator new(unsigned long, std::align_val_t, std::nothrow_t const&)

[petrhosek]

The issue isn't .globl which is always emitted by Clang for operator new and operator delete, even in the Chromium build. Rather, Chromium build uses the -fvisibility-global-new-delete=force-hidden option which is lowered to .private_extern directive which allows the linker to internalize the symbol and it is why those symbols end up hidden in Chromium binaries.

We don't want to use _LIBCPP_OVERRIDABLE_FUNCTION for operator delete since we don't need the overriding detection for those symbols, rather we want to ensure that operator new is marked as .private_extern when -fvisibility-global-new-delete=force-hidden is set.

I have reworked the implementation to avoid the use of assembly altogether in #122983 which should address this issue once landed.