Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

ayuckhulk
Copy link

This PR is a copy of #10941 but for release/2.0.0 branch.
We will upgrade coreclr upstream branch on Tizen soon from release/2.0.0 branch.

This PR fix FieldDesc::GetExactFieldType when FieldDesc doesn't exactly represent the owner type (like in generics).

To get the exact type from FieldDesc, field signature should point past the calling convention.

@janvorli, @jkotas PTAL

cc @Dmitri-Botcharnikov @chunseoklee @seanshpark

…sent the owner type

To get the exact type from FieldDesc, field signature should point past the calling convention.
@ayuckhulk
Copy link
Author

@dotnet-bot test Windows_NT x64 Debug Build and Test

1 similar comment
@chunseoklee
Copy link

@dotnet-bot test Windows_NT x64 Debug Build and Test

@chunseoklee
Copy link

@gkhanna79 PTAL As ayuckhulk mentioned, it is need for tizen release.
@janvorli PTAL

@gkhanna79
Copy link
Member

Is there a test that needs to be added for this? Also, can you please help me understand why is this required for 2.0.0?

@chunseoklee
Copy link

@gkhanna79 please refer to #10941 for technical details.
Since current GetExactFieldType returns the wrong type, gdbjit does not show proper type to VS Tizen debugger. For tizen release, this feature is needed.

@gkhanna79
Copy link
Member

Thanks. Should any tests be added to accompany the change?

@gkhanna79
Copy link
Member

Also, aside from CI, what kind of testing has been done for the change?

@chunseoklee
Copy link

@ayuckhulk Please review gkhanna79's question.

@ayuckhulk
Copy link
Author

@gkhanna79 We have a bunch of internal tests for GDBJIT, CoreCLR and LLDB. Some of them (related to generics) were failing because of this issue.

@gkhanna79
Copy link
Member

@janvorli @jkotas Are you good with this change?

@janvorli
Copy link
Member

janvorli commented Jun 1, 2017

@gkhanna79 strange, I was sure I have commented here few hours ago, probably forgotten to push the Comment button. I don't know this stuff well so I cannot tell if there are possible side effects of this change to other coreclr stuff. I think @jkotas would know.

@jkotas
Copy link
Member

jkotas commented Jun 1, 2017

This code is only reachable from gdbjit in CoreCLR.

I am fine with this change. It has no risk for MS release.

@gkhanna79 gkhanna79 merged commit aa9c795 into dotnet:release/2.0.0 Jun 1, 2017
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants