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

[sdb] Fix support for async debugging in optimized mode, roslyn gener… #5442

Closed
wants to merge 1 commit into from

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Aug 24, 2017

…ates valuetype IAsyncStateMethod implementations. Fixes #58728.

/* Roslyn generates valuetypes in optimize mode */
/* This is ok to do because the caller will not modify the object */
return mono_value_box_checked (frame->domain, frame->method->klass, *(gpointer*)addr, &error);
mono_error_assert_ok (&error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not make sense to assert after return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

…ates valuetype IAsyncStateMethod implementations. Fixes #58728.
MonoError error;
/* Roslyn generates valuetypes in optimize mode */
/* This is ok to do because the caller will not modify the object */
MonoObject *obj = mono_value_box_checked (frame->domain, frame->method->klass, *(gpointer*)addr, &error);
Copy link
Contributor

Choose a reason for hiding this comment

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

So addr is a pointer to a pointer to the value type, not a pointer to the value type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be wrong, the test suite doesn't hit the code path which actually uses the object, and it just uses the object as an id, so this fix is probably wrong.

@marek-safar
Copy link
Member

This looks like simple fix, could we get it to -08, -06 -04 branches

@vargaz vargaz closed this Aug 24, 2017
@baulig
Copy link
Contributor

baulig commented Aug 25, 2017

There are more locations than just that one.

@vargaz
Copy link
Contributor Author

vargaz commented Aug 25, 2017

So the result of the get_this () function is used as an id in parts of the code, so this patch which boxes the valuetype will not work.

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.

None yet

5 participants