[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

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

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


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

I don't know :)

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

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

+ [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

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

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

+ {
+ 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

+ {
+ 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].

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

+ 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.

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

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


👍 for the named params!

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

Comment threadsrc/mono/mono/component/debugger-engine.c Outdated

thaystgand 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@radicalcomments.
4e6f443

Addressing@radicalcomment.
21ff307

Addressing@radicalcomments.
a352202

build-analysisBot mentioned this pull request Jan 11, 2023
Build fails with "eng/common/tools.sh: line 474: 537 Segmentation fault" #76759
Closed

runfoappBot 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@radicalcomments.
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@radicalcomments
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

Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs Outdated

lambdageek mentioned this pull request Jan 20, 2023
[tracking] Publish .NET assemblies in Webcil files #80807
Closed
16 tasks

radicaland 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:mainJan 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))

Uh oh!

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!

radicalJan 4, 2023

Choose a reason for hiding this comment

Uh oh!

thaystgJan 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radicalJan 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

radicalJan 17, 2023

Choose a reason for hiding this comment

Uh oh!

radicalJan 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radicalJan 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-pipelinesBot commented Jan 19, 2023

Uh oh!

radicalJan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants