Skip to content

[native] Remove the libc++ dependencies from the CoreCLR host's startup path - #12570

Merged
simonrozsival merged 17 commits into
mainfrom
dev/simonrozsival/clr-host-drop-strings
Sep 5, 2026
Merged

simonrozsival merged 17 commits into
mainfrom
dev/simonrozsival/clr-host-drop-strings

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Aug 28, 2026 •

Copy link
Copy Markdown
Member

PR relationship: Depends on #12552 (and transitively #12551). This is the top of stack #12654. The absolute symbol and size totals below were measured in the former serialized stack and include the parallel prerequisite work.

Part of #12533 (drop libc++ from the CoreCLR host).

This clears host.cc entirely. The CoreCLR host goes from 21 undefined libc++ references to 12, and every one that remains is now in a single file, assembly-store.cc.

The three uses in host.cc

1. The assembly store path

scan_filesystem_for_assemblies_and_libraries joined the native library directory and the store filename with a std::string. It now uses Util::join_paths, added earlier in this stack, which keeps the path in a stack buffer unless it does not fit and frees only when the returned pointer differs from that buffer.

2. APP_CONTEXT_BASE_DIRECTORY

static std::string app_context_base_directory;

The static is needed because the value has to outlive coreclr_initialize — CoreCLR keeps the pointer. But std::string has a non-trivial constructor, so a function-local static of that type forces a guard variable, and 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 along with the string. Runtime initialization is process-global, so replacing the stored value cannot invalidate a live CLR instance.

3. The FastDev TPA list

static std::string fastdev_tpa_list;
static std::vector<const char*> fastdev_prop_names;
static std::vector<const char*> fastdev_prop_values;

This block is inside if constexpr (Constants::is_debug_build), so it contributes nothing to the Release numbers below — but it has to go before libc++ can be dropped from Debug builds, which is why it is included here rather than left for later. The two property arrays are a fixed prop_count + 1 entries, so they become calloced arrays.

build_tpa_list now returns the string

It previously filled a std::string& out-parameter and returned bool; it now returns a malloced string the caller owns, or nullptr.

The TPA list is the one place in this PR with no useful upper bound — it holds one absolute path per assembly in the override directory — so a fixed buffer is not appropriate. It is accumulated through a small growable buffer that doubles on demand.

FastDev allocation failures are non-fatal: both list construction and extended runtime-property allocation log a warning, free partial state, reset TPA mode and fall back to the probe-only path. discard_tpa_list() owns that reset and has a no-op Release stub, avoiding references to Debug-only state from the discarded branch of the non-template if constexpr.

open_assembly

new uint8_t[]/delete[] become malloc/free. That buffer is handed to CoreCLR and never freed — see the existing TODO above it about ownership — so this is not a behavioral change.

It also gains a null check. That is a small bug fix: new uint8_t[] would have thrown std::bad_alloc and aborted, whereas the new path logs, closes the file descriptor and returns nullptr. The previous code would have leaked asm_fd had it ever returned rather than aborted.

Environment override buffer

The Debug environment override reader used std::make_unique<char[]>. It now uses malloc/free, preserves the previous non-fatal behavior for empty files without relying on malloc(0), and closes the override file descriptor after reading. Read errors retain their errno value across close() so diagnostics remain accurate.

Header cleanup

<string> and <vector> are no longer included by host.cc, and <string> is gone from fastdev-assemblies.hh — which is where host.cc was picking it up from in the first place.

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

The nine removed from host.cc.o: __cxa_guard_acquire, __cxa_guard_release, operator new, operator delete, std::string::append, std::string::push_back, std::string::__grow_by_and_replace, ~basic_string and __libcpp_verbose_abort.

before after delta
libnet-android.release.so 523,224 520,112 −3,112 B

Verification

  • CoreCLR, MonoVM and NativeAOT all build clean.
  • The final review fixes were validated with dotnet build src/native/native-clr.csproj --no-restore.
  • Debug was verified explicitly. fastdev-assemblies.cc is only added to the build under if(DEBUG_BUILD) (src/native/clr/host/CMakeLists.txt), so a Release build never compiles it and would not have caught a mistake in the FastDev work. There is no Debug CoreCLR build configured locally, so both host.cc and fastdev-assemblies.cc were compiled standalone with -DDEBUG -DDEBUG_BUILD using the exact flags from compile_commands.json. Both compile clean and llvm-nm --undefined-only reports 0 libc++ references for each.
  • The Debug/Release API shape around discard_tpa_list() was compiled explicitly both with and without -DDEBUG.

Not covered

Only android-arm64 was built locally; the other ABIs rely on CI. The FastDev path is Debug-only and was not exercised on a device as part of this change.

Copilot AI lite review requested due to automatic review settings August 28, 2026 13:12
@simonrozsival simonrozsival changed the title [native] Remove the libc++ dependencies from the CoreCLR host [native] Remove the libc++ dependencies from the CoreCLR host's startup path Aug 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Lite
Findings: 2 Medium severity

New issues introduced by this change (2)
Severity Finding
Medium severity src/​native/​clr/​host/​fastdev-assemblies.cc — ERROR: FastDevAssemblies::tpa_in_use is set to true on success but never reset on failure. If…
Medium severity src/​native/​clr/​host/​host.cc — WARNING: This Debug-only FastDev path aborts the process if allocating the extended runtime…
What changed in this PR

This PR refactors the CoreCLR native host FastDev/initialization code to remove dependencies on libc++/STL types (e.g., std::string, std::vector) by switching to C allocation patterns and existing in-tree path helpers.

Changes:

  • Change FastDev TPA list construction to return a malloc-owned char* instead of filling a std::string.
  • Replace STL-based path/property list handling in host.cc with Util::join_paths, malloc/calloc, and explicit lifetime management.
  • Update FastDev assembly loading buffer allocation to use malloc/free instead of new[]/delete[].
File Description
src/​native/​clr/​include/​host/​fastdev-assemblies.hh Updates the FastDev API to return an owned char* TPA list (no STL types).
src/​native/​clr/​host/​host.cc Reworks CoreCLR init property handling and path building to avoid STL and manage memory explicitly.
src/​native/​clr/​host/​fastdev-assemblies.cc Implements a growable C buffer for TPA list building and switches FastDev file buffer allocation to malloc.

Comment thread src/native/clr/host/fastdev-assemblies.cc
Comment thread src/native/clr/host/host.cc Outdated
@simonrozsival simonrozsival added the drop-libcpp Work to remove the libc++ dependency from Android NativeAOT label Aug 28, 2026
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/clr-host-drop-strings branch from add6e9c to 6e8a14b Compare September 3, 2026 06:15
@simonrozsival
simonrozsival changed the base branch from dev/simonrozsival/fix-jstring-array-wrapper to dev/simonrozsival/clr-android-system-paths September 3, 2026 06:16
@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12570

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Needs Changes — 0 errors, 0 warnings, 1 suggestion. The raw-allocation ownership and cleanup paths are consistent, and the existing StackTraceContainsLineNumbers device test exercises the FastDev TPA behavior. The only feedback is a non-blocking utility-consolidation suggestion.

CI is not yet green: only license/cla has completed successfully, while no dotnet-android Azure DevOps checks are present for head 91690f72; the native ABI matrix therefore remains unvalidated.

Generated by Android PR Reviewer for #12570 · gpt56 · 99.4 AIC · ⌖ 9.54 AIC · ⊞ 25.7K
Comment /review to run again

Comment thread src/native/clr/host/host.cc Outdated
simonrozsival and others added 16 commits September 3, 2026 15:11
`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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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.

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.

* 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 <[email protected]>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Reset FastDev TPA state on every rebuild and keep runtime property allocation failures on the probe-only path.

Co-authored-by: Copilot App <[email protected]>
`FastDevAssemblies::tpa_in_use` only exists under `#if defined(DEBUG)`, but
host.cc referenced it from an `if constexpr (Constants::is_debug_build)` branch.
The discarded branch of an `if constexpr` in a non-template function is still
name-looked-up, so Release builds failed with "no member named 'tpa_in_use'".

Route the reset through a new `discard_tpa_list ()` which, like `open_assembly ()`
and `build_tpa_list ()`, has a no-op Release stub.

Co-authored-by: Copilot App <[email protected]>
Preserve empty-file handling without relying on malloc(0), close the override file descriptor after reading, and document the process-global lifetime assumed by the CoreCLR application base directory.

Co-authored-by: Copilot App <[email protected]>
Force the existing path helper onto its heap-owned path so separator handling stays centralized while the CoreCLR property retains explicit process-lifetime ownership.

Co-authored-by: Copilot App <[email protected]>
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/clr-host-drop-strings branch from c968434 to 45e3d4e Compare September 3, 2026 13:11
@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Sep 3, 2026
Base automatically changed from dev/simonrozsival/clr-android-system-paths to main September 4, 2026 22:19
@simonrozsival

Copy link
Copy Markdown
Member Author

@jonathanpeppers please re-approve

@simonrozsival
simonrozsival merged commit f74e1e8 into main Sep 5, 2026
44 checks passed
@simonrozsival
simonrozsival deleted the dev/simonrozsival/clr-host-drop-strings branch September 5, 2026 21:31
simonrozsival added a commit that referenced this pull request Sep 8, 2026
…12571)

Part of #12533: remove the remaining allocating C++ library types from the CoreCLR assembly store and its decompressed-assembly cache.

<!-- stack-prerequisites -->
All prerequisites (#12541, #12545, #12551, #12552, #12560, #12568, and #12570) have merged. This branch was rebuilt on `main` at `4b0cc8efa9`. The runtime changes are confined to `src/native/clr/host/assembly-store.cc`, with cache regression coverage and APK size baseline updates alongside them. The already-merged startup, timing, DSO-loader, and other prerequisite changes are no longer included in this PR's diff. Upstream's pthread-backed `lock_guard` is preserved.
<!-- /stack-prerequisites -->

### Changes

- Replace `std::deque<WriteRequest>` with an intrusive FIFO linked through `WriteRequest::next`.
- Replace the queued request's `std::unique_ptr<uint8_t[]>` with an explicit `uint8_t *payload`. Allocate the request and payload separately with two `malloc` calls and release both with two `free` calls on every completed or discarded write. If payload allocation fails, release the request and skip the write. No trailing-storage pointer arithmetic or explicit over-alignment is needed.
- Replace the cache directory and per-request `std::string` paths with a process-lifetime directory string and checked `snprintf` formatting. Requests store the descriptor index instead of a path; the writer reconstructs the destination from the immutable cache directory. Replace temporary-file name construction and matching with `snprintf` and `strstr`.
- Use a scoped `CachePath` buffer for directory creation, reads, writes, and stale-file cleanup. Short paths remain on the stack; longer paths retry in an exactly sized `malloc` allocation that is freed on every exit. Formatting and allocation failures are logged and remain non-fatal for the optional cache, unlike the abort-on-failure `Util::format_with_retry` helper.
- Allocate the decompression tracking array and assembly-name table with `calloc`, replacing their `unique_ptr`/`new[]` allocations.
- Extend the existing device cache regression test with a Java Application that supplies a code-cache directory longer than 1 KB. Cover persistence, mapping, corruption recovery, and stale temporary-file cleanup for both ordinary and long paths.

The asynchronous writer, queue byte limit, cache footer validation, and scope-based pthread locking are retained. `Util::LocalPathBufferSize` is the stack fast-path size, not a hard path limit; truncated paths are never used for I/O. The change does not alter linker flags or other runtime hosts.

### Original measurements

These results were recorded on the original pre-rebuild branch, **not on the current revision**:

| Release, arm64 | Before | After |
|---|---:|---:|
| Undefined libc++ references in `assembly-store.cc.o` | 12 | 0 |
| Undefined libc++ references across the CoreCLR host | 12 | 0 |
| `libnet-android.release.so` size | 520,112 bytes | 203,776 bytes |

Reaching zero references allowed the linker to stop pulling members from `libc++_static.a`. Relinking without libc++ also succeeded on that original branch; dropping the linker flag remains a follow-up.

### Current validation status

The malloc fallback passed sanitizer-backed host cases covering stack/heap boundaries, nested lifetimes, allocation failure, and formatting/retry failures. The cache namespace extracted unchanged from the current source, using the real CRC32 and mutex helpers with runtime configuration/property stubs, compiled with the Android NDK for arm64, arm32, and x86_64. A standalone arm64 emulator harness passed cache initialization, asynchronous persistence, mmap reads, corruption recovery, and stale-file cleanup with both short and greater-than-1-KB paths. The generated Java regression-test application also compiled against the Android API. `git diff --check` passes.

A local `dotnet build src/native/native-clr.csproj` restored packages but could not reach native compilation because this worktree lacks `xa-prep-tasks.dll` and `Xamarin.Android.Tools.BootstrapTasks.dll`. The complete native host build and repository device regression suite have **not** been run for this revision; the locally built SDK is unavailable. Binary measurements have not been repeated. The original branch's reported full native builds and binary measurements remain historical, not validation of this revision.
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

drop-libcpp Work to remove the libc++ dependency from Android NativeAOT ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants