[mono][debugger] Insert and search seq_point in the same MonoJitMemoryManager by thaystg · Pull Request #54757 · dotnet/runtime

thaystg · 2021-06-25T18:41:44Z

Fixing side effect of #48483
When starting XAML Hot Reload, in transform.c the seq_point was saved using jit_mm_for_method: https://github.com/thaystg/runtime/blob/abccfadbf570033efee8ac9a6992f6f7ee51f474/src/mono/mono/mini/interp/transform.c#L9760
When trying to search was trying on get_default_jit_mm.

How to reproduce:
Start an Android App with interpreter mode and XAML HotReload enabled.
This message will appear on output: Assertion at /__w/1/s/src/mono/mono/mini/debugger-engine.c:1088, condition found_sp' not met`
And the app will close.
If you disable XAML HotReload it will work fine.

Done in this PR:

Insert and search seq_point in the same MonoJitMemoryManager
Fix debugging ALC on wasm debugger
Adding a test case

When starting XAML Hot Reload, in transform.c the seq_point was saved using jit_mm_for_method: https://github.com/thaystg/runtime/blob/abccfadbf570033efee8ac9a6992f6f7ee51f474/src/mono/mono/mini/interp/transform.c#L9760 When trying to search was trying on get_default_jit_mm.

ghost · 2021-06-25T18:41:48Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2021-06-25T18:44:46Z

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixing side effect of #48483
When starting XAML Hot Reload, in transform.c the seq_point was saved using jit_mm_for_method: https://github.com/thaystg/runtime/blob/abccfadbf570033efee8ac9a6992f6f7ee51f474/src/mono/mono/mini/interp/transform.c#L9760
When trying to search was trying on get_default_jit_mm.

Author:	thaystg
Assignees:	-
Labels:	`area-Debugger-mono`
Milestone:	-

lambdageek · 2021-06-25T18:50:35Z

@thaystg will this break non-interpreted debugging now?

runtime/src/mono/mono/mini/seq-points.c

Lines 241 to 248 in 57bfe47

    
           MonoJitMemoryManager *jit_mm = get_default_jit_mm (); 
        
           jit_mm_lock (jit_mm); 
        
           // FIXME: The lookup can fail if the method is JITted recursively though a type cctor 
        
           if (!g_hash_table_lookup (jit_mm->seq_points, cfg->method_to_register)) 
        
           	g_hash_table_insert (jit_mm->seq_points, cfg->method_to_register, cfg->seq_point_info); 
        
           else 
        
           	mono_seq_point_info_free (cfg->seq_point_info); 
        
           jit_mm_unlock (jit_mm);

doesn't seem right for JIT

thaystg · 2021-06-25T18:59:42Z

I tested and it doesnt' break, but I will add more changes in this PR.

src/mono/mono/mini/debugger-engine.c

src/mono/mono/mini/seq-points.c

src/mono/mono/mini/debugger-engine.c

This reverts commit fd7f206.

This reverts commit 6faffb8.

This reverts commit 44601d3.

lambdageek

Ok, for .net 6 we will use the default memory manager for the sequence points.
@thaystg please make a GH Issue to clean this up (by using jit_mm_for_method for sequence points) next release.

lewing · 2021-06-25T20:06:53Z

/backport to release/6.0-preview6

github-actions · 2021-06-25T20:07:07Z

Started backporting to release/6.0-preview6: https://github.com/dotnet/runtime/actions/runs/972568284

github-actions · 2021-06-25T20:08:45Z

@lewing backporting to release/6.0-preview6 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: [mono][debugger] Fixing side effect of #48483
Applying: Fixing everywhere that uses seq_points.
Using index info to reconstruct a base tree...
M	src/mono/mono/mini/debugger-engine.c
Falling back to patching base and 3-way merge...
Auto-merging src/mono/mono/mini/debugger-engine.c
CONFLICT (content): Merge conflict in src/mono/mono/mini/debugger-engine.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Fixing everywhere that uses seq_points.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

lewing · 2021-06-25T20:12:44Z

@lambdageek @thaystg can you give me more background on the customer scenarios that are broken without this?

lewing · 2021-06-25T20:13:54Z

cc @marek-safar

thaystg · 2021-06-25T20:47:30Z

Anything like this would reproduce the problem, loading an assembly in a different load context.

        private static void Main(string[] args)
        {
            var context = new AssemblyLoadContext("testContext", true);
            Assembly a = context.LoadFromAssemblyPath("/Users/thaysgrazia/runtime/artifacts/bin/HelloWorld/x64/Release/osx-x64/Xamarin.HotReload.Agent.dll");
            Type myType = a.GetType("Xamarin.HotReload.HotReloadAgent");
            MethodInfo myMethod = myType.GetMethod("BreakpointCheckpoint");
            myMethod.Invoke(null, null);
        }

Try to add a breakpoint on BreakpointCheckpoint.
This will hit the assert.
* Assertion at /Users/thaysgrazia/runtime/src/mono/mono/mini/debugger-engine.c:1081, condition found_sp' not met`

thaystg · 2021-06-25T20:49:49Z

And it's not possible to run any android app with XAML HotReload enabled, because the app will stop in an asssert on debugger-engine.

lewing · 2021-06-25T21:27:37Z

@thaystg and I discussed how to add tests for this and she is working on that.

lewing · 2021-06-25T22:15:32Z

and it looks like android hit a bad device

JIT/RyuJIT/JIT.RyuJIT.XUnitWrapper.dll JIT/Performance/JIT.Performance.XUnitWrapper.dll JIT/SIMD/JIT.SIMD.XUnitWrapper.dll JIT/superpmi/JIT.superpmi.XUnitWrapper.dll JIT/Intrinsics/JIT.Intrinsics.XUnitWrapper.dll -parallel none -nocolor -noshadow -xml testResults.xml
Microsoft.DotNet.XUnitConsoleRunner v2.5.0 (64-bit .NET 6.0.0-preview.4.21253.7)
  Discovering: JIT.opt.XUnitWrapper (method display = ClassAndMethod, method display options = None)
  Discovered:  JIT.opt.XUnitWrapper (found 78 test cases)
  Starting:    JIT.opt.XUnitWrapper (parallel test collections = off, max threads = 2)
    JIT/opt/AssertionPropagation/ArrBoundBinaryOp/ArrBoundBinaryOp.sh [FAIL]
      
      Return code:      85
      Raw output file:      /datadisks/disk1/work/A7E50945/w/AE2B09CF/uploads/Reports/JIT.opt/AssertionPropagation/ArrBoundBinaryOp/ArrBoundBinaryOp.output.txt
      Raw output:
      BEGIN EXECUTION
      XHarness command issued: android run --instrumentation=net.dot.MonoRunner --package-name=net.dot.JIT_opt --output-directory=/datadisks/disk1/work/A7E50945/w/AE2B09CF/uploads/Reports/JIT.opt/AssertionPropagation/ArrBoundBinaryOp --arg=entrypoint:libname=ArrBoundBinaryOp.dll --expected-exit-code=100 -v
      [21:03:15] dbug: Android Run command called: App = net.dot.JIT_opt
                       
      [21:03:15] dbug: Timeout = 900 seconds.
      [21:03:15] dbug: ADBRunner using ADB.exe supplied from /datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/tools/net6.0/any/../../../runtimes/any/native/adb/linux/adb
      [21:03:15] dbug: Full resolved path:'/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb'
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  start-server'
      [21:03:15] dbug: 
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  devices -l'
      [21:03:15] dbug: Evaluating output line for device serial: emulator-5554          device product:sdk_phone_x86 model:Android_SDK_built_for_x86 device:generic_x86 transport_id:1
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  -s emulator-5554 shell pm list packages -3'
      [21:03:15] dbug: Evaluating output line for device serial: emulator-5556          device product:sdk_phone_x86_64 model:Android_SDK_built_for_x86_64 device:generic_x86_64 transport_id:7
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  -s emulator-5556 shell pm list packages -3'
      [21:03:15] fail: No devices with app 'package:net.dot.JIT_opt' was found among attached and authorized devices.
      [21:03:15] fail: Cannot find a device with app=package:net.dot.JIT_opt, please check that a device is attached
      XHarness exit code: 85 (ADB_DEVICE_ENUMERATION_FAILURE)
      Test Harness Exitcode is : 85
      To run the test:
      > set CORE_ROOT=/datadisks/disk1/work/A7E50945/p
      > /datadisks/disk1/work/A7E50945/w/AE2B09CF/e/JIT/opt/AssertionPropagation/ArrBoundBinaryOp/ArrBoundBinaryOp.sh
      Expected: True
      Actual:   False
      Stack Trace:
           at JIT_opt._AssertionPropagation_ArrBoundBinaryOp_ArrBoundBinaryOp_._AssertionPropagation_ArrBoundBinaryOp_ArrBoundBinaryOp_sh() in JIT.opt.XUnitWrapper.dll:token 0x6000005+0x2b2
      Output:
        
        Return code:      85
        Raw output file:      /datadisks/disk1/work/A7E50945/w/AE2B09CF/uploads/Reports/JIT.opt/AssertionPropagation/ArrBoundBinaryOp/ArrBoundBinaryOp.output.txt
        Raw output:
        BEGIN EXECUTION
        XHarness command issued: android run --instrumentation=net.dot.MonoRunner --package-name=net.dot.JIT_opt --output-directory=/datadisks/disk1/work/A7E50945/w/AE2B09CF/uploads/Reports/JIT.opt/AssertionPropagation/ArrBoundBinaryOp --arg=entrypoint:libname=ArrBoundBinaryOp.dll --expected-exit-code=100 -v
        [21:03:15] dbug: Android Run command called: App = net.dot.JIT_opt
                         
        [21:03:15] dbug: Timeout = 900 seconds.
        [21:03:15] dbug: ADBRunner using ADB.exe supplied from /datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/tools/net6.0/any/../../../runtimes/any/native/adb/linux/adb
        [21:03:15] dbug: Full resolved path:'/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb'
        [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  start-server'
        [21:03:15] dbug: 
        [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  devices -l'
        [21:03:15] dbug: Evaluating output line for device serial: emulator-5554          device product:sdk_phone_x86 model:Android_SDK_built_for_x86 device:generic_x86 transport_id:1
        [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  -s emulator-5554 shell pm list packages -3'
        [21:03:15] dbug: Evaluating output line for device serial: emulator-5556          device product:sdk_phone_x86_64 model:Android_SDK_built_for_x86_64 device:generic_x86_64 transport_id:7
        [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  -s emulator-5556 shell pm list packages -3'
        [21:03:15] fail: No devices with app 'package:net.dot.JIT_opt' was found among attached and authorized devices.
        [21:03:15] fail: Cannot find a device with app=package:net.dot.JIT_opt, please check that a device is attached
        XHarness exit code: 85 (ADB_DEVICE_ENUMERATION_FAILURE)
        Test Harness Exitcode is : 85
        To run the test:
        > set CORE_ROOT=/datadisks/disk1/work/A7E50945/p
        > /datadisks/disk1/work/A7E50945/w/AE2B09CF/e/JIT/opt/AssertionPropagation/ArrBoundBinaryOp/ArrBoundBinaryOp.sh
    JIT/opt/AssertionPropagation/ArrBoundMinLength/ArrBoundMinLength.sh [FAIL]
      
      Return code:      85
      Raw output file:      /datadisks/disk1/work/A7E50945/w/AE2B09CF/uploads/Reports/JIT.opt/AssertionPropagation/ArrBoundMinLength/ArrBoundMinLength.output.txt
      Raw output:
      BEGIN EXECUTION
      XHarness command issued: android run --instrumentation=net.dot.MonoRunner --package-name=net.dot.JIT_opt --output-directory=/datadisks/disk1/work/A7E50945/w/AE2B09CF/uploads/Reports/JIT.opt/AssertionPropagation/ArrBoundMinLength --arg=entrypoint:libname=ArrBoundMinLength.dll --expected-exit-code=100 -v
      [21:03:15] dbug: Android Run command called: App = net.dot.JIT_opt
                       
      [21:03:15] dbug: Timeout = 900 seconds.
      [21:03:15] dbug: ADBRunner using ADB.exe supplied from /datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/tools/net6.0/any/../../../runtimes/any/native/adb/linux/adb
      [21:03:15] dbug: Full resolved path:'/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb'
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  start-server'
      [21:03:15] dbug: 
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  devices -l'
      [21:03:15] dbug: Evaluating output line for device serial: emulator-5554          device product:sdk_phone_x86 model:Android_SDK_built_for_x86 device:generic_x86 transport_id:1
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  -s emulator-5554 shell pm list packages -3'
      [21:03:15] dbug: Evaluating output line for device serial: emulator-5556          device product:sdk_phone_x86_64 model:Android_SDK_built_for_x86_64 device:generic_x86_64 transport_id:7
      [21:03:15] dbug: Executing command: '/datadisks/disk1/work/A7E50945/p/microsoft.dotnet.xharness.cli/1.0.0-prerelease.21324.2/runtimes/any/native/adb/linux/adb  -s emulator-5556 shell pm list packages -3'
      [21:03:15] fail: No devices with app 'package:net.dot.JIT_opt' was found among attached and authorized devices.
      [21:03:15] fail: Cannot find a device with app=package:net.dot.JIT_opt, please check that a device is attached
      XHarness exit code: 85 (ADB_DEVICE_ENUMERATION_FAILURE)
      Test Harness Exitcode is : 85
      To run the test:
      > set CORE_ROOT=/datadisks/disk1/work/A7E50945/p

lewing · 2021-06-26T01:26:30Z

similar failure here https://dev.azure.com/dnceng/public/_build/results?buildId=1206935&view=logs&j=b6f63eff-75cc-59ca-fdbf-d2121601a613&t=92df95dc-ad7c-5fec-a8b4-c8301b7a2225&l=31

thaystg · 2021-06-26T02:56:10Z

Added a test case.

lewing

small nits

src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs

Co-authored-by: Larry Ewing <lewing@microsoft.com>

thaystg requested review from lambdageek and vargaz June 25, 2021 18:41

thaystg requested review from SamMonoRT and imhameed as code owners June 25, 2021 18:41

thaystg changed the title ~~[mono][debugger] Fixing side effect of #48483~~ [mono][debugger] Save and search seq_point in the same MonoJitMemoryManager Jun 25, 2021

thaystg changed the title ~~[mono][debugger] Save and search seq_point in the same MonoJitMemoryManager~~ [mono][debugger] Insert and search seq_point in the same MonoJitMemoryManager Jun 25, 2021

vargaz approved these changes Jun 25, 2021

View reviewed changes

lambdageek previously approved these changes Jun 25, 2021

View reviewed changes

thaystg added the area-Debugger-mono label Jun 25, 2021

Fixing everywhere that uses seq_points.

6faffb8

vargaz reviewed Jun 25, 2021

View reviewed changes

src/mono/mono/mini/debugger-engine.c Show resolved Hide resolved

lambdageek reviewed Jun 25, 2021

View reviewed changes

src/mono/mono/mini/seq-points.c Show resolved Hide resolved

Not used.

fd7f206

lambdageek reviewed Jun 25, 2021

View reviewed changes

src/mono/mono/mini/debugger-engine.c Show resolved Hide resolved

thaystg added 4 commits June 25, 2021 16:36

Revert "Not used."

9087db9

This reverts commit fd7f206.

Revert "Fixing everywhere that uses seq_points."

f68f74e

This reverts commit 6faffb8.

Revert "[mono][debugger] Fixing side effect of #48483"

ecc9b94

This reverts commit 44601d3.

Always use get_default_jit_mm for seq_points.

74e6cc8

thaystg requested a review from BrzVlad as a code owner June 25, 2021 19:45

lambdageek approved these changes Jun 25, 2021

View reviewed changes

thaystg added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) labels Jun 25, 2021

lewing mentioned this pull request Jun 25, 2021

[release/6.0-preview6][mono][debugger] Fix debugging with XAML hot reload enabled. #54763

Closed

lewing mentioned this pull request Jun 25, 2021

[mono][debugger] .NET6 Mobile debugger tests #54768

Closed

Adding a test case and fixing wasm debugging on ALC.

af81a76

thaystg requested a review from marek-safar as a code owner June 26, 2021 02:55

Moving implementation to avoid changing line number

f1a19e5

lewing reviewed Jun 27, 2021

View reviewed changes

src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs Outdated Show resolved Hide resolved

src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs Outdated Show resolved Hide resolved

thaystg and others added 2 commits June 27, 2021 21:47

Update src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs

cf59305

Co-authored-by: Larry Ewing <lewing@microsoft.com>

Update src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs

94df762

Co-authored-by: Larry Ewing <lewing@microsoft.com>

lewing merged commit 084392d into main Jun 28, 2021

lewing deleted the thaystg-patch-1 branch June 28, 2021 12:44

drasticactions mentioned this pull request Jul 2, 2021

[Bug] XAML HOT Reload stops at initialize with Android Emulator dotnet/maui#1528

Closed

ghost locked as resolved and limited conversation to collaborators Jul 28, 2021

Conversation

thaystg commented Jun 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jun 25, 2021

Uh oh!

ghost commented Jun 25, 2021

Uh oh!

lambdageek commented Jun 25, 2021

Uh oh!

thaystg commented Jun 25, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lambdageek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lewing commented Jun 25, 2021

Uh oh!

github-actions bot commented Jun 25, 2021

Uh oh!

github-actions bot commented Jun 25, 2021

Uh oh!

lewing commented Jun 25, 2021

Uh oh!

lewing commented Jun 25, 2021

Uh oh!

thaystg commented Jun 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaystg commented Jun 25, 2021

Uh oh!

lewing commented Jun 25, 2021

Uh oh!

lewing commented Jun 25, 2021

Uh oh!

lewing commented Jun 26, 2021

Uh oh!

thaystg commented Jun 26, 2021

Uh oh!

lewing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thaystg commented Jun 25, 2021 •

edited

Loading

lambdageek left a comment •

edited

Loading

thaystg commented Jun 25, 2021 •

edited

Loading