[NativeAOT] Migrate GC bridge logging to printf#12211
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
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
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
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
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
Use the APK description produced by the failed macOS BuildReleaseArm64 XForms CoreCLR test in Azure DevOps build 1520455. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd41994d-0fbf-4ce3-be3d-50e5ad9b74fb
Convert the remaining std::format-style GC bridge and logger diagnostics compiled into the NativeAOT host to the canonical printf logging helpers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 13af6b96-9e20-422c-b3ba-8cba7f1aa5bb
Return static segment error messages as C strings so printf logging can consume them directly without a string_view. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 13af6b96-9e20-422c-b3ba-8cba7f1aa5bb
Convert the remaining easy formatted GC bridge diagnostics reachable by NativeAOT to the canonical shared printf logging APIs while preserving gating, no-check behavior, formatting, and null handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4aed776f-8743-45ba-b96b-079253595056
There was a problem hiding this comment.
Pull request overview
This PR continues the NativeAOT/CoreCLR native-host logging migration by converting the remaining GC-bridge formatted log sites in src/native/clr/host/gc-bridge.cc from std::format-style logging to the shared printf-style logging helpers introduced earlier in the series.
Changes:
- Replace
log_info(...)/log_info_nocheck_fmt(...)formatted calls withlog_infof(...)andlog_writef(...). - Preserve existing GC-spew/category gating (
Logger::gc_spew_enabled()andUtil::should_log()), including the explicit “no-check” write after the gate. - Use
PRIxPTR+uintptr_tfor handle-width hex formatting and add<cinttypes>for the format macro.
Express the static string-segment error mapping as a compile-time function now that it returns string literals. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 13af6b96-9e20-422c-b3ba-8cba7f1aa5bb
…gging-next' into jonathanpeppers-finish-gc-bridge-printf
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
✅ LGTM — clean, behavior-preserving migration
Reviewed the migration of the five remaining NativeAOT-reachable formatted logging calls in src/native/clr/host/gc-bridge.cc from the std::format-style APIs to the printf-style shared APIs.
Correctness verified against the API definitions:
| Old call | New call | Gating behavior |
|---|---|---|
log_info (macro → DO_LOG_FMT) |
log_infof |
Both check log_categories & category before writing — ✅ equivalent |
log_info_nocheck_fmt |
log_writef (..., LogLevel::Info, ...) |
Both write unconditionally — ✅ equivalent, and correctly placed after the explicit Util::should_log (LOG_GC) gate |
Format specifier mapping is correct:
{}onsize_t→%zu✅{:#x}on handle →0x%" PRIxPTR "with<cinttypes>added ✅ (and improved by usinguintptr_t— see inline){}onchar*→%swithoptional_string()guard ✅ (a real nullptr-crash fix — see inline)
Positive callouts:
- The
LOG_GCcategory,gc_spew_enabled()/Util::should_log()gates, message text, and control flow are all preserved exactly as the PR description claims — verified against the full source. <cinttypes>(the C++ header, per repo convention) is the correct include forPRIxPTR.- Two of the changes (
optional_stringguard and signed→unsigned handle) are genuine robustness/correctness improvements over the pre-migration code, not just mechanical swaps.
Notes:
- CI on the
dnceng-publicpipeline was stillpendingat review time (no checks reported yet) — this is not a blocker but should be green before merge, since the change is native code that only the build validates. - As the PR states, the three
std::formatcalls inbridge-processing.ccfeeding_monodroid_gref_logare intentionally out of scope.
No blocking issues. Two 💡 suggestions posted inline highlighting the subtle improvements for future reference.
Generated by Android PR Reviewer for #12211 · 92.3 AIC · ⌖ 18.9 AIC · ⊞ 6.8K
Comment /review to run again
Dependency
Depends on #12210 and targets its
jonathanpeppers-native-printf-logging-nextbranch.Scope
Migrates the five remaining easy NativeAOT-reachable formatted logging calls in
src/native/clr/host/gc-bridge.ccto the canonical shared printf APIs introduced by #12155.Behavior preservation
LOG_GCcategory, existinggc_spew_enabled()andUtil::should_log()gates, and the no-check write after the explicit gate.%zuforsize_tvalues andPRIxPTRfor handle-width hexadecimal values, including the explicit0xprefix corresponding to{:#x}.optional_string().Non-goals
Does not migrate the three nearby
std::formatcalls inbridge-processing.ccthat feedOSBridge::_monodroid_gref_log; those require a separate API design.Validation
dotnet build src\native\native-nativeaot.csproj -p:AndroidSupportedTargetJitAbis=arm64-v8a -p:AndroidSupportedTargetAotAbis=arm64 -v:minimaldotnet build src\native\native-clr.csproj -p:AndroidSupportedTargetJitAbis=arm64-v8a -p:AndroidSupportedTargetAotAbis=arm64 -v:minimalgit diff --check