[mono][hot reload] Fix sequence point logic for compiler-generated me…

…thods in updates (dotnet#65865)

* [tests] Pass PDB delta to ApplyUpdate

* extra tracing output

* Return 0 if a method has no sequence points

   -1 means there was some kind of problem

   0 means 0 sequence points

   In particular for mono_ppdb_get_seq_points_enc a compiler-generated method might have 0 sequence points in the PDB delta, but we should consider that as ok, and not fall back to looking in the baseline image

* pass MONO_DEBUG=getn-seq-points to hot reload tests

   For platforms where we use the remote executor, set the environment variable when starting the remote process.

   For WASM, pass --setenv when building the project.

* fix whitespace

src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs

@@ -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);
         }
@@ -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.

src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj

@@ -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>

src/mono/mono/component/hot_reload.c

@@ -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;
 			}
@@ -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
@@ -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);

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;
 }