Skip to content

Commit

Permalink
[mono] catch the case of updated methods in mono_debug_lookup_source_…
Browse files Browse the repository at this point in the history
…location (#94540)

* Add new crashing test

* [mono] catch the case of updated methods in mono_debug_lookup_source_location

   If a newly added method is in a stack trace, look it up in the EnC debug information, if available.

* add debugger test

Co-authored-by: Thays Grazia <thaystg@gmail.com>
  • Loading branch information
lambdageek and thaystg authored Nov 12, 2023
1 parent e37b853 commit 38463e2
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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 NewMethodThrows
{
public string ExistingMethod(string x)
{
return x;
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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 NewMethodThrows
{
public string ExistingMethod(string x)
{
return NewMethod(x);
}

public string NewMethod(string x)
{
throw new InvalidOperationException (x);
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<TestRuntime>true</TestRuntime>
<DeltaScript>deltascript.json</DeltaScript>
</PropertyGroup>
<ItemGroup>
<Compile Include="NewMethodThrows.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"changes": [
{"document": "NewMethodThrows.cs", "update": "NewMethodThrows_v1.cs"},
]
}

47 changes: 46 additions & 1 deletion src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -938,5 +938,50 @@ public static void TestGenericAddInstanceField()
Assert.Equal(dt, z.GetIt());
});
}
}

[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
public static void TestNewMethodThrows()
{
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows).Assembly;
var x = new System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows();
Assert.Equal("abcd", x.ExistingMethod("abcd"));
ApplyUpdateUtil.ApplyUpdate(assm);
InvalidOperationException exn = Assert.Throws<InvalidOperationException>(() => x.ExistingMethod("spqr"));
Assert.Equal("spqr", exn.Message);
var stackTrace = new System.Diagnostics.StackTrace(exn, fNeedFileInfo: true);
var frames = stackTrace.GetFrames();
// the throwing method and its caller and a few frames of XUnit machinery for Assert.Throws, above
Assert.True(frames.Length >= 2);
var throwingMethod = frames[0].GetMethod();
var newMethod = typeof (System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows).GetMethod("NewMethod");
Assert.Equal(newMethod, throwingMethod);
// We don't have the filename on all runtimes and platforms
var frame0Name = frames[0].GetFileName();
Assert.True(frame0Name == null || frame0Name.Contains("NewMethodThrows.cs"));
var existingMethod = typeof (System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows).GetMethod("ExistingMethod");
var throwingMethodCaller = frames[1].GetMethod();
Assert.Equal(existingMethod, throwingMethodCaller);
var frame1Name = frames[0].GetFileName();
Assert.True(frame1Name == null || frame1Name.Contains("NewMethodThrows.cs"));
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod\System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.GenericAddStaticField\System.Reflection.Metadata.ApplyUpdate.Test.GenericAddStaticField.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.GenericAddInstanceField\System.Reflection.Metadata.ApplyUpdate.Test.GenericAddInstanceField.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows\System.Reflection.Metadata.ApplyUpdate.Test.NewMethodThrows.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetOS)' == 'browser'">
<WasmFilesToIncludeFromPublishDir Include="$(AssemblyName).dll" />
Expand Down
22 changes: 22 additions & 0 deletions src/mono/mono/metadata/mono-debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,28 @@ mono_debug_lookup_source_location (MonoMethod *method, guint32 address, MonoDoma
if (mono_debug_format == MONO_DEBUG_FORMAT_NONE)
return NULL;

MonoImage *img = m_class_get_image (method->klass);
if (G_UNLIKELY (img->has_updates)) {
guint32 idx = mono_metadata_token_index (method->token);
MonoDebugInformationEnc *mdie = (MonoDebugInformationEnc *) mono_metadata_update_get_updated_method_ppdb (img, idx);
if (mdie != NULL) {
offset = il_offset_from_address (method, address);
if (offset < 0) {
mono_debugger_unlock ();
return NULL;
}

MonoDebugSourceLocation * ret = mono_ppdb_lookup_location_enc (mdie->ppdb_file, mdie->idx, offset);
if (ret)
return ret;
} else {
/// added method without EnC info, maybe the delta came in without a PDB delta
gboolean added_method = idx >= table_info_get_rows (&img->tables[MONO_TABLE_METHOD]);
if (added_method)
return NULL;
}
}

mono_debugger_lock ();
minfo = lookup_method (method);
if (!minfo || !minfo->handle) {
Expand Down
62 changes: 62 additions & 0 deletions src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,68 @@ public async Task DebugHotReloadMethod_AddingNewClassUsingDebugAttribute()
await StepAndCheck(StepKind.Into, $"dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 119, 12, "ApplyUpdateReferencedAssembly.MethodBody10.StaticMethod1");
}

[ConditionalFact(nameof(RunningOnChrome))]
public async Task DebugHotReloadMethod_AddingNewMethodAndThrowException()
{
//await SetPauseOnException("all");
string asm_file = Path.Combine(DebuggerTestAppPath, "ApplyUpdateReferencedAssembly.dll");
string pdb_file = Path.Combine(DebuggerTestAppPath, "ApplyUpdateReferencedAssembly.pdb");
string asm_file_hot_reload = Path.Combine(DebuggerTestAppPath, "../wasm/ApplyUpdateReferencedAssembly.dll");

var bp_notchanged = await SetBreakpoint(".*/MethodBody1.cs$", 129, 12, use_regex: true);
var bp_invalid = await SetBreakpoint(".*/MethodBody1.cs$", 133, 12, use_regex: true);

var pause_location = await LoadAssemblyAndTestHotReloadUsingSDBWithoutChanges(
asm_file, pdb_file, "MethodBody11", "StaticMethod1", expectBpResolvedEvent: true, sourcesToWait: new string [] { "MethodBody0.cs", "MethodBody1.cs" });

CheckLocation("dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 129, 12, scripts, pause_location["callFrames"]?[0]["location"]);
//apply first update
pause_location = await LoadAssemblyAndTestHotReloadUsingSDB(
asm_file_hot_reload, "MethodBody11", "NewMethodStaticWithThrow", 1);

JToken top_frame = pause_location["callFrames"]?[0];
AssertEqual("ApplyUpdateReferencedAssembly.MethodBody11.NewMethodStaticWithThrow", top_frame?["functionName"]?.Value<string>(), top_frame?.ToString());
CheckLocation("dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 133, 12, scripts, top_frame["location"]);
await StepAndCheck(StepKind.Over, "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 134, 12, "ApplyUpdateReferencedAssembly.MethodBody11.NewMethodStaticWithThrow",
locals_fn: async (locals) =>
{
CheckNumber(locals, "i", 20);
await Task.CompletedTask;
}
);

await SetPauseOnException("all");

pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", null, 0, 0, null);

await CheckValue(pause_location["data"], JObject.FromObject(new
{
type = "object",
subtype = "error",
className = "System.Exception",
uncaught = false
}), "exception0.data");

pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", null, 0, 0, null);
try
{
pause_location = await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", null, 0, 0, null);
}
catch (System.Exception ae)
{
System.Console.WriteLine(ae);
var eo = JObject.Parse(ae.Message);

AssertEqual("Uncaught", eo["exceptionDetails"]?["text"]?.Value<string>(), "text");

await CheckValue(eo["exceptionDetails"]?["exception"], JObject.FromObject(new
{
type = "object",
subtype = "error",
className = "Error"
}), "exception");
}
}
// Enable this test when https://github.com/dotnet/hotreload-utils/pull/264 flows into dotnet/runtime repo
// [ConditionalFact(nameof(RunningOnChrome))]
// public async Task DebugHotReloadMethod_ChangeParameterName()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,23 @@ public static void StaticMethod1 () {
// return M1(1, 2);
// }
// }













public class MethodBody11 {
public static void StaticMethod1 () {
Console.WriteLine("breakpoint in a line that will not be changed");
Console.WriteLine("original");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,18 @@ public static void StaticMethod2 () {
Console.WriteLine($"do not step into here");
}
}

public class MethodBody11 {
public static void StaticMethod1 () {
Console.WriteLine("breakpoint in a line that will not be changed");
Console.WriteLine("original");
}
public static void NewMethodStaticWithThrow () {
int i = 20;
Console.WriteLine($"add a breakpoint in the new static method, look at locals {i}");
throw new Exception("my exception");
/*var newvar = new MethodBody6();
newvar.NewMethodInstance (10);*/
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,18 @@ public static void StaticMethod2 () {
Console.WriteLine($"do not step into here");
}
}

public class MethodBody11 {
public static void StaticMethod1 () {
Console.WriteLine("breakpoint in a line that will not be changed");
Console.WriteLine("original");
}
public static void NewMethodStaticWithThrow () {
int i = 20;
Console.WriteLine($"add a breakpoint in the new static method, look at locals {i}");
throw new Exception("my exception");
/*var newvar = new MethodBody6();
newvar.NewMethodInstance (10);*/
}
}
}

0 comments on commit 38463e2

Please sign in to comment.