[native] Stop linking libc++ into the CoreCLR and NativeAOT runtimes - #12572
Draft
simonrozsival wants to merge 83 commits into
Draft
[native] Stop linking libc++ into the CoreCLR and NativeAOT runtimes#12572simonrozsival wants to merge 83 commits into
simonrozsival wants to merge 83 commits into
Conversation
Use fixed buffers for straightforward type-name, override-path, and system-property values, formatting composed strings with snprintf. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the remaining logger, max-gref, and timing property consumers to explicit fixed buffers so the CLR dynamic-local-string property overload can be removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The logger interface no longer exposes local-string types, so keep the temporary include local to its remaining fallback-path implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allocate managed type and timing strings to their exact sizes instead of treating the former local-string stack threshold as a maximum. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep exact-size type and timing strings independent of libc++ ownership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the existing NativeAOT fixed-storage limit while preserving unbounded CoreCLR path construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use malloc only when typemap or override names exceed their sensible local buffer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve stack storage for typical managed type and override paths while allocating the exact required capacity for larger values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route managed type and override path heap-buffer cleanup through Util. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rely on free(nullptr) and name stack-backed CLR string storage explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eliminate separate heap pointers and free generated CLR strings only when they differ from their stack buffers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Narrowing the `strings.hh` include in `logger.hh` also removed two symbols that headers were picking up transitively through it: * `strings.hh` included `shared/helpers.hh`, which is where `os-bridge.hh` was getting `abort_unless` from. * `strings.hh` included `<unistd.h>`, which is where `bridge-processing.cc` was getting `gettid()` from. Include both explicitly at their point of use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Narrowing the strings.hh include in logger.hh removed the transitive path that util.cc relied on for dynamic_local_string, breaking the CoreCLR and NativeAOT builds. Include the header where it is used. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Addresses review feedback: the fixed-buffer overload returned -1 when a bundled (build-time) property value did not fit into the caller's buffer, so long values were treated as if the property were not set at all. The `dynamic_local_string` based overload it replaced grew onto the heap and had no such limit. Bundled properties come from `@(AndroidEnvironment)` files and are stored as NUL-terminated strings in static application data, so they are neither subject to Android's 92 byte property limit nor in need of copying. Return a `std::string_view` instead of an `int`: for Android system properties it views the caller's scratch buffer, for bundled properties it points directly at the application data, which restores the previous behaviour and avoids a copy. `FastTiming::parse_options()` used to tokenize its argument in place, which is not safe for a view over static data, so it now parses without mutating. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
…perty
The previous commit changed `monodroid_get_system_property ()` to return a
`std::string_view` so that bundled properties, whose length is not limited by
`PROPERTY_VALUE_BUFFER_LEN`, could be returned without copying them into the
caller's scratch buffer.
That works, but `std::string_view` deliberately makes no promise about
NUL-termination, while every value this function can return happens to be
NUL-terminated: `__system_property_get ()` terminates what it writes, and
bundled properties are NUL-terminated strings in static application data. The
header had to document that invariant in a comment ("The returned value is
always NUL-terminated") precisely because the type denies it, and callers such
as `get_max_gref_count_from_system ()` silently relied on it by passing
`.data ()` to `strtol ()` and to a `%s` format specifier.
Return `const char*` instead (and `nullptr` when the property is not set). The
lifetime rule is unchanged and still uniform - the result is valid for at least
as long as the caller's buffer - but NUL-termination is now guaranteed by the
type rather than by a comment, so `.data ()` no longer has to be laundered
through a `std::string_view`. Callers that need to tokenize the value construct
a `std::string_view` explicitly, which is honest about what they are doing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
- `format_managed_type_name ()` builds the name with a single `snprintf ()` instead of three `memcpy ()` calls and hand-rolled length arithmetic. The negative-required-capacity retry contract is unchanged, and the helper is now guarded by `#if defined (DEBUG)` like its only caller, which removes the unused-function warning this pull request introduced. - `FastTiming::parse_options ()` takes a `const char*` again and tokenizes with `strchr ()`/`strncmp ()`/`strtoull ()`. It had been rewritten around `std::string_view`, which added a C++ layer to code that was already plain C. The parser still cannot NUL-terminate in place - the value may point at immortal bundled property data - so each parameter is bounded by its length instead. The `duration=` and `filename=` edge cases behave as they did before. - The property lookup chain (`monodroid_get_system_property ()`, `monodroid__system_property_get ()` and `lookup_system_property ()`) takes `const char *name`, matching the other overloads. Previously it took a `std::string_view` and immediately called `.data ()` on it, which is the same NUL-termination laundering that motivated changing the return type. This also lets `HostEnvironment::lookup_system_property ()` use `strcmp ()` directly and drops `<string_view>` from `android-system-shared.cc` entirely. - Shorten the comments added by this pull request, and drop the `strings.hh` include from `logger.cc`, which no longer uses `dynamic_local_string`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
`Logger::init_logging_categories ()` split the property value with `std::string_view`, and `set_category ()`, `set_log_file ()` and `open_file ()` took views as well. Tokenize the value with `strchr ()`/`strncmp ()` instead and pass the parameters around as a pointer and a length. This also removes a subtle NUL-termination assumption: `open_file ()` called `unlink (path.data ())`, which is only correct because every caller happened to pass a view over a NUL-terminated buffer. It now takes a `const char*`. A single `param_matches ()` helper does all of the comparisons, so the parameters no longer have to be NUL-terminated in place - the value may point at immortal bundled property data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The function returns either the caller's scratch buffer or a pointer into application data, which is easy to mistake for the "stack buffer or malloc" convention used elsewhere in this header, where the caller has to free the result when it differs from the buffer it passed in. Say so explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
`Util::create_public_directory ()`, `Util::monodroid_fopen ()` and `Util::set_world_accessable ()` each took a `std::string_view` and immediately called `.data ()` on it to hand the path to `mkdir ()`, `fopen ()` or `chmod ()`. That is only correct because every caller happens to pass a view over a NUL-terminated buffer, which nothing enforces. Take a `const char*` instead, which is what these functions actually need. `Logger::open_file ()` and `Logger::init_reference_logging ()` follow, so `logger.cc` no longer refers to `std::string_view` at all and the `"..."sv` literals are gone with it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Address issues found while reviewing the C string conversion of the system property APIs. `AndroidSystem::lookup_system_property()` returned `prop_iter->first`, which is the map *key* -- the property name -- rather than its value, while reporting `prop_iter->second.length ()` as the length. In a DEBUG build with bundled `@(AndroidEnvironment)` properties, callers such as `Logger::init_logging_categories()` and `get_max_gref_count_from_system()` therefore parsed the property name instead of the value. Return `prop_iter->second` instead. `monodroid__system_property_get()` had a fallback branch that copied through a heap buffer whenever the caller's buffer was smaller than `PROPERTY_VALUE_BUFFER_LEN`. Its only caller now rejects that case before calling, so the branch was dead -- and it wrote a terminating NUL one byte past the end of the caller's buffer. Remove it, along with the now-unused `sp_value_len` parameter. This also drops a `new[]`/`delete[]` pair, removing two more libc++ references from the object file. Passing an undersized buffer to `monodroid_get_system_property()` was reported as `nullptr`, indistinguishable from an unset property. It is a programming error, so `abort_unless()` on it instead. Finally, derive the "gref="/"lref=" prefix length with `sizeof()` rather than hardcoding it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Use one helper to conditionally add the lib prefix and .so suffix for runtime DSO lookup and P/Invoke override loading. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Return formatted DSO and lookup-path lengths through caller-owned buffers, removing the remaining local strings from both normalization paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve the unbounded behavior of dynamic local strings without introducing libc++ ownership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Return the malloc-allocated joined path directly after releasing the temporary DSO name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Calculate complete DSO sizes first, use the sensible local buffer when possible, and allocate only larger names and paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Return the selected stack or heap buffer from DSO formatters and report the exact required capacity when local storage is too small. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rely on free(nullptr) and name stack-backed DSO storage explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use non-template DSO helpers and make callers provide each stack buffer capacity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove heap-buffer out parameters and free returned DSO strings only when they differ from their stack buffers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`get_full_dso_path` gained a second overload whose parameter list is identical to the existing one and differs only in its return type, which is not a valid overload. Rename the raw `ssize_t` variant to `format_full_dso_path` and leave the `char*` wrapper as the only `get_full_dso_path`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
`AndroidSystem` kept five of its members in `std::string`/`std::array<std::string>`: `primary_override_dir`, `native_libraries_dir`, `app_code_cache_dir`, `single_app_lib_directory` and `override_dirs`. Because they are `inline static` with dynamic initialization, the compiler emits a guard variable *and* an `atexit` registration for them in **every** translation unit that includes `android-system.hh` - even in ones that never touch them. `logger.cc`, `internal-pinvokes-clr.cc`, `internal-pinvokes-shared.cc` and `android-system-shared.cc` each paid four libc++ references (`~basic_string`, `operator delete`, `__cxa_guard_acquire`, `__cxa_guard_release`) without using a single one of these directories. Replace them with `path_buffer<N>`, a trivial aggregate holding an inline buffer plus an optional heap buffer. Being a POD, static instances are constant-initialized, so neither a guard variable nor an `atexit` registration is emitted. Paths that fit in `SENSIBLE_PATH_MAX` need no allocation at all and longer ones are moved to the heap, so - unlike the fixed `char[]` array NativeAOT used for `primary_override_dir` - there is no hard limit on the path length and no abort when it is exceeded. The directory arrays become plain `const char*` arrays whose entries are `malloc`ed, which also drops an `operator new[]` from the non-split-APK path. This lets `primary_override_dir` be shared by all three hosts, removing three `#if defined (XA_HOST_NATIVEAOT)` blocks and `determine_primary_override_dir()`. Undefined libc++ references in the CoreCLR archives: 58 -> 31. `libnet-android.release.so`: 539,464 -> 536,184 bytes (-3,280). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The inline-buffer-plus-heap-fallback `path_buffer` was more machinery than these three values need. They are assigned exactly once, early during startup, and only read afterwards, so the inline buffer only ever saved a single `malloc` per value while costing 3 KB of `.bss`. Replace it with plain `const char*` members initialized to `""`. Pointers to a string literal are constant-initialized just like the aggregate was, so the guard variables and `atexit` registrations stay gone, which was the whole point of the change. The values are duplicated with a new `Util::duplicate_string()` helper, which aborts if the allocation fails. Also format the APK library directory with `snprintf` instead of open-coded `memcpy` calls - the exact length is computed up front, so the buffer is already known to be the right size. Undefined libc++ references are unchanged at 31. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Addresses review feedback: - `app_lib_directories_size * sizeof (const char*)` is now computed with `Helpers::multiply_with_overflow_check`. - A zero-length array is handled explicitly. `malloc (0)` may legitimately return `nullptr`, which the previous code would have misreported as an allocation failure; `setup_apk_directories ()` already aborts with a more accurate message when no directory ends up being added. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The view returned by `get_string_view ()` pointed at the UTF characters owned by the wrapper, so it dangled as soon as the wrapper released them. Nothing relied on the view being a view: two of the three callers immediately passed it to a path helper, and the third only needed a suffix comparison. Return the C string instead and let the callers build a view when they need one. `setup_apk_directories ()` used `std::string_view::ends_with ()`, so add a `Util::ends_with ()` that works on plain C strings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The hand-written copy existed to support a caller that passed a pointer and a length rather than a C string, but that caller formats its buffer with `snprintf ()` and only reaches the call when the result fits, so the buffer is already NUL terminated. With every caller passing a C string there is nothing left for `std::string_view` to do and the copy is just `strdup ()`. Keep the wrapper rather than calling `strdup ()` directly: it aborts on allocation failure, which saves each of the four callers from checking for null. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The only caller of `get_full_dso_path ()` iterates over a container of `const char*` directories and wrapped each one in a `std::string_view` purely to satisfy the signature. Take a C string instead and measure it once inside `format_full_dso_path ()`. `dso_path` stays a view: it originates in the DSO cache lookup, which compares name mutations built with `substr ()`, so a view is the right type there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
…ader
This clears the remaining `libc++` references from `android-system.cc.o`,
bringing that object down from 5 to 0 and the CoreCLR total from 31 to 26.
None of the five references had anything to do with path handling:
* `std::binary_semaphore::try_acquire_for` pulled in
`std::chrono::steady_clock::now` and libc++'s timed backoff policy, and
`release` pulled in `__cxx_atomic_notify_all`. Replace it with a small
`BinarySemaphore` built directly on `pthread_mutex_t`/`pthread_cond_t`,
which is the primitive already used elsewhere in the tree. As a bonus it
waits on `CLOCK_MONOTONIC`, so the timeout is no longer affected by wall
clock adjustments.
* `MainThreadDsoLoader`'s destructor was `virtual` even though the class is
never derived from and only ever lives on the stack. That made the
compiler emit the deleting destructor, which references `operator delete`.
* `SystemLoadLibraryWrapper::load` created a `std::string` purely to get a
NUL-terminated copy of a `std::string_view`. Use a stack buffer with a
heap fallback instead, and split the actual loading into an overload
taking a `const char*` so there is a single place to free the copy.
`libnet-android.release.so` shrinks by 10,464 bytes (536,368 -> 525,904).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
- `SystemLoadLibraryWrapper::load ()` never released the local reference returned by `NewStringUTF ()`. This runs while loading the application's shared libraries, before control returns to Java, so nothing reclaims the references in between and the local reference table can fill up. Delete the reference once `CallStaticVoidMethod ()` returns. `DeleteLocalRef ()` is safe to call with a pending exception, so it can happen before the exception check. - `BinarySemaphore::try_acquire_for ()` ignored the return value of `clock_gettime ()`. On failure `deadline` stayed zero, which makes `pthread_cond_timedwait ()` return `ETIMEDOUT` right away and turns the wait into a silent spurious timeout. Abort instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The previous commit replaced `std::binary_semaphore` with a `BinarySemaphore` class built on `pthread_mutex_t`/`pthread_cond_t`. That was reimplementing a primitive libc already provides: `sem_t` from `<semaphore.h>` is a POSIX semaphore, lives in libc rather than libc++, and needs no wrapper at all. Delete the 99-line header and use `sem_init()`/`sem_post()`/`sem_timedwait()` directly. The only reason to prefer a condition variable here was that `sem_timedwait()` supports `CLOCK_REALTIME` only until API 28 (`sem_timedwait_monotonic_np()` is `__INTRODUCED_IN(28)` and `sem_clockwait()` is API 30, while we support API 24), so a wall clock adjustment inside the window can cut the 3s wait short or stretch it. For a sanity timeout on loading a shared library that is an acceptable trade for deleting a hand-written synchronization primitive. `sem_timedwait()` also takes an *absolute* deadline, so unlike the relative timeout it replaces, retrying after `EINTR` cannot extend the total wait, and the deadline needs no nanosecond normalization because the timeout is a whole number of seconds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The deadline arithmetic and the `EINTR` retry loop obscured what `load()` is actually doing. Move them into a small `try_acquire_for()` helper so the wait reads as a single line again, as it did when this was a `std::binary_semaphore`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
…rapper `jstring_array_wrapper::operator[]` fetches the element's JNI reference on first access, but the UTF characters behind it are only fetched later, when something calls `get_cstr ()`. `jstring_wrapper::release ()` bailed out early whenever `cstr` was null, so an element that was indexed but never read kept its local reference until control returned to Java. Release the characters and the reference independently instead. The overflow storage used `new jstring_wrapper[]`/`delete[]`, which is where `operator new[]` and `operator delete[]` entered `host.cc`. Allocate the array with `malloc ()` and run the constructors and destructors explicitly. Placement new is a compile-time construct, so it does not pull anything in from libc++. This removes `_Znam` and `_ZdaPv` from `host.cc`, taking the CoreCLR libc++ reference count from 23 down to 21. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Part of #12533. This clears `host.cc` entirely, taking the CoreCLR host from 21 undefined libc++ references to 12 - all of which now live in `assembly-store.cc`. Three separate uses, two of which only compile in Release: * `scan_filesystem_for_assemblies_and_libraries` built the assembly store path with a `std::string`. It now uses `Util::join_paths`, added earlier in this stack, which keeps the path on the stack unless it does not fit and frees only when the returned pointer differs from the stack buffer. * `APP_CONTEXT_BASE_DIRECTORY` was held in a function-local `static std::string`. The static needs process lifetime because the value has to outlive `coreclr_initialize`, and a `std::string` has a non-trivial constructor, so it forced a guard variable - this was the only source of `__cxa_guard_acquire`/`__cxa_guard_release` in the file. A plain `char*` is constant-initialized, so the guard pair disappears. * The FastDev block used a `std::string` plus two `std::vector<const char*>`. It is inside `if constexpr (Constants::is_debug_build)`, so it contributes nothing to Release, but it has to go before libc++ can be dropped from Debug builds too. The property arrays are a fixed `prop_count + 1` entries, so they become `calloc`ed arrays. `FastDevAssemblies::build_tpa_list` consequently returns a `malloc`ed string the caller owns rather than filling a `std::string&` out-parameter. The TPA list has no useful upper bound - one absolute path per assembly in the override directory - so it is accumulated through a small growable buffer that doubles on demand. Allocation failure there falls back to the probe-only path rather than aborting, since FastDev is a debug convenience. `open_assembly` swaps `new uint8_t[]`/`delete[]` for `malloc`/`free`. That buffer is handed to CoreCLR and never freed (see the existing TODO), so this is not a behavioural change. It also gains a null check, which fixes a file descriptor leak on the allocation-failure path that `new` would have turned into a `std::bad_alloc` abort. `<string>` and `<vector>` are no longer included by `host.cc`, and `<string>` is gone from `fastdev-assemblies.hh`, which `host.cc` was picking it up from. ### Results Undefined libc++ references in the CoreCLR host, Release: | object | before | after | |---|---:|---:| | `host.cc.o` | 9 | **0** | | `assembly-store.cc.o` | 12 | 12 | | **total** | **21** | **12** | `libnet-android.release.so` goes from 523,224 to 520,112 bytes (-3,112). Debug is not built locally, so `host.cc` and `fastdev-assemblies.cc` were compiled standalone with `-DDEBUG -DDEBUG_BUILD` using the flags from `compile_commands.json`. Both compile clean and report 0 undefined libc++ references. ### Verification * CoreCLR, MonoVM and NativeAOT all build clean. * Debug-mode compilation of both affected files verified as described above, which matters because `fastdev-assemblies.cc` is only added to the build under `if(DEBUG_BUILD)` and is therefore never compiled in Release. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Context: #12533 `assembly-store.cc` was the last file in the CoreCLR host referencing libc++. With it converted the host is at **zero** undefined libc++ references, which means the linker now pulls nothing out of `libc++_static.a` at all. The decompressed-assembly cache accounted for all of it: * `cache_dir` was a `std::string` built by repeated `append`. It is now a single `snprintf` into a stack buffer which the store ID is appended to in place, then `strdup`ed for the lifetime of the process. Both directory levels are still created and validated in turn. * `build_path` returned a `std::string` per call. It becomes `format_cache_path`, formatting into a caller-supplied buffer. * `WriteRequest` held a `std::string path` and a `std::unique_ptr<uint8_t[]> data`. The path is gone entirely - `cache_dir` is immutable once the cache is enabled, so the writer thread rebuilds the path from `descriptor_index` alone - and the payload now lives immediately after the structure, making a request and its bytes a single allocation instead of two. * `std::deque<WriteRequest>` becomes an intrusive FIFO threaded through `WriteRequest::next`. The queue is only ever pushed at the tail and popped at the head, so a singly linked list is a complete replacement; it also removes the per-request node allocation the deque made on top of the payload allocation. * `tracking` was a `std::unique_ptr<uint8_t*[]>` and `assembly_store_names` a `new std::string_view[]`. Both are now `calloc`/`free`. * The two `std::to_string` calls and the `.tmp.` scan in `remove_stale_temp_files` become `snprintf` and `strstr`. Both `snprintf`-formatted paths are bounded by `Util::LocalPathBufferSize` and checked for truncation, where the `std::string` versions grew without limit. A path that long could not be opened anyway, and the failure is logged and disables the cache rather than being silently ignored. Behaviour is otherwise unchanged. Allocation failure still disables the cache instead of taking the process down, except for `assembly_store_names`, which is required for correct assembly lookup and where `new[]` would previously have aborted anyway - exceptions are disabled, so its failure called `std::terminate`. ### Results Undefined libc++ references in the CoreCLR host, Release, arm64: | object | before | after | |---------------------|-------:|------:| | `assembly-store.cc.o` | 12 | **0** | | **host total** | **12** | **0** | | | before | after | delta | |-----------------------------|--------:|--------:|-----------:| | `libnet-android.release.so` | 520,112 | 203,776 | **-316,336 B** | The size drop is far larger than the source change because reaching zero references means the linker stops pulling members out of `libc++_static.a` entirely. ### Verification * CoreCLR, MonoVM and NativeAOT all build clean. * `llvm-nm --undefined-only` over every CoreCLR host object reports 0 libc++ references; the 12 present before this change are gone. * The resulting `.so` still exports `JNI_OnLoad` and the `Java_mono_android_Runtime_*` entry points, and its `DT_NEEDED` list is unchanged. * Relinking with `-nostdlib++` in place of `-static-libstdc++` succeeds, confirming that the host no longer needs libc++ at link time. Actually dropping the flag is left to a follow-up. Only `android-arm64` was built locally; the other ABIs rely on CI. The cache itself is exercised by the existing assembly store tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Now that the CoreCLR host has zero references to the C++ standard
library, we can stop linking it in entirely.
* Add a `coreclr-default-common` CMake preset that sets
`ANDROID_STL=none` (mirroring `nativeaot-default-common`, but
without the STL). This drops `-static-libstdc++` from the link
line and replaces it with `-nostdlib++`. Every CMake target
already adds `${SYSROOT_CXX_INCLUDE_DIR}` explicitly, so the
libc++ *headers* remain available.
* Stop building `common/libunwind` for CoreCLR. Only `mono/tracing`
links `xa::unwind`; the CoreCLR host built it and never used it.
* Stop shipping `libc++_static.a`, `libc++abi.a` and `libunwind.a`
in the CoreCLR runtime pack, and drop them from the archive list
used by `LinkNativeRuntime`. `KnownSets.CplusPlusRuntime` now only
holds the clang built-ins archive, so it is renamed to
`KnownSets.CompilerRuntime`.
Verified on `android-arm64` Release: the host builds and links
cleanly with `ANDROID_STL=none`, `libnet-android.release.so` is
unchanged at 203,776 bytes, has zero undefined libc++/libunwind
symbols, and its `DT_NEEDED` list is unchanged. MonoVM and NativeAOT
are unaffected.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
simonrozsival
added
the
drop-libcpp
simonrozsival
marked this pull request as ready for review
August 28, 2026 14:00
simonrozsival
changed the base branch from
dev/simonrozsival/clr-assembly-store-strings
to
dev/simonrozsival/fix-jstring-array-wrapper
August 28, 2026 14:01
simonrozsival
changed the base branch from
dev/simonrozsival/fix-jstring-array-wrapper
to
dev/simonrozsival/clr-assembly-store-strings
August 28, 2026 14:05
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/native/CMakePresets.json.in — 💡 Maintainability — coreclr-default-common duplicates ANDROID_CPP_FEATURES from… |
What changed in this PR
Removes the C++ standard library (libc++/libc++abi) and libunwind from the CoreCLR native host build and runtime packs now that the host no longer references the C++ standard library, while keeping compiler built-ins available for linking.
Changes:
- Stop shipping/linking libc++/libc++abi/libunwind for CoreCLR by pruning NDK redistributables and runtime pack asset lists.
- Introduce a CoreCLR-specific CMake preset baseline (
ANDROID_STL=none) and adjust CoreCLR presets to inherit from it. - Narrow
libunwindbuild to MonoVM-only and rename the native archive set fromCplusPlusRuntimetoCompilerRuntime(clang builtins).
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Utilities/NativeRuntimeComponents.cs | Renames the “c++” archive set to “compiler-rt” and removes libc++/libc++abi/libunwind archives from the known archive list. |
| src/Xamarin.Android.Build.Tasks/Tasks/LinkNativeRuntime.cs | Updates the set used for the “compiler support” archive group when ordering link inputs. |
| src/native/native.targets | Stops copying NDK “CplusPlus” redistributable archives into CoreCLR runtime packs. |
| src/native/CMakePresets.json.in | Adds coreclr-default-common preset with ANDROID_STL=none and updates CoreCLR presets to inherit from it. |
| src/native/CMakeLists.txt | Builds common/libunwind only for MonoVM (where it is actually linked). |
| build-tools/scripts/Ndk.targets | Removes libc++/libc++abi/libunwind from the NDK redistributable item list. |
| build-tools/scripts/Ndk.projitems | Removes now-unused UnwindArchDir metadata. |
| build-tools/create-packs/Microsoft.Android.Runtime.proj | Stops including NDK “CplusPlus” redistributables in the CoreCLR runtime pack assets. |
Comment on lines
+70
to
+74
| "inherits": "common", | ||
| "cacheVariables": { | ||
| "ANDROID_STL": "none", | ||
| "ANDROID_CPP_FEATURES": "no-rtti no-exceptions" | ||
| } |
NativeAOT already had zero references to the C++ standard library after #12523, so switching `nativeaot-default-common` to `ANDROID_STL=none` costs nothing and turns any future libc++ use into a link error instead of silently pulling the archive back in. Verified on `android-arm64` Release with a clean configure: all three NativeAOT artifacts -- `libnaot-android.release.so`, `libnaot-android.release-static-release.a` and `libxa-java-interop-release.a` -- are byte-for-byte identical to the `c++_static` build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
simonrozsival
changed the title
simonrozsival
marked this pull request as draft
September 3, 2026 06:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now that the CoreCLR host has zero references to the C++ standard library (#12571), we can stop linking it in entirely. This is the payoff PR for the CoreCLR half of #12533.
Changes
Stop linking libc++. A new hidden
coreclr-default-commonCMake preset setsANDROID_STL=none. All sixcoreclr-default-{debug,release}-{armeabi-v7a,arm64-v8a,x86_64}presets now inherit from it instead of the shareddefault-common(which MonoVM still uses).This drops
-static-libstdc++from the link line and replaces it with-nostdlib++. The libc++ headers remain available: every CMake target already lists${SYSROOT_CXX_INCLUDE_DIR}in its include directories, so CMake now emits those-isystemflags explicitly instead of relying on the toolchain's implicit search path.NativeAOT gets the same treatment. NativeAOT reached zero libc++ references back in #12523, so
nativeaot-default-commonswitches toANDROID_STL=noneas well.Stop building libunwind for CoreCLR.
add_subdirectory(common/libunwind)ran forIS_MONO_RUNTIME OR IS_CLR_RUNTIME, but onlymono/tracingever linksxa::unwind. The CoreCLR host built the whole thing and never used it.Stop shipping the archives.
libc++_static.a,libc++abi.aandlibunwind.aare no longer copied into the CoreCLR runtime pack, and are removed from the archive list thatLinkNativeRuntimeconsumes.KnownSets.CplusPlusRuntimenow only holds the clang built-ins archive, so it is renamed toKnownSets.CompilerRuntime("compiler-rt").MonoVM is untouched and keeps
ANDROID_STL=c++_static.Why this matters beyond the size win
The 316 KB the CoreCLR host lost in #12571 came from reaching zero references, at which point the linker stops pulling members out of
libc++_static.a. That state is fragile: a singlestd::stringadded later silently pulls the archive back in and the win evaporates.With
-nostdlib++, that regression becomes a link error instead.A note on
LinkNativeRuntimeLinkNativeRuntime(the "link the app's native runtime statically" feature, gated on_AndroidEnableNativeRuntimeLinking) also linkslibcoreclr_static.a,libbrotli*.aand the BCL PAL archives from dotnet/runtime. Those are C++ and very likely still need libc++.That property is not set anywhere in the repo today, so the feature is dormant. Removing the archives now keeps the two code paths honest about what our own runtime needs; if the feature is ever turned on, libc++ can be reintroduced there specifically rather than being linked into everything by default.
Verification
android-arm64Release, clean CMake configures atANDROID_STL=none:CoreCLR
libunwinddoes not appear in the build graph.libnet-android.release.sois unchanged at 203,776 bytes.libc++/libc++abi/_Unwind_*symbols.DT_NEEDEDunchanged;JNI_OnLoadand theJava_mono_android_Runtime_*entry points still exported.NativeAOT
libnaot-android.release.so,libnaot-android.release-static-release.aandlibxa-java-interop-release.a-- are byte-for-byte identical (matching SHA-1) to thec++_staticbuild.MonoVM still builds, and still builds libunwind 1.8.1.