[cdac] Add basic cdacreader project

[elinor-fung]

This change adds a basic cdacreader project under src/native/managed.

• Built as part of the clr subset (via runtime-prereqs referencing compile-native.proj)
• Not yet integrated into anything in the product (that is, the dac doesn't try to use it yet)
• Can return a class that can implement the ISOSDacInterface* interfaces - currently does ISOSDacInterface9

Contributes to #99298
cc @AaronRobinsonMSFT @davidwrighton @jkotas @mikem8361 @noahfalk

Manually tested with:

intptr_t handle;
cdac_reader_init_fn(0, &handle);

IUnknown* unk;
cdac_reader_get_sos_interface_fn(handle, &unk);
ISOSDacInterface9* sos_dac;
HRESULT hr = unk->QueryInterface<ISOSDacInterface9>(&sos_dac);
unk->Release();
if (FAILED(hr))
    return hr;

int version;
int ret = sos_dac->GetBreakingChangeVersion(&version);
sos_dac->Release();
if (ret == 0)
    std::cout << "Version: " << version << std::endl;

cdac_reader_free_fn(handle);

[dotnet-policy-service]

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

[AaronRobinsonMSFT]

Do we want to adopt an HRESULT pattern for error codes or more of a POSIX pattern?

[mikem8361]

Is the Target class the data target (like ICLRDataTarget) that will have read memory etc. methods on it?

[lambdageek]

It's Target from https://github.com/dotnet/runtime/blob/main/docs/design/datacontracts/contract_csharp_api_design.cs#L85

it's the cDAC's internal abstraction for the target process

[mikem8361]

How does hosting code provide a ICLRDataTarget instance?

[lambdageek]

That's not wired up yet in this PR, but in the previous prototypes the cDAC exported a function like:

typedef cdac_reader_result_t (*cdac_reader_func_t) (cdac_reader_foreignptr_t addr, uint32_t count, void* user_data, uint8_t *dest);

cdac_reader_result_t cdac_reader_set_reader_func(cdac_reader_h handle, cdac_reader_func_t reader, void* user_data);

that was called from the existing DAC, passing a callback that wrapped ReadDataFromTarget:

https://github.com/lambdageek/runtime/blob/ab6e9603222842034c4f4452816e14371db450d9/src/coreclr/debug/daccess/cdac.cpp#L262-L296

[elinor-fung]

Do we want to adopt an HRESULT pattern for error codes or more of a POSIX pattern?

I don't feel strongly either way, but I went HRESULT of negative being error since that is what exports on coreclr and hosting components do.

[jkotas]

This API represents versions of (poorly defined) set of algorithmic and data contracts that SOS and CLRMD happens to depend on. It should eventually obsoleted, and SOS and CLRMD switched to use the versions of properly defined contracts.

(It is ok to keep it for now if it helps.)

[mikem8361]

Are we trying to implement the actual C++ ISOSDacInterface9 interface here (HRESULT GetBreakingChangeVersion(int* pVersion))? Or is this part of the new C# cDAC interface being defined?

[lambdageek]

Are we trying to implement the actual C++ ISOSDacInterface9 interface here ? Or is this part of the new C# cDAC interface being defined?

We're trying to implement the actual ISOSDacInterface9 here. This is going to be consumed by the existing DAC in cases where it will delegate to the cdacreader.

Or is this part of the new C# cDAC interface being defined?

It is not a goal right now to define a public interface to the cDAC

[mikem8361]

Then shouldn't the C# method be public int GetBreakingChangeVersion(out int version)? Or is there some magic (code generation/COM interop) that will expose the C++ function HRESULT GetBreakingChangeVersion(int* pVersion)?

[AaronRobinsonMSFT]

that will expose the C++ function HRESULT GetBreakingChangeVersion(int* pVersion)?

COM interop has elided the HRESULT by default since at least .NET Framework 2.0. We can have a matching signature as you've suggested, but the out isn't correct either since it is unmanaged memory, so it should be int*. I would prefer we either match the signature precisely using blittable types or we write it in canonical .NET form. In this case that means using the last "out" param as the return and letting the marshalling system throw an Exception for a failing HRESULT or convert an Exception to an HRESULT in the other direction.

The one caveat for this is when S_FALSE is used. In cases where an API can return a non-zero success HRESULT we need to use the PreserveSig mechanism and check the int manually.

[elinor-fung]

It should eventually obsoleted, and SOS and CLRMD switched to use the versions of properly defined contracts.

Agreed. At least for now, since all the sos commands use it, I chose it as the simple starting point for putting together what @lambdageek mentions about the existing DAC consuming/delegating to the cdacreader.