[wasm][debugger] Implement support to symbolOptions from dap. by thaystg · Pull Request #79284 · dotnet/runtime

thaystg · 2022-12-06T16:46:11Z

Receive the symbolOptions settings from VS and use it do load debug information.

related to: microsoft/vscode-js-debug#1467

ghost · 2022-12-06T16:46:23Z

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

Issue Details

Receive the symbolOptions settings from VS and use it do load debug information.

related to: microsoft/vscode-js-debug#1467

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

…lOptions

…g to get symbols.

…lOptions

src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

radical · 2023-01-04T22:50:09Z

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

+
+                string keyTrim = key.TrimEnd('*');
+
+                if (document.StartsWith(keyTrim, StringComparison.OrdinalIgnoreCase))


Curious - does this need to be culture sensitive? And case?

I don't know :)

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

Co-authored-by: Larry Ewing <lewing@microsoft.com> Co-authored-by: Ankit Jain <radical@gmail.com>

src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj

radical · 2023-01-13T17:37:47Z

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

+        [ConditionalTheory(nameof(RunningOnChrome))]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task SteppingIntoLibrarySymbolsLoadedFromSymbolServer(bool justMyCode)


Maybe also:

a test that first hits the bp, and steps with no symbol server set. Then in the same test, sets the symbol server, and steps again

A separate test that (a) sets a symbol server; (b) steps; (c) adds another symbol server; (d) steps again

A separate test that (a) adds multiple symbol servers; (b) steps; (c) removes one of the symbol servers; (d) steps again

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

Co-authored-by: Ankit Jain <radical@gmail.com>

radical · 2023-01-17T18:39:37Z

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

+        {
+            List<AssemblyInfo> ret = new List<AssemblyInfo> ();
+            if (symbolStore == null)
+                return ret;


return Enumerable.Empty<AssemblyInfo> instead. It will save on the unnecessary allocation which will have a non-zero sized backing array.

radical · 2023-01-17T18:42:36Z

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

+            {
+                if (string.IsNullOrEmpty(urlServer))
+                    continue;
+                symbolStore = new HttpSymbolStore(_tracer, symbolStore, new Uri($"{urlServer}/"), null);


This should handle exceptions, and continue to try the remaining urls.

And a corresponding test with [ goodUrl1, badUrl, goodUrl2].

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

radical · 2023-01-17T19:07:56Z

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

+        internal void UpdatePdbInformationFromSymbolServer(Stream streamToReadFrom)
        {
            var pdbStream = new MemoryStream();
+            streamToReadFrom.Position = 0;
            streamToReadFrom.CopyTo(pdbStream);
+            pdbStream.Position = 0;
            pdbMetadataReader = MetadataReaderProvider.FromPortablePdbStream(pdbStream).GetMetadataReader();
+            if (pdbMetadataReader == null)
+                return;
+            ProcessSourceLink();
+            foreach (var method in this.Methods)
+            {
+                method.Value.pdbMetadataReader = pdbMetadataReader;
+                method.Value.UpdatePdbInformation(method.Value.methodDefHandle);
+            }
        }


This can be merged with LoadPDBFromSymbolServer.

src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

Changing when the symbols from symbol server is loaded because it takes a long time to load, as there are a lot of assemblies loaded not found on symbol servers.

radical

Other than the suggested tests, LGTM! Thank you for patiently working with the feedback! And for pursuing the whole process to make this happen!!

@radical

Removing timeout change as @radical has split it into 2 files Fixing test behavior, when justMyCode is disabled but the symbols are not loaded from symbol server.

thaystg · 2023-01-19T19:10:22Z

/azp run runtime-wasm-non-libtests

azure-pipelines · 2023-01-19T19:10:41Z

Azure Pipelines successfully started running 1 pipeline(s).

radical · 2023-01-20T02:36:08Z

src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs

-                            context.IsSteppingThroughMethod = true;
+                        if (event_kind == EventKind.Step)
+                            context.IsSkippingHiddenMethod = true;
                        if (await SkipMethod(isSkippable: true, shouldBeSkipped: true, StepKind.Out))


👍 for the named params!

src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs

- Don't call `UpdateSymbolStore` from `DebugStore..ctor` because that gets called multiple times in `LoadStore`, but only once instance gets used. - Use an isolated symbol cache path per test

# Conflicts: # src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

@radical

…#79284) * Draft to implement support to symbolOptions from dap. * removing debugger.launch * Adding tests and fix compilation error * Adding test case. * Fix test cases, and implement support to PDBChecksum used on nuget.org to get symbols. * merge * Fixing tests. * Tests are timing out. * Apply suggestions from code review Co-authored-by: Larry Ewing <lewing@microsoft.com> Co-authored-by: Ankit Jain <radical@gmail.com> * adressing @radical comments. * Addressing @radical comment. * Addressing @radical comments. * Apply suggestions from code review Co-authored-by: Ankit Jain <radical@gmail.com> * Addressing @radical comments. * Addressing @radical comments Changing when the symbols from symbol server is loaded because it takes a long time to load, as there are a lot of assemblies loaded not found on symbol servers. * use MicrosoftCodeAnalysisCSharpVersion for scripting package. * Adding more tests as asked by @radical Removing timeout change as @radical has split it into 2 files Fixing test behavior, when justMyCode is disabled but the symbols are not loaded from symbol server. * [wasm] some cleanup - Don't call `UpdateSymbolStore` from `DebugStore..ctor` because that gets called multiple times in `LoadStore`, but only once instance gets used. - Use an isolated symbol cache path per test * remove debug spew * Addressing radical comment. Co-authored-by: Larry Ewing <lewing@microsoft.com> Co-authored-by: Ankit Jain <radical@gmail.com>

Draft to implement support to symbolOptions from dap.

fcf126b

ghost added the area-Debugger-mono label Dec 6, 2022

ghost assigned thaystg Dec 6, 2022

Merge remote-tracking branch 'upstream/main' into thays_support_symbo…

2d191e2

…lOptions

thaystg mentioned this pull request Dec 6, 2022

[wasm] Implementing support to supportsSetSymbolOptions microsoft/vscode-js-debug#1467

Merged

thaystg added 2 commits December 6, 2022 13:58

removing debugger.launch

2850e02

Adding tests and fix compilation error

e00341e

build-analysis bot mentioned this pull request Dec 6, 2022

iOS & tvOS legs are failing to AOT System.Net.Http.Json #79279

Closed

thaystg added 4 commits December 6, 2022 19:39

Adding test case.

d9edba7

Fix test cases, and implement support to PDBChecksum used on nuget.or…

524531f

…g to get symbols.

Merge remote-tracking branch 'upstream/main' into thays_support_symbo…

fa2899a

…lOptions

merge

2c3dddd

build-analysis bot mentioned this pull request Dec 13, 2022

[wasm] Library tests failing during linking for AOT - SIGKILL #79569

Closed

thaystg added 2 commits December 19, 2022 15:45

Fixing tests.

8218e60

Tests are timing out.

5e62ffe

build-analysis bot mentioned this pull request Dec 20, 2022

Tracking issue for CI build timeouts #76454

Closed

thaystg marked this pull request as ready for review December 20, 2022 13:31

thaystg requested review from lewing, marek-safar and radical as code owners December 20, 2022 13:31

radical reviewed Jan 4, 2023

View reviewed changes

lewing reviewed Jan 5, 2023

View reviewed changes

src/mono/mono/component/debugger-engine.c Outdated Show resolved Hide resolved

thaystg and others added 4 commits January 6, 2023 15:34

Apply suggestions from code review

5374f0a

Co-authored-by: Larry Ewing <lewing@microsoft.com> Co-authored-by: Ankit Jain <radical@gmail.com>

adressing @radical comments.

4e6f443

Addressing @radical comment.

21ff307

Addressing @radical comments.

a352202

build-analysis bot mentioned this pull request Jan 11, 2023

Build fails with "eng/common/tools.sh: line 474: 537 Segmentation fault" #76759

Closed

runfoapp bot mentioned this pull request Jan 11, 2023

Long Running Test: Interop/MonoAPI/MonoMono/PInvokeDetach/PInvokeDetach.sh #73040

Closed

radical reviewed Jan 13, 2023

View reviewed changes

Apply suggestions from code review

c954ad3

Co-authored-by: Ankit Jain <radical@gmail.com>

ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 17, 2023

Addressing @radical comments.

9c384a4

radical requested changes Jan 17, 2023

View reviewed changes

ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 17, 2023

Addressing @radical comments

5648b8f

Changing when the symbols from symbol server is loaded because it takes a long time to load, as there are a lot of assemblies loaded not found on symbol servers.

ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 18, 2023

thaystg added 2 commits January 18, 2023 18:15

Merge branch 'main' into thays_support_symbolOptions

3366e20

use MicrosoftCodeAnalysisCSharpVersion for scripting package.

8f00d90

radical approved these changes Jan 18, 2023

View reviewed changes

This was referenced Jan 19, 2023

Infra improvements for Helix #68176

Closed

Methodical_others test JIT/Methodical/Coverage/copy_prop_byref_to_native_int crashing #69832

Open

thaystg added 2 commits January 19, 2023 15:37

Adding more tests as asked by @radical

af713af

Removing timeout change as @radical has split it into 2 files Fixing test behavior, when justMyCode is disabled but the symbols are not loaded from symbol server.

Merge branch 'main' into thays_support_symbolOptions

04e3372

lewing mentioned this pull request Jan 19, 2023

[mono][wasm] Bundle assemblies as WebCIL #79416

Merged

3 tasks

Merge branch 'main' into thays_support_symbolOptions

ecc3730

radical reviewed Jan 20, 2023

View reviewed changes

src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs Outdated Show resolved Hide resolved

lambdageek mentioned this pull request Jan 20, 2023

[tracking] Publish .NET assemblies in Webcil files #80807

Closed

16 tasks

radical and others added 4 commits January 20, 2023 22:57

[wasm] some cleanup

9ad0aa5

- Don't call `UpdateSymbolStore` from `DebugStore..ctor` because that gets called multiple times in `LoadStore`, but only once instance gets used. - Use an isolated symbol cache path per test

remove debug spew

5ca16da

Merge remote-tracking branch 'origin' into thays_support_symbolOptions

b48ba60

# Conflicts: # src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs

Addressing radical comment.

8bf7b71

thaystg merged commit 4c992b1 into dotnet:main Jan 23, 2023

thaystg mentioned this pull request Feb 6, 2023

What's new in .NET 8 Preview 1 dotnet/core#8133

Closed

3 tasks

ghost locked as resolved and limited conversation to collaborators Feb 23, 2023


		string keyTrim = key.TrimEnd('*');

		if (document.StartsWith(keyTrim, StringComparison.OrdinalIgnoreCase))

Conversation

thaystg commented Dec 6, 2022

Uh oh!

ghost commented Dec 6, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical Jan 4, 2023

Choose a reason for hiding this comment

Uh oh!

thaystg Jan 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

radical Jan 17, 2023

Choose a reason for hiding this comment

Uh oh!

radical Jan 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical Jan 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

thaystg commented Jan 19, 2023

Uh oh!

azure-pipelines bot commented Jan 19, 2023

Uh oh!

radical Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants