Removing conversion from typehandle to methodtable

[rcj1]

No description provided.

[Copilot]

Pull Request Overview

This PR removes the explicit conversion from TypeHandle to MethodTable by directly using the TypeHandle's internal target address field instead of calling GetMethodTable().

Key Changes

• Eliminates intermediate MethodTable pointer variable
• Replaces TypeHandle::GetMethodTable() call with direct access to the internal m_asTAddr field
• Simplifies null checking by using the address field directly

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[rcj1]

@jkotas I realized that removing the conversion without any changes to SOS may not be the best approach. If I look at it in windbg, we then print out an address in the "MT" column that is actually not a method table. Then there is the command shortcut, which of course leads to "is not a MethodTable".

I don't know if there is a more meaningful approach, maybe we just set these to be displayed as zero on the SOS side. It also turns out that the "more detailed names" only appear for ValueType and Class at least now. I could look into expanding that as an enhancement.

[noahfalk]

I don't know if there is a more meaningful approach, maybe we just set these to be displayed as zero on the SOS side

I think Jan's original suggestion to pass through MethodTable pointers as-is and convert all other TypeDescs into NULL is just fine. We aren't losing much.

The other alternative is change the implementation of DumpMT to accept any TypeHandle and pretty print it, not just MethodTable pointers. I'm guessing doing that nicely may require some new DAC APIs in which case its probably better to file it as a feature suggestion for another time rather than continue to expand the scope of this PR.

[max-charlamb]

Suggested change

	FieldDescData->MTOfType = HOST_CDADDR(PTR_MethodTable(th.AsTAddr()));
	FieldDescData->MTOfType = TO_CDADDR(th.AsTAddr());

Any reason to convert the TADDR to a pointer to convert it back to a TADDR?

[max-charlamb]

I don't know if there is a more meaningful approach, maybe we just set these to be displayed as zero on the SOS side

I think Jan's original suggestion to pass through MethodTable pointers as-is and convert all other TypeDescs into NULL is just fine. We aren't losing much.

The other alternative is change the implementation of DumpMT to accept any TypeHandle and pretty print it, not just MethodTable pointers. I'm guessing doing that nicely may require some new DAC APIs in which case its probably better to file it as a feature suggestion for another time rather than continue to expand the scope of this PR.

This wouldn't require any new APIs. The cDAC already supports this behavior today. All of the APIs that take a MethodTable implicitly allow TypeDescs as an internal runtime optimization.

You are correct that the DAC implementation doesn't support this right now. If we set the field to 0 for TypeDescs, I think we should leave an issue/comment to convert it to pass in the TypeDesc when the DAC is deprecated.

[rcj1]

I think we can pass on zero for TypeDescs in both DAC and cDAC if we use our existing method table validation. In the DAC we can use DacValidateMethodTable, and in the cDAC we can expose MT validation as an API in addition to having it wrapped up in GetTypeHandle.

[rcj1]

There is another cDAC API, GetMethodTableData, whose expected behavior is to fail on non-valid method tables, but it looks like it just returns an empty data structure. If we have a way to validate method tables, we could implement the correct behavior for these. Alternatively we could throw on the methods that expect a valid method tables, such as GetBase, but I believe having a way to validate method tables is cleaner.

[max-charlamb]

L

I think we can pass on zero for TypeDescs in both DAC and cDAC if we use our existing method table validation. In the DAC we can use DacValidateMethodTable, and in the cDAC we can expose MT validation as an API in addition to having it wrapped up in GetTypeHandle.

We can differentiate using Jan’s suggestion.

#119417 (comment)

[rcj1]

L

I think we can pass on zero for TypeDescs in both DAC and cDAC if we use our existing method table validation. In the DAC we can use DacValidateMethodTable, and in the cDAC we can expose MT validation as an API in addition to having it wrapped up in GetTypeHandle.

We can differentiate using Jan’s suggestion.

#119417 (comment)

That sounds good. I think we just keep the original DAC behavior for now or pass null, and if we wanted to change it do it in conjunction with possible changes to SOS.

[rcj1]

We already have IsFunctionPointer and IsGenericVariable