Skip to content

Commit 4a6399a

Browse files
Copilotlewing
andauthored
Fix memory leaks from mono_type_full_name in marshal-shared.c error paths (dotnet#125616)
`mono_type_full_name()` returns a heap-allocated string, but several call sites in `marshal-shared.c` passed the result directly into `g_strdup_printf` without freeing it — leaking on every hit. These paths can fire repeatedly during AOT compilation against assemblies with many invalid marshal configurations. # Description Store each `mono_type_full_name()` result in a local, use it, then `g_free` it: ```c // Before char *msg = g_strdup_printf("Type %s ...", mono_type_full_name(m_class_get_byval_arg(klass))); // After char *type_name = mono_type_full_name(m_class_get_byval_arg(klass)); char *msg = g_strdup_printf("Type %s ...", type_name); g_free(type_name); ``` Fixed sites: - **Auto-layout check** — `g_strdup_printf` with single `mono_type_full_name`; result stored, used, and freed - **Explicit-layout `g_error` path** — left as-is (inline `mono_type_full_name` call); `g_error` is fatal and never returns, and in `DISABLE_ASSERT_MESSAGES` builds its variadic args are discarded entirely — introducing a local variable causes an unused-variable build error in WASM/WASI configurations - **Generic type marshal check** — `g_strdup_printf` with single `mono_type_full_name`; result stored, used, and freed - **Struct field type check** — `g_strdup_printf` with two `mono_type_full_name` calls; both results stored, used, and freed # Customer Impact Cumulative memory growth during AOT compilation of assemblies containing types with invalid marshal configurations (auto-layout structs, non-blittable generic fields, reference-typed struct fields). # Regression No — pre-existing leak, not a regression. # Testing Validated by code inspection and CI. These are error paths with no dedicated test infrastructure in the repo; the fix is mechanical and low-risk. # Risk Very low. Changes are confined to error paths that emit a marshal exception. No logic changes — only introduction of local variables and paired `g_free` calls before strings go out of scope. The `g_error` path is intentionally left unchanged to avoid a `-Werror,-Wunused-variable` build failure in WASM/WASI configurations where `g_error` discards its variadic arguments. # Package authoring no longer needed in .NET 9 IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Memory leaks from mono_type_full_name in marshal-shared.c error paths</issue_title> > <issue_description>## Description > > `mono_type_full_name()` returns an allocated string, but several call sites in `src/mono/mono/metadata/marshal-shared.c` pass it directly into `g_strdup_printf` or `g_error` without freeing the intermediate allocation: > > - **Line 778**: `g_strdup_printf("Type %s ...", mono_type_full_name(...))` > - **Line 816**: `g_error("Type %s ...", mono_type_full_name(...))` > - **Line 866**: `g_strdup_printf("Generic type %s ...", mono_type_full_name(...))` > - **Lines 912-913**: `g_strdup_printf("Type %s with field type %s ...", mono_type_full_name(...), mono_type_full_name(...))` > > Each leaks the string returned by `mono_type_full_name()`. While these are error paths, they can fire repeatedly during AOT compilation (e.g., when processing assemblies with many invalid marshal configurations), causing cumulative memory growth. > > ## Fix > > Store the result in a temporary, use it in the format string, then `g_free` it: > > ```c > char *type_name = mono_type_full_name(m_class_get_byval_arg(klass)); > char *msg = g_strdup_printf("Type %s ...", type_name); > g_free(type_name); > mono_marshal_shared_mb_emit_exception_marshal_directive(mb, msg); > ``` > > This pattern should be applied to all call sites in the file.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes dotnet#125576 <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lewing <24063+lewing@users.noreply.github.com> Co-authored-by: Larry Ewing <lewing@microsoft.com>
1 parent baa4920 commit 4a6399a

1 file changed

Lines changed: 12 additions & 4 deletions

File tree

src/mono/mono/metadata/marshal-shared.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,8 +774,10 @@ mono_marshal_shared_emit_struct_conv_full (MonoMethodBuilder *mb, MonoClass *kla
774774

775775
if (klass != mono_class_try_get_safehandle_class ()) {
776776
if (mono_class_is_auto_layout (klass)) {
777+
char *type_name = mono_type_full_name (m_class_get_byval_arg (klass));
777778
char *msg = g_strdup_printf ("Type %s which is passed to unmanaged code must have a StructLayout attribute.",
778-
mono_type_full_name (m_class_get_byval_arg (klass)));
779+
type_name);
780+
g_free (type_name);
779781
mono_marshal_shared_mb_emit_exception_marshal_directive (mb, msg);
780782
return;
781783
}
@@ -862,8 +864,10 @@ mono_marshal_shared_emit_struct_conv_full (MonoMethodBuilder *mb, MonoClass *kla
862864
break;
863865
case MONO_TYPE_GENERICINST:
864866
if (!mono_type_generic_inst_is_valuetype (ftype)) {
867+
char *type_name = mono_type_full_name (ftype);
865868
char *msg = g_strdup_printf ("Generic type %s cannot be marshaled as field in a struct.",
866-
mono_type_full_name (ftype));
869+
type_name);
870+
g_free (type_name);
867871
mono_marshal_shared_mb_emit_exception_marshal_directive (mb, msg);
868872
break;
869873
}
@@ -908,9 +912,13 @@ mono_marshal_shared_emit_struct_conv_full (MonoMethodBuilder *mb, MonoClass *kla
908912
case MONO_TYPE_CLASS:
909913
case MONO_TYPE_SZARRAY:
910914
case MONO_TYPE_ARRAY: {
915+
char *klass_name = mono_type_full_name (m_class_get_byval_arg (klass));
916+
char *field_type_name = mono_type_full_name (ftype);
911917
char *msg = g_strdup_printf ("Type %s with field type %s cannot be marshaled as an unmanaged struct field.",
912-
mono_type_full_name (m_class_get_byval_arg (klass)),
913-
mono_type_full_name (ftype));
918+
klass_name,
919+
field_type_name);
920+
g_free (klass_name);
921+
g_free (field_type_name);
914922
mono_marshal_shared_mb_emit_exception_marshal_directive (mb, msg);
915923
break;
916924
}

0 commit comments

Comments
 (0)