Skip to content

Replace simple CLR local strings - #12517

Open
simonrozsival wants to merge 13 commits into
mainfrom
dev/simonrozsival/replace-simple-local-strings
Open

Replace simple CLR local strings#12517
simonrozsival wants to merge 13 commits into
mainfrom
dev/simonrozsival/replace-simple-local-strings

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Aug 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Remove simple CLR local strings while preserving dynamic growth where the old local-string type supported it.

Changes

  • make debug typemap-name and CoreCLR primary-override-path formatters report the negative required capacity and retry with exact-size malloc() storage
  • keep common values in sensible stack buffers and free returned storage only when it differs from the stack buffer
  • avoid separate heap out-parameters and cleanup helpers
  • replace CLR system-property local strings with explicit (buffer, buffer_size) calls using the platform property bound
  • parse logging and timing properties without dynamically growing local strings
  • remove the CLR monodroid_get_system_property(..., dynamic_local_property_string&) overload
  • keep NativeAOT's pre-existing primary-override array intentionally bounded

Validation

  • Local native builds intentionally skipped; relying on CI validation

Copilot AI lite review requested due to automatic review settings August 25, 2026 13:51
Copilot AI reviewed Aug 25, 2026
View reviewed changes

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.

Pull request overview

This PR continues the effort to remove straightforward uses of the native local-string hierarchy in the CLR host/runtime by switching to fixed-size buffers (with snprintf) and by using a fixed-buffer system property retrieval API that rejects undersized buffers to avoid truncation.

Changes:

  • Added a monodroid_get_system_property(std::string_view, char*, size_t) overload and updated fixed-array callers to write directly into their buffers.
  • Reworked primary override directory path formatting to use snprintf into fixed buffers.
  • Replaced debug typemap full-name composition with fixed-buffer snprintf formatting.
Show a summary per file
File Description
src/native/clr/runtime-base/android-system-shared.cc Adds fixed-buffer monodroid_get_system_property overload that rejects undersized buffers and avoids truncation.
src/native/clr/include/runtime-base/android-system.hh Switches property callers to fixed buffers and formats primary override dir using snprintf.
src/native/clr/host/typemap.cc Uses snprintf into a fixed buffer to build managed type debug names for typemap lookup.
src/native/clr/host/assembly-store.cc Uses a fixed property buffer for debug.net.asmcache instead of dynamic_local_property_string.

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/native/clr/host/typemap.cc Outdated
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from c111658 to 3a05311 Compare August 25, 2026 13:57
@simonrozsival
simonrozsival changed the base branch from dev/simonrozsival/remove-timing-dynamic-strings to main August 25, 2026 13:58
@simonrozsival simonrozsival added the drop-libcpp Work to remove the libc++ dependency from Android NativeAOT label Aug 25, 2026
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from 2a03078 to cda8d20 Compare August 25, 2026 15:08
@simonrozsival
simonrozsival changed the base branch from main to dev/simonrozsival/remove-timing-dynamic-strings August 25, 2026 15:10
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from cda8d20 to 2e9c443 Compare August 25, 2026 15:22
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch 2 times, most recently from 6d947da to ddae8f9 Compare August 25, 2026 15:42
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from ddae8f9 to 4478d1c Compare August 25, 2026 15:51
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from 4478d1c to ce12492 Compare August 25, 2026 15:59
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from ce12492 to 85f0111 Compare August 25, 2026 18:53
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from 85f0111 to 8c8f213 Compare August 25, 2026 21:27
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch 2 times, most recently from 4a079b8 to fb358cf Compare August 25, 2026 21:41
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from fb358cf to ea5dfcb Compare August 25, 2026 21:51
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from ea5dfcb to bec9cc5 Compare August 25, 2026 22:23
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from bec9cc5 to aea8dce Compare August 26, 2026 07:34
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch 2 times, most recently from 77b88ac to 64ae7e8 Compare August 27, 2026 06:25
@simonrozsival simonrozsival mentioned this pull request Aug 27, 2026
Open
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/replace-simple-local-strings branch from 64ae7e8 to 968857f Compare August 27, 2026 10:21
Base automatically changed from dev/simonrozsival/remove-timing-dynamic-strings to main August 27, 2026 14:25
simonrozsival and others added 12 commits August 27, 2026 09:25
@simonrozsival @jonathanpeppers
Replace simple CLR local strings
64f53d1 
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>
@simonrozsival @jonathanpeppers
Remove CLR dynamic property strings
fa4f4f9 
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>
@simonrozsival @jonathanpeppers
Narrow the logger strings include
72b86d4 
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>
@simonrozsival @jonathanpeppers
Preserve unbounded generated names
962c572 
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>
@simonrozsival @jonathanpeppers
Use C allocation for generated names
23ac7c9 
Keep exact-size type and timing strings independent of libc++ ownership.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival @jonathanpeppers
Allocate override paths to their exact size
48a37eb 
Keep the existing NativeAOT fixed-storage limit while preserving unbounded CoreCLR path construction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival @jonathanpeppers
Keep short generated names on the stack
56b79d1 
Use malloc only when typemap or override names exceed their sensible local buffer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival @jonathanpeppers
Retry formatting oversized CLR strings
931f293 
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>
@simonrozsival @jonathanpeppers
Use shared CLR buffer cleanup
21a8325 
Route managed type and override path heap-buffer cleanup through Util.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival @jonathanpeppers
Use free directly for CLR buffers
833f4aa 
Rely on free(nullptr) and name stack-backed CLR string storage explicitly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival @jonathanpeppers
Use CLR pointers for buffer ownership
6ee0f1d 
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>
@simonrozsival @jonathanpeppers
Restore includes that strings.hh used to provide
a7e70fe 
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
@jonathanpeppers
jonathanpeppers force-pushed the dev/simonrozsival/replace-simple-local-strings branch from 968857f to a7e70fe Compare August 27, 2026 14:25
@simonrozsival
Include strings.hh directly in util.cc
114f986 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@jonathanpeppers jonathanpeppers Awaiting requested review from jonathanpeppers jonathanpeppers is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

drop-libcpp Work to remove the libc++ dependency from Android NativeAOT

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@simonrozsival