[native] Remove the last libc++ dependencies from the CoreCLR host - #12571
Open
simonrozsival wants to merge 1 commit into
Open
Conversation
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
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/clr/host/assembly-store.cc — format_cache_path() currently fails silently. If cache_dir is near… |
What changed in this PR
This PR continues the CoreCLR-host libc++ removal work by refactoring assembly-store.cc to eliminate remaining std::string/std::deque/std::unique_ptr usage in the decompressed-assembly cache path and write-queue implementation, bringing the host to zero undefined libc++ references.
Changes:
- Replaces
std::deque<WriteRequest>with an intrusive FIFO (WriteRequest::next) and moves request payload inline (single allocation per request). - Converts cache path construction from
std::stringbuilding to boundedsnprintfintoUtil::LocalPathBufferSizestack buffers. - Replaces
new[]-allocated tracking/name tables withcalloc/free, and makescache_dirastrdup-owned immutable C string.
| File | Description |
|---|---|
| src/native/clr/host/assembly-store.cc | Removes remaining libc++-driven types from the decompressed-assembly cache (paths, write queue, tracking/name tables) using C allocation + fixed buffers. |
Comment on lines
+155
to
+159
| auto format_cache_path (char *buffer, size_t buffer_size, uint32_t descriptor_index) noexcept -> bool | ||
| { | ||
| std::string tmp_path = req.path; | ||
| tmp_path.append (".tmp."sv); | ||
| tmp_path.append (std::to_string (getpid ())); | ||
| int length = snprintf (buffer, buffer_size, "%s/%u.bin", cache_dir, descriptor_index); | ||
| return length > 0 && static_cast<size_t>(length) < buffer_size; | ||
| } |
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.

Part of #12533 (drop
libc++from the CoreCLR host). Stacked on top of #12570.Context: #12533
assembly-store.ccwas the last file in the CoreCLR host referencinglibc++. With it converted the host is at zero undefined libc++
references, which means the linker now pulls nothing out of
libc++_static.aat all.The decompressed-assembly cache accounted for all of it:
cache_dirwas astd::stringbuilt by repeatedappend. It is nowa single
snprintfinto a stack buffer which the store ID isappended to in place, then
strduped for the lifetime of theprocess. Both directory levels are still created and validated in
turn.
build_pathreturned astd::stringper call. It becomesformat_cache_path, formatting into a caller-supplied buffer.WriteRequestheld astd::string pathand astd::unique_ptr<uint8_t[]> data. The path is gone entirely -cache_diris immutable once the cache is enabled, so the writerthread rebuilds the path from
descriptor_indexalone - and thepayload 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 threadedthrough
WriteRequest::next. The queue is only ever pushed at thetail 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.
trackingwas astd::unique_ptr<uint8_t*[]>andassembly_store_namesanew std::string_view[]. Both are nowcalloc/free.The two
std::to_stringcalls and the.tmp.scan inremove_stale_temp_filesbecomesnprintfandstrstr.Both
snprintf-formatted paths are bounded byUtil::LocalPathBufferSizeand checked for truncation, where the
std::stringversions grew withoutlimit. 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 andwhere
new[]would previously have aborted anyway - exceptions aredisabled, so its failure called
std::terminate.Results
Undefined libc++ references in the CoreCLR host, Release, arm64:
assembly-store.cc.olibnet-android.release.soThe size drop is far larger than the source change because reaching zero
references means the linker stops pulling members out of
libc++_static.aentirely.Verification
llvm-nm --undefined-onlyover every CoreCLR host object reports 0libc++ references; the 12 present before this change are gone.
.sostill exportsJNI_OnLoadand theJava_mono_android_Runtime_*entry points, and itsDT_NEEDEDlistis unchanged.
-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-arm64was built locally; the other ABIs rely on CI. Thecache itself is exercised by the existing assembly store tests.