diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression.cs new file mode 100644 index 00000000000000..d98c367138bdf8 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; + + +namespace System.Reflection.Metadata.ApplyUpdate.Test; + +public class StaticLambdaRegression +{ + public int count; + + public string TestMethod() + { + count++; +#if false + Message (static () => "hello"); +#endif + return count.ToString(); + } + +#if false + public void Message (Func msg) => Console.WriteLine (msg()); +#endif +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression_v1.cs new file mode 100644 index 00000000000000..50194feb249638 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression_v1.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; + + +namespace System.Reflection.Metadata.ApplyUpdate.Test; + +public class StaticLambdaRegression +{ + public int count; + + public string TestMethod() + { + count++; +#if true + Message (static () => "hello2"); +#endif + return count.ToString(); + } + +#if true + public void Message (Func msg) => Console.WriteLine (msg()); +#endif +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression_v2.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression_v2.cs new file mode 100644 index 00000000000000..bde43c311c4b2f --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/StaticLambdaRegression_v2.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; + + +namespace System.Reflection.Metadata.ApplyUpdate.Test; + +public class StaticLambdaRegression +{ + public int count; + + public string TestMethod() + { + count++; +#if true + Message (static () => "hello2"); +#endif + Message (static () => "goodbye"); + return count.ToString(); + } + +#if true + public void Message (Func msg) => Console.WriteLine (msg()); +#endif +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression.csproj new file mode 100644 index 00000000000000..015a252a700f82 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression.csproj @@ -0,0 +1,11 @@ + + + System.Runtime.Loader.Tests + $(NetCoreAppCurrent) + true + deltascript.json + + + + + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/deltascript.json new file mode 100644 index 00000000000000..f8ece3182bcbc9 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression/deltascript.json @@ -0,0 +1,7 @@ +{ + "changes": [ + {"document": "StaticLambdaRegression.cs", "update": "StaticLambdaRegression_v1.cs"}, + {"document": "StaticLambdaRegression.cs", "update": "StaticLambdaRegression_v2.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 9e28894b6bbf37..527073494eebb2 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -395,5 +395,37 @@ public static void IsSupported() bool result = MetadataUpdater.IsSupported; Assert.False(result); } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestStaticLambdaRegression() + { + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression).Assembly; + var x = new System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression(); + + Assert.Equal (0, x.count); + + x.TestMethod(); + x.TestMethod(); + + Assert.Equal (2, x.count); + + ApplyUpdateUtil.ApplyUpdate(assm, usePDB: false); + + x.TestMethod(); + x.TestMethod(); + + Assert.Equal (4, x.count); + + ApplyUpdateUtil.ApplyUpdate(assm, usePDB: false); + + x.TestMethod(); + x.TestMethod(); + + Assert.Equal (6, x.count); + + }); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs index 405afc9f50edbf..c7953ea59f1961 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs @@ -66,7 +66,7 @@ internal static bool HasApplyUpdateCapabilities() private static System.Collections.Generic.Dictionary assembly_count = new(); - internal static void ApplyUpdate (System.Reflection.Assembly assm) + internal static void ApplyUpdate (System.Reflection.Assembly assm, bool usePDB = true) { int count; if (!assembly_count.TryGetValue(assm, out count)) @@ -83,9 +83,13 @@ internal static void ApplyUpdate (System.Reflection.Assembly assm) string dmeta_name = $"{basename}.{count}.dmeta"; string dil_name = $"{basename}.{count}.dil"; + string dpdb_name = $"{basename}.{count}.dpdb"; byte[] dmeta_data = System.IO.File.ReadAllBytes(dmeta_name); byte[] dil_data = System.IO.File.ReadAllBytes(dil_name); - byte[] dpdb_data = null; // TODO also use the dpdb data + byte[] dpdb_data = null; + + if (usePDB) + dpdb_data = System.IO.File.ReadAllBytes(dpdb_name); MetadataUpdater.ApplyUpdate(assm, dmeta_data, dil_data, dpdb_data); } @@ -94,6 +98,20 @@ internal static void AddRemoteInvokeOptions (ref RemoteInvokeOptions options) { options = options ?? new RemoteInvokeOptions(); options.StartInfo.EnvironmentVariables.Add(DotNetModifiableAssembliesSwitch, DotNetModifiableAssembliesValue); + /* Ask mono to use .dpdb data to generate sequence points even without a debugger attached */ + if (IsMonoRuntime) + AppendEnvironmentVariable(options.StartInfo.EnvironmentVariables, "MONO_DEBUG", "gen-seq-points"); + } + + private static void AppendEnvironmentVariable(System.Collections.Specialized.StringDictionary env, string key, string addedValue) + { + if (!env.ContainsKey(key)) + env.Add(key, addedValue); + else + { + string oldValue = env[key]; + env[key] = oldValue + "," + addedValue; + } } /// Run the given test case, which applies updates to the given assembly. diff --git a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj index e21bc7e673f5ff..4c586e5dd0e6af 100644 --- a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj +++ b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj @@ -7,7 +7,8 @@ false - --setenv=DOTNET_MODIFIABLE_ASSEMBLIES=debug + + --setenv=DOTNET_MODIFIABLE_ASSEMBLIES=debug --setenv=MONO_DEBUG=gen-seq-points @@ -56,6 +57,7 @@ + diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index f10540254619c1..d4e4ab74c7a075 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1176,8 +1176,7 @@ delta_info_compute_table_records (MonoImage *image_dmeta, MonoImage *image_base, g_assert (table != MONO_TABLE_ENCLOG); g_assert (table != MONO_TABLE_ENCMAP); g_assert (table >= prev_table); - /* FIXME: check bounds - is it < or <=. */ - if (rid < delta_info->count[table].prev_gen_rows) { + if (rid <= delta_info->count[table].prev_gen_rows) { base_info->any_modified_rows[table] = TRUE; delta_info->count[table].modified_rows++; } else @@ -1483,7 +1482,13 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de * still resolves to the same MonoMethod* (but we can't check it in * pass1 because we haven't added the new AssemblyRefs yet. */ - if (ca_base_cols [MONO_CUSTOM_ATTR_PARENT] != ca_upd_cols [MONO_CUSTOM_ATTR_PARENT]) { + /* NOTE: Apparently Roslyn sometimes sends NullableContextAttribute + * deletions even if the ChangeCustomAttribute capability is unset. + * So tacitly accept updates where a custom attribute is deleted + * (its parent is set to 0). Once we support custom attribute + * changes, we will support this kind of deletion for real. + */ + if (ca_base_cols [MONO_CUSTOM_ATTR_PARENT] != ca_upd_cols [MONO_CUSTOM_ATTR_PARENT] && ca_upd_cols [MONO_CUSTOM_ATTR_PARENT] != 0) { mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing CA table cols with a different Parent. token=0x%08x", log_token); unsupported_edits = TRUE; continue; @@ -1799,6 +1804,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen g_assert (add_member_klass); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); MonoDebugInformationEnc *method_debug_information = hot_reload_get_method_debug_information (delta_info->ppdb_file, token_index); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Debug info for method 0x%08x has ppdb idx 0x%08x", log_token, method_debug_information ? method_debug_information->idx : 0); add_method_to_baseline (base_info, delta_info, add_member_klass, log_token, method_debug_information); add_member_klass = NULL; } @@ -1941,6 +1947,21 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen return TRUE; } +static void +dump_methodbody (MonoImage *image) +{ + if (!mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE)) + return; + MonoTableInfo *t = &image->tables [MONO_TABLE_METHODBODY]; + uint32_t rows = table_info_get_rows (t); + for (uint32_t i = 0; i < rows; ++i) + { + uint32_t cols[MONO_METHODBODY_SIZE]; + mono_metadata_decode_row (t, i, cols, MONO_METHODBODY_SIZE); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, " row[%02d] = doc: 0x%08x seq: 0x%08x", i + 1, cols [MONO_METHODBODY_DOCUMENT], cols [MONO_METHODBODY_SEQ_POINTS]); + } +} + /** * * LOCKING: Takes the publish_lock @@ -2002,7 +2023,10 @@ hot_reload_apply_changes (int origin, MonoImage *image_base, gconstpointer dmeta mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb image user string size: 0x%08x", image_dpdb->heap_us.size); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb image blob heap addr: %p", image_dpdb->heap_blob.data); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pdb image blob heap size: 0x%08x", image_dpdb->heap_blob.size); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "ppdb methodbody: "); + dump_methodbody (image_dpdb); ppdb_file = mono_create_ppdb_file (image_dpdb, FALSE); + g_assert (ppdb_file->image == image_dpdb); } BaselineInfo *base_info = baseline_info_lookup_or_add (image_base); diff --git a/src/mono/mono/metadata/debug-mono-ppdb.c b/src/mono/mono/metadata/debug-mono-ppdb.c index ad6315b73cb3c8..08d0a2b7f4ae3a 100644 --- a/src/mono/mono/metadata/debug-mono-ppdb.c +++ b/src/mono/mono/metadata/debug-mono-ppdb.c @@ -505,7 +505,7 @@ mono_ppdb_get_seq_points_internal (MonoImage *image, MonoPPDBFile *ppdb, MonoMet docidx = cols [MONO_METHODBODY_DOCUMENT]; if (!cols [MONO_METHODBODY_SEQ_POINTS]) - return -1; + return 0; ptr = mono_metadata_blob_heap (image, cols [MONO_METHODBODY_SEQ_POINTS]); size = mono_metadata_decode_blob_size (ptr, &ptr); @@ -599,7 +599,7 @@ gboolean mono_ppdb_get_seq_points_enc (MonoDebugMethodInfo *minfo, MonoPPDBFile *ppdb_file, int idx, char **source_file, GPtrArray **source_file_list, int **source_files, MonoSymSeqPoint **seq_points, int *n_seq_points) { MonoMethod *method = minfo->method; - if (mono_ppdb_get_seq_points_internal (ppdb_file->image, ppdb_file, method, idx, source_file, source_file_list, source_files, seq_points, n_seq_points) > 0) + if (mono_ppdb_get_seq_points_internal (ppdb_file->image, ppdb_file, method, idx, source_file, source_file_list, source_files, seq_points, n_seq_points) >= 0) return TRUE; return FALSE; } diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index cac76bda28d4f9..754af035ff3c2f 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -2276,6 +2276,9 @@ mono_metadata_method_has_param_attrs (MonoImage *m, int def) MonoTableInfo *methodt = &m->tables [MONO_TABLE_METHOD]; guint lastp, i, param_index = mono_metadata_decode_row_col (methodt, def - 1, MONO_METHOD_PARAMLIST); + if (param_index == 0) + return FALSE; + /* FIXME: metadata-update */ if (def < table_info_get_rows (methodt)) lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); diff --git a/src/mono/mono/metadata/mono-debug.c b/src/mono/mono/metadata/mono-debug.c index 78b1e07a421836..944a5605532cc9 100644 --- a/src/mono/mono/metadata/mono-debug.c +++ b/src/mono/mono/metadata/mono-debug.c @@ -846,6 +846,10 @@ mono_debug_method_lookup_location (MonoDebugMethodInfo *minfo, int il_offset) MonoDebugSourceLocation * ret = mono_ppdb_lookup_location_enc (mdie->ppdb_file, mdie->idx, il_offset); if (ret) return ret; + } else { + gboolean added_method = idx >= table_info_get_rows (&img->tables[MONO_TABLE_METHOD]); + if (added_method) + return NULL; } } @@ -1144,6 +1148,25 @@ mono_debug_get_seq_points (MonoDebugMethodInfo *minfo, char **source_file, GPtrA if (mono_ppdb_get_seq_points_enc (minfo, mdie->ppdb_file, mdie->idx, source_file, source_file_list, source_files, seq_points, n_seq_points)) return; } + /* + * dotnet watch sometimes sends us updated with PPDB deltas, but the baseline + * project has debug info (and we use it for seq points?). In tht case, just say + * the added method has no sequence points. N.B. intentionally, comparing idx to + * the baseline tables. For methods that already existed, use their old seq points. + */ + if (idx >= table_info_get_rows (&img->tables[MONO_TABLE_METHOD])) { + if (source_file) + *source_file = NULL; + if (source_file_list) + *source_file_list = NULL; + if (source_files) + *source_files = NULL; + if (seq_points) + *seq_points = NULL; + if (n_seq_points) + *n_seq_points = 0; + return; + } } if (minfo->handle->ppdb) mono_ppdb_get_seq_points (minfo, source_file, source_file_list, source_files, seq_points, n_seq_points);