Skip to content

[debugger][mono] Fix side effect of 73024 #73309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
radical merged 5 commits into from
Aug 4, 2022
Merged

[debugger][mono] Fix side effect of 73024 #73309

radical merged 5 commits into from
Aug 4, 2022

Conversation

@thaystg
Copy link
Member

@thaystg thaystg commented Aug 3, 2022 •
edited by radical
Loading

Fix side effect of #73024

What may happen is a field that has the same address of the owner, but a different type, and if this is the case will not cause the infinite recursion.
Maybe a better fix would be to pass "owner" address to buffer_add_value_full and check if they are the same when it's inside handle_vtype label.

Fixes #73342

@thaystg thaystg requested review from lewing, radical and vargaz August 3, 2022 15:53
@thaystg thaystg requested a review from marek-safar as a code owner August 3, 2022 15:53
@ghost ghost added the area-Debugger-mono label Aug 3, 2022
@ghost ghost assigned thaystg Aug 3, 2022
@ghost
Copy link

ghost commented Aug 3, 2022

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

Issue Details

Fix side effect of #73024

What may happen is a field that has the same address of the owner valuetype, but different type, and if this is the case will not cause the infinite recursion.
Maybe a better fix would be to pass "owner" address to buffer_add_value_full and check if they are the same when it's inside handle_vtype label.

Author: thaystg
Assignees: -
Labels:

area-Debugger-mono
Milestone: -

@radical
Copy link
Member

radical commented Aug 3, 2022

There are 31 tests still failing - https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-73309-merge-e4bb6e8ba165422a94/chrome-DebuggerTests.Attempt.2/1/console.071619e7.log?helixlogtype=result .

@radical
Copy link
Member

radical commented Aug 3, 2022 •
edited
Loading

One test failing:

 DebuggerTests.MiscTests.InspectLocalRecursiveFieldValue [FAIL]
  Failed DebuggerTests.MiscTests.InspectLocalRecursiveFieldValue [53 ms]
  Error Message:
   [Runtime.getProperties returned False instead of True, for {
  "objectId": "dotnet:scope:1"
}, with Result: [Result: IsOk: False, IsErr: True, Value: , Error: {
  "message": "SendDebuggerAgentCommand failed for GetValues: [Result: IsOk: False, IsErr: True, Value: ,
	Error: {\n  \"result\": {\n    \"type\": \"object\",\n    \"subtype\": \"error\",\n    \"className\": \"RangeError\",\n    
	\"description\": \"RangeError: Maximum call stack size exceeded\\n

@radical radical mentioned this pull request Aug 4, 2022
Closed
@radical radical merged commit 6ee429c into dotnet:main Aug 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@radical radical radical approved these changes

@lewing lewing Awaiting requested review from lewing

@vargaz vargaz Awaiting requested review from vargaz

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees

@thaystg thaystg

Labels

area-Debugger-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[wasm] Debugger tests with chrome broken on CI

2 participants

@thaystg @radical