Skip to content

[mono][hot reload] Fix sequence point logic for compiler-generated methods in updates #65865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ 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 = System.IO.File.ReadAllBytes(dpdb_name);

MetadataUpdater.ApplyUpdate(assm, dmeta_data, dil_data, dpdb_data);
}
Expand All @@ -94,6 +95,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
<!-- Some tests rely on no deps.json file being present. -->
<GenerateDependencyFile>false</GenerateDependencyFile>
<!-- EnC tests on targets without a remote executor need the environment variable set before launching the test -->
<WasmXHarnessMonoArgs>--setenv=DOTNET_MODIFIABLE_ASSEMBLIES=debug</WasmXHarnessMonoArgs>
<!-- Also ask Mono to use make sequence points even without a debugger attached to match "dotnet watch" behavior -->
<WasmXHarnessMonoArgs>--setenv=DOTNET_MODIFIABLE_ASSEMBLIES=debug --setenv=MONO_DEBUG=gen-seq-points</WasmXHarnessMonoArgs>

<!-- disabled due to https://github.com/dotnet/runtime/issues/65672 -->
<XUnitUseRandomizedTestOrderer>false</XUnitUseRandomizedTestOrderer>
Expand Down
19 changes: 19 additions & 0 deletions src/mono/mono/component/hot_reload.c
Original file line number Diff line number Diff line change
Expand Up @@ -1799,6 +1799,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;
}
Expand Down Expand Up @@ -1941,6 +1942,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
Expand Down Expand Up @@ -2002,7 +2018,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);
Expand Down
4 changes: 2 additions & 2 deletions src/mono/mono/metadata/debug-mono-ppdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down