[NativeAOT] Migrate runtime utility logging to printf#12148
[NativeAOT] Migrate runtime utility logging to printf#12148simonrozsival wants to merge 21 commits into
Conversation
Introduce printf-style native logging and abort helpers while preserving the existing std::format APIs for MonoVM and CoreCLR. Migrate the NativeAOT-specific formatted call sites to the new primitives. Refs #12139 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 187b207a-083b-461e-9071-e9aab61c9d07
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Keep the printf logging PR focused on the runtime implementation instead of introducing new host-native logging test infrastructure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Convert NativeAOT-reachable environment, file, mmap, and shared-host diagnostics to the printf helpers introduced by #12140. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
There was a problem hiding this comment.
Pull request overview
This PR continues the NativeAOT libc++ reduction work by migrating remaining formatted logging in NativeAOT-reachable shared runtime utilities away from std::format/{}-style logging and onto the printf-checked log_*f() and Helpers::abort_applicationf() APIs introduced in the stacked dependency PR #12140.
Changes:
- Convert runtime utility and environment diagnostics to
log_*f()with bounded%.*sforstd::string_viewand null-safeoptional_string()forconst char*. - Replace a fatal
mmap()failure path’sstd::formatallocation withHelpers::abort_applicationf()and migrate detailed mmap diagnostics to printf formatting. - Migrate Android system-property and shared host (JNI class-name lookup) diagnostics to
log_*f()with correct format specifiers.
Show a summary per file
| File | Description |
|---|---|
| src/native/clr/runtime-base/util.cc | Migrates directory creation, fopen, and chmod/fchmod error logging to log_*f(). |
| src/native/clr/runtime-base/android-system-shared.cc | Migrates system property warnings and max-gref override diagnostics to log_warnf() with correct size_t/long formats. |
| src/native/clr/include/runtime-base/util.hh | Migrates env-var and mmap-related diagnostics to log_*f() / abort_applicationf(), using bounded %.*s formatting. |
| src/native/clr/host/host-shared.cc | Migrates Java class-name lookup failures to log_errorf() (including %p). |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 0
Convert host environment and integer parsing diagnostics to #12140 printf helpers while preserving MonoVM formatting branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
## Summary Introduce printf-style native logging primitives and migrate the initial NativeAOT formatted logging call sites to them. This is the first implementation step from #12139 toward removing the Android NativeAOT dependency on libc++. ## Changes - add `log_writev()`, `log_writef()`, `log_debugf()`, `log_infof()`, `log_warnf()`, and `log_errorf()` alongside the existing `std::format` APIs; - implement the same printf helpers in the CLR and MonoVM shared logging backends so shared runtime code can use one logging path; - add `Helpers::abort_applicationf()` for formatted fatal messages without truncating tombstone text; - include `<cstdio>` explicitly for `vasprintf()`; - omit an unused `log_fatalf()` wrapper; formatted fatal termination uses `abort_applicationf()` instead; - replace the NativeAOT GC-user-peer initialization error's `std::format`; - replace the NativeAOT JNI on-load debug message's `{}` formatting; - avoid `std::format` inside `Helpers::abort_application()`. The existing `std::format` APIs remain available so unrelated call sites can be migrated incrementally. The current stacked migrations are #12148 (shared runtime utilities), #12150 (JNI reference logging), #12153 (timing logging), and #12155 (configuration diagnostics). ## Runtime behavior - NativeAOT and CoreCLR use the CLR shared logging implementation. - MonoVM uses its own shared logging implementation with the same printf helper surface. - category filtering and Android log priorities remain unchanged. - null format pointers are logged as `<null>` rather than passed to `__android_log_vprint()`. ## Validation - `git diff --check`; - Release build of `src/native/native-nativeaot.csproj`; - Release build of `src/native/native-clr.csproj`; - NDK Clang C++23 syntax compilation of the CLR helper changes across NativeAOT/CoreCLR configurations; - NDK Clang C++23 syntax compilation of the MonoVM printf implementation in 8 Android configurations; - focused review of `va_list`, category filtering, format checking, source-location behavior, and Android log-priority mapping; - latest head: `09f0690da`. Part of #12139.
## Summary Continue the printf-style logging migration introduced by #12140 in the shared native timing infrastructure. The timing headers are used by MonoVM, CoreCLR, and NativeAOT. All three runtimes now use the same printf-style path for variable timing diagnostics rather than maintaining MonoVM-only `std::format` branches. **Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only the timing migration. Part of #12139. ## Scope Only two shared timing headers change: - `src/native/common/include/runtime-base/timing.hh` - `src/native/common/include/runtime-base/timing-internal.hh` These files do not overlap #12141, #12142, #12145, #12148, #12150, or #12155. ## Changes ### Managed timing records - replace the owning `std::format` result in `Timing::do_log()` with `log_writef()` for every runtime; - preserve the exact `message; elapsed: seconds:milliseconds::nanoseconds` field order; - pass duration values as explicitly converted `unsigned long long` values matching `%llu`. ### Fast timing diagnostics Migrate variable diagnostics to `log_warnf()` for every runtime: - timing event buffer reallocation sizes; - `CLOCK_MONOTONIC_RAW` errors; - unknown event-kind values; - invalid event-index source method names. Constant warning messages continue using the existing non-formatting `string_view` overload. ## Behavior preserved - timing enablement and category gating are unchanged; - timing sequence acquisition/release is unchanged; - elapsed duration units, values, and output order are unchanged; - event-buffer growth, clock error handling, unknown-event fallback, and index validation are unchanged; - no timing data structures, locks, vectors, strings, or event lifecycle code change. ## Runtime behavior | Runtime | Result | |---|---| | NativeAOT | Uses #12140 printf helpers for variable timing diagnostics | | CoreCLR | Uses #12140 printf helpers for variable timing diagnostics | | MonoVM | Uses the matching MonoVM printf helper implementation from #12140 | ## Measured impact Representative Android arm64 Release objects compiled before the MonoVM path was unified: | Runtime/object | Before | After | Difference | |---|---:|---:|---:| | CoreCLR `internal-pinvokes-clr.cc.o` | 145,472 B | 17,080 B | -128,392 B (-88.26%) | | NativeAOT `host.cc.o` | 176,104 B | 175,960 B | -144 B | CoreCLR's representative object drops from 58 formatting symbols to zero. NativeAOT's representative object still contains formatting symbols from other included functionality, but the timing call sites migrated here no longer instantiate them. ## Non-goals - no new timing tests or logging test framework; - no change to timing data ownership or synchronization; - no changes to the logging helper implementation introduced by #12140; - no attempt to migrate unrelated timing output/file-generation code outside these shared timing headers. ## Validation - `git diff --check`; - NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants; - NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants; - NDK Clang C++23 syntax compilation of all 31 Android arm64 Release MonoVM source variants; - representative before/after object compilation, size comparison, and symbol inspection; - focused review of chrono values, integer format widths, source-location output, and category semantics; - latest head: `21ff66b26`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Continue the printf-style logging migration introduced by #12140 by converting shared CoreCLR/NativeAOT JNI reference logging in `OSBridge`. The `src/native/clr/` location is shared infrastructure: the NativeAOT host explicitly compiles `clr/host/os-bridge.cc`, while MonoVM uses its separate `mono/monodroid/osbridge.cc` implementation. This PR therefore affects CoreCLR and NativeAOT, but not MonoVM. **Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only this migration. Part of #12139. ## Scope Only two files change: - `src/native/clr/host/os-bridge.cc` - `src/native/clr/include/host/os-bridge.hh` This does not overlap the open sibling work: - #12141 changes `gc-bridge.cc/.hh` — worker-thread and semaphore handoff; - #12142 changes logging/path ownership and source-location parsing elsewhere; - #12145 changes `bridge-processing.cc/.hh` — temporary-peer graph storage; - #12148 changes shared host/runtime utility logging. ## Changes ### Format reference records once - add a private printf-checked `OSBridge::log_itf()` helper; - use `vasprintf()` to produce one NUL-terminated reference-log line; - pass the same formatted line to the existing logcat and file-writing path; - release the C allocation after both destinations have consumed it; - on allocation failure, fall back to the static format string rather than throwing from a `noexcept` `std::format` path. ### Migrate JNI reference events Replace six owning `std::format` constructions: - new global reference; - deleted global reference; - new weak global reference; - deleted weak global reference; - new local reference; - deleted local reference. The replacements use `%d`, `%p`, `%c`, and `%s` with compiler format checking from the private helper declaration. ### Remove ranges-heavy stack-trace splitting - replace `std::views::split` with an in-place `std::string_view` newline scan; - preserve empty, consecutive, and trailing lines; - preserve logcat-only, file-only, and combined output behavior; - write non-NUL-terminated line views with explicit lengths; - convert stack-trace and raw gref logcat forwarding to `%.*s` / `%s`. ## Responsibility boundary with #12141 - `GCBridge` (#12141) controls **when and where** a bridge round runs: callback handoff, worker thread, semaphore synchronization, and Java GC triggering. - `OSBridge` (this PR) tracks and logs **JNI reference events**: gref/weak-gref counters, reference creation/deletion lines, and optional stack traces. The classes call each other through existing stable helpers, but this PR changes no GC scheduling, graph processing, callback lifecycle, or synchronization state. ## Behavior preserved - reference counters are incremented/decremented at the same points; - logging remains gated by `LOG_GREF` / `LOG_LREF` before formatting; - each reference line is still emitted to logcat and, when configured, the corresponding file; - stack traces retain one output line per newline-delimited segment, including empty segments; - logcat/file flushing behavior is unchanged; - pointer, reference-type, thread-name, thread-id, and counter fields retain the same ordering and textual structure. ## Measured impact Android arm64 Release NativeAOT `os-bridge.cc` object, compiled with the repository-generated production NDK command: | Artifact | Before | After | Difference | |---|---:|---:|---:| | `os-bridge.cc.o` | 168,784 B | 31,288 B | -137,496 B (-81.46%) | | format/basic-string symbols | 21 | 6 | -15 | The remaining C++ string symbols come from other shared header functionality; the six `std::format` reference-log instantiations are removed. ## Non-goals - no new logging test framework is introduced; - no GC bridge synchronization or graph-processing code changes; - no MonoVM `osbridge.cc` changes; - no changes to the public logging helpers introduced by #12140. ## Validation - `git diff --check`; - no file overlap with #12141, #12142, #12145, or #12148; - NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants; - NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants; - before/after NativeAOT object compilation and symbol inspection; - focused read-only review of newline handling, allocation cleanup, OOM fallback, format types, category gating, and dual-destination output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0f0df1cb-536b-4c53-902e-cbbb9d8e9a15
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0f0df1cb-536b-4c53-902e-cbbb9d8e9a15
|
/review |
Resolve the logging declaration conflicts by retaining the lightweight canonical header, and preserve the reviewed NativeAOT status string constants. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0f0df1cb-536b-4c53-902e-cbbb9d8e9a15
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Review Summary — ⚠️ Needs Changes (minor)
Verdict:
This is a well-scoped, mechanical migration of formatted native logging from std::format/log_* to the printf-style log_*f() / Helpers::abort_applicationf() helpers introduced by #12140, limited to four NativeAOT-reachable runtime utility files. I reviewed the four in-scope files (the remaining files in the diff belong to the stacked base #12140).
What I verified
- Format-specifier correctness —
%zuforsize_t,%ldfor thelonggref count,%dforfd,%pwith explicitvoid*casts, and%.*swith an explicitstatic_cast<int>(length())+.data()for everystd::string_view. Alllog_*f/abort_applicationfhelpers carry__attribute__((format(printf, ...))), so type mismatches are compile-time errors — the PR's per-variant NDK syntax checks cover these. - NUL-safety — non-owning
string_views correctly use%.*swith a bounded length rather than%son a possibly non-terminated.data(); raw C-strings are wrapped inoptional_string(). Theset_environment_variablewarn path is now null-safe viaoptional_string(name)(an improvement over the previous{}on a raw pointer). - Behavior preservation — category gating (debug/info) stays inside the helpers; warn/error/fatal remain unconditional; no control-flow, JNI, or error-path changes.
Observation
- 💡 The mmap
log_infofdiagnostic dropped the original field-width/alignment specifiers ({:<8p},{:<12}) — see the inline comment. Cosmetic, but a real change to log column alignment versus the "formatting preserved" claim.
Notes
mergeable_stateisblockedand the PR depends on #12140 merging first (and targets the stacked branch), as documented in the description — not a code issue.
Nice, careful work — the varargs/string_view length handling is exactly right.
Generated by Android PR Reviewer for #12148 · 72.7 AIC · ⌖ 13 AIC · ⊞ 6.8K
Comment /review to run again
Include the lightweight declaration header from both implementations and the printf-only abort helper so signatures remain checked without adding the std::format surface. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0f0df1cb-536b-4c53-902e-cbbb9d8e9a15
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd41994d-0fbf-4ce3-be3d-50e5ad9b74fb
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd41994d-0fbf-4ce3-be3d-50e5ad9b74fb
## Summary Continue the printf-style logging migration introduced by #12140 for native configuration and parsing diagnostics. This PR converts CoreCLR/NativeAOT host-environment messages and the shared MonoVM/CoreCLR/NativeAOT integer-parsing messages to the printf helpers from #12140. Shared code now has one logging path instead of runtime-specific format branches. **Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only this migration. Part of #12139. ## Scope Only two files change: - `src/native/clr/include/host/host-environment.hh` - `src/native/common/include/runtime-base/strings.hh` These files do not overlap #12141, #12142, #12145, #12148, #12150, or #12153. ## Changes ### Host environment diagnostics `host-environment.hh` is shared by CoreCLR and NativeAOT, but not MonoVM. - migrate generated system-property diagnostics to `%s` formatting; - migrate XDG directory creation diagnostics to `%s` formatting; - migrate XDG directory failure diagnostics while preserving the original errno text; - retain `optional_string()` handling for null C-string inputs. ### Integer parsing diagnostics `strings.hh` is shared by all three runtimes. Its variable diagnostics now use `log_errorf()` without runtime-specific branches: - an invalid starting index, using `%zu` for `size_t`; - signed/unsigned range failures, using explicitly converted `%lld` / `%llu` values; - values that do not represent an integer in the selected `%d` base; - trailing non-numeric characters. The header forward-declares only `log_errorf()` instead of including the heavyweight formatting declarations from `shared/log_types.hh`. ## Behavior preserved - integer conversion logic, range checks, errno handling, and output assignment are unchanged; - the copied parse buffer remains NUL-terminated before `%s` logging; - host environment variables and XDG directory creation behavior are unchanged; - debug category gating and unconditional error/warning semantics remain unchanged. ## Runtime behavior | Runtime | Result | |---|---| | NativeAOT | Uses #12140 printf helpers for all migrated diagnostics | | CoreCLR | Uses #12140 printf helpers for shared parsing and host-environment diagnostics | | MonoVM | Uses the matching MonoVM `log_errorf()` implementation for shared parsing diagnostics | ## Measured impact Representative Android arm64 Release objects compiled before the MonoVM path was unified: | Runtime/object | Before | After | Difference | |---|---:|---:|---:| | NativeAOT `host-environment.cc.o` | 131,288 B | 130,848 B | -440 B | | CoreCLR `timing-internal.cc.o` | 194,048 B | 193,088 B | -960 B | These call sites did not contain explicit `std::format` expressions, so the impact is smaller than #12150/#12153; the change prevents their formatted logging templates from being instantiated. ## Non-goals - no new configuration or parsing behavior; - no changes to environment-variable storage, XDG path construction, or integer parsing buffers; - no new logging tests; - no attempt to migrate unrelated configuration code outside these two headers. ## Validation - `git diff --check`; - NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants; - NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants; - NDK Clang C++23 syntax compilation of all 31 Android arm64 Release MonoVM source variants; - representative before/after object compilation and size comparison; - focused review of signed/unsigned conversions, format widths, null termination, include dependencies, and runtime behavior; - latest head: `72cad5d01`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
Summary
Continue the printf-style native logging migration started by #12140.
This stacked PR converts formatted logging in NativeAOT-reachable shared runtime utilities to the
log_*f()andHelpers::abort_applicationf()APIs introduced by #12140, avoiding additionalstd::formatinstantiations in those paths.Base/dependency: #12155 must merge first. This PR targets
dev/simonrozsival/nativeaot-config-printf-loggingrather thanmainso its diff remains limited to this runtime utility migration.Part of #12139.
Scope
The change is deliberately limited to four files that are compiled into the Android NativeAOT host:
src/native/clr/include/runtime-base/util.hhsrc/native/clr/runtime-base/util.ccsrc/native/clr/runtime-base/android-system-shared.ccsrc/native/clr/host/host-shared.ccThese files do not overlap the currently open sibling work:
Changes
Runtime and environment utilities
fopen,chmod, andfchmoddiagnostics to printf-style logging;std::string_viewoutput through%.*s;optional_string().Memory-mapped APK diagnostics
std::formatallocation in the fatalmmap()error path withHelpers::abort_applicationf();%p,%zu,%d, and%.*sformatting;std::string_viewis NUL-terminated.Android system properties
size_tformat;debug.mono.max_grefcvalues while preserving the compile-time property-name length;longformat.Shared host diagnostics
Formatting and behavior preservation
%p, are explicitly passed asvoid*, and retain the existing%-8pminimum-width alignment;size_tuses%zu,longuses%ld, and descriptors use%d; existing mmap lengths retain their%-12zualignment;%.*swith an explicit length;Non-goals
os-bridge.cc; that path also writes the formatted text to files and needs a separate design.std::formatuse in one change.Validation
git diff --check;std::string_viewlengths, pointer conversions, category semantics, and varargs use.The higher-level native project builds were also attempted, but the local Gradle 9.4.1 distribution failed while starting the unrelated
manifestmergerbuild, before native compilation. The direct checks above use the repository-generated production NDK compile commands and generated headers.