[cdac] GetMethodDescData for jitted methods

[lambdageek]

This is enough for !PrintException without R2R methods on the stack

There's also a ReJIT contract here which just checks whether rejit is enabled.

Most of the complexity is in validating MethodDescs

Contributes to #99302
Contributes to #108553

[davidwrighton]

I've read through this and it seems like you've made good progress towards our goal here. I've a few comments about formatting + a few more general comments.

1. I see a lot of code dedicated to MethodDesc validation. Is that really necessary, and does the behavior match our existing DAC? We may run this in an environment where the full heap isn't available, and we might want code to do a best effort job of producing results. Also, are there changes to the MethodDesc validation logic here, or just refactoring to a new location? Is the validation logic documented in the data contract markdown?
2. I don't see documentation updates in the markdown yet. I see that some existing contracts look to have their implementation modified, but I don't see the corresponding documentation updates.
3. Would it be possible to build a bulleted list of the TODO items that are not assertions that are needed to make this contract work completely?

[lambdageek]

I see a lot of code dedicated to MethodDesc validation.

yes

Is that really necessary, and does the behavior match our existing DAC?

I tried to make it match the existing DAC.

Is it necessary? I'm not sure. We do need some validation - I have discovered that the !U command in SOS doesn't differentiate between a MethodDesc and an IP address and expects to be able to call GetMethodDescData with an IP address as the "method desc" argument and get E_INVALIDARG to tell it "this was not a method desc, it was an IP address"

We may run this in an environment where the full heap isn't available, and we might want code to do a best effort job of producing results.

Understood

Also, are there changes to the MethodDesc validation logic here, or just refactoring to a new location?

There is additional validation here. The version checked in previously was an incomplete snapshot (basically just establishing the internal API for validation).

Is the validation logic documented in the data contract markdown?

No. Is it part of the contract? I've been thinking of it as an artifact of the reader

[davidwrighton]

Is the validation logic documented in the data contract markdown?

No. Is it part of the contract? I've been thinking of it as an artifact of the reader

I don't know. It's certainly something that the implementation of the contract depends on to some extent, so all of the dependencies of it (such as the fields it accesses, etc) should probably be in the contract, but the actual validation logic is a more interesting question. I would leave it out of the contract spec for now, so I wouldn't worry about it this week, but given that SOS actually depends on this logic doing something useful for correct functioning, its implied that something is required to make our diagnostic stack work. I would file a bug that you didn't write contract docs for this and leave it at that.

[lambdageek]

I updated the RuntimeTypeSystem contract markdown with the new code and fixed up other contracts to match the implementations as necessary

Created issues:

• [cdac] Finish unimplemented parts of GetMethodDescData #109426
• [cdac] document TypeValidation and MethodValidation #109425