Skip to content
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.

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

[debugger] Fix NOT_IMPLEMENTED while debugging. (#19248) #1274

Merged
merged 1 commit into from
Mar 24, 2020

Conversation

UnityAlex
Copy link
Collaborator

  • Changed the behavior on debugger-agent, if we can't parse the new behavior is to return invalid_argument and not assert and stop debugging
  • Changed the mono_domain_set_fast before return from assembly_commands.
  • Add error message when return INVALID_ARGUMENT

Fixes mono#19146

Fixes case 1197204

- Changed the behavior on debugger-agent, if we can't parse the new behavior is to return invalid_argument and not assert and stop debugging
- Changed the mono_domain_set_fast before return from assembly_commands.
- Add error message when return INVALID_ARGUMENT

Fixes mono#19146
@UnityAlex UnityAlex requested a review from joncham March 23, 2020 15:57
@jbevain
Copy link

jbevain commented Mar 23, 2020

FYI we're fixing the VSTU side of the issue in Visual Studio 2019 16.6.

@joshpeterson
Copy link

Can we make this change in the IL2CPP debugger code as well?

@UnityAlex
Copy link
Collaborator Author

Can we make this change in the IL2CPP debugger code as well?

Yep I can bring it over to there too.

ReadInt (); // id
ReadByte (); // flags
ErrorCode = ReadShort ();
if (ErrorCode == (int)Mono.Debugger.Soft.ErrorCode.INVALID_ARGUMENT && len > offset)
ErrorMsg = ReadString ();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a protocol bump for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was not in the PR upstream: mono#19248

I can ask there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mono#19248 (comment)

Looks like the answer is "No, but yes."

@UnityAlex UnityAlex requested a review from joncham March 24, 2020 12:19
@UnityAlex
Copy link
Collaborator Author

@joncham you're going to have to merge this one because of the CLA bot

@joncham joncham merged commit 73c75f0 into unity-master Mar 24, 2020
@joncham joncham deleted the unity-master-fix-1197204 branch March 24, 2020 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash by hitting NOT_IMPLEMENTED assert during managed debugging
5 participants