[cdac] Read types from contract descriptor

[elinor-fung]

• Read/store types from contract descriptor into an internal representation of the type layout info
• Add functions to Target for getting type info and contract version
• Add placeholder for thread contract and getting thread store data
  • Add ThreadStore to data descriptor
  • GetThreadStoreData continues to return E_NOTIMPL, but will go through trying to use the contract
• Store processed data for the target by its address and data model type
• Add unit tests for getting type info

Contributes to #99298

[dotnet-policy-service]

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

[lambdageek]

Seems ok to me

I think there's a good balance between ceremony and separation of concerns, but if we're wrong, we can adjust once we have some contracts with non-trivial interactions.

[lambdageek]

Store processed data for the target by its address - assumes each address will only be processed as one type of data

thinking about Mono, we have some places where we lean on type punning kind of heavily. For example our intrusive JIT info hashes that are treated kind of abstractly by the unwinder but concretely by the execution engines (ie the JIT and interp keep their own data past the hash key/bucket link) and rely on the first member of a struct being at the same address as the whole struct. So different contracts might want to view the same address differently. But we may want to adjust the representation to be contract-friendly, instead.

[AaronRobinsonMSFT]

Why is this okay? Does the contract not really matter in this case? The below logic means we could fail to add the contract because one already exists, and then return the one we just created. This seems like we could get into a state where an implementation has state and isn't the one being used. I think we need some other invariant in the system if this pattern is considered safe.

[elinor-fung]

Agreed, this should return the existing one if it got registered in between. I don't think the contracts should really have state other than what is tied to the target itself.

[lambdageek]

I don't think the contracts should really have state other than what is tied to the target itself.

I kind of want contract implementations to be valuetypes. That way it's evident they have no identity. Not sure if that's going overboard, though

[AaronRobinsonMSFT]

That way it's evident they have no identity. Not sure if that's going overboard, though

That is the kind of architecture invariant that would work for me. It isn't a compile time enforcement, but does make people ask the right question. The counter to that though then becomes, why do we have a cache if none of the objects have state? Will the constructor have a lot of logic to set itself up?

[lambdageek]

Will the constructor have a lot of logic to set itself up?

My feeling is, yes. (well not literally the constructor, but the Create static virtual method). In the C++ prototype there was a lot of work upfront to query the target and establish that the necessary data was available. I think something similar is already evident in the Thread contract, too

[elinor-fung]

1ad1da3 turns Thread into a readonly struct - thoughts? I didn't want the additional interface originally, but after actually doing it, it doesn't seem like too much.

[lambdageek]

thoughts?

I like it, but I will also readily acknowledge we landed in a very non-idiomatic way of programming C#. But maybe with static virtuals and DIMs this is what programming with immutable values looks like in C# now?

[lambdageek]

rely on the first member of a struct being at the same address as the whole struct.

Thinking about it a little bit more, this will be true for all structs whether or not one happens to rely on it. So any C/C++ struct or class whose first member also happens to be a struct or class will have at least 2 different views of the same address. (I don't think we will want to completely flatten the data layout of every class)

[elinor-fung]

Ah, indeed. I'll switch it to be by address+type. I'm keeping it on the target, as I think we do want the cache per target rather than more contract-based / per contract.