ghost
commented
2 years ago Tagging subscribers to this area: @JulieLeeMSFT See info in area-owners.md if you want to be subscribed.
Issue Details
Runtime related code has different HResult definitions: managed part defines HResult as `struct {int Value}` while native part defines HResult as `int`. Generally it is different types and native<->managed conversion should be done explicitly. But in many cases this conversion is processed implicitly relying on expectation of same types representation on specific platform. It could produce bugs on some platforms in some cases. So happened on Linux x86 while `dotnet dump analyze` execution. In particular example native [SOSInitializeByHost](https://github.com/dotnet/diagnostics/blob/a74196805d9aea2d1ee54a2952b3c11dfee008b2/src/SOS/Strike/symbols.cpp#L52) function ```C++ extern "C" HRESULT STDMETHODCALLTYPE SOSInitializeByHost(IUnknown* punk, IDebuggerServices* debuggerServices) { IHost* host = nullptr; HRESULT hr; if (punk != nullptr) { hr = punk->QueryInterface(__uuidof(IHost), (void**)&host); if (FAILED(hr)) { return hr; } } ``` calls managed [QueryInterface](https://github.com/microsoft/clrmd/blob/be891ed2cc2c8b98d9e0b531b513b75a4d4bfd88/src/Microsoft.Diagnostics.Runtime/src/Utilities/COMInterop/ComCallableIUnknown.cs#L97) method ```C# private HResult QueryInterfaceImpl(IntPtr _, in Guid guid, out IntPtr ptr) { if (_interfaces.TryGetValue(guid, out IntPtr value)) { Interlocked.Increment(ref _refCount); ptr = value; return HResult.S_OK; } ptr = IntPtr.Zero; return HResult.E_NOINTERFACE; } ``` Native SOSInitializeByHost expects QueryInterface would [return `int`](https://github.com/dotnet/diagnostics/blob/a74196805d9aea2d1ee54a2952b3c11dfee008b2/src/shared/pal/inc/pal_mstypes.h#L662): ```C++ typedef LONG HRESULT; ``` while managed ReversePInvoke for QueryInterface [returns `struct {int Value}`](https://github.com/microsoft/clrmd/blob/554f7c1e3c93736b6d24694b7f253f260692b4f3/src/Microsoft.Diagnostics.Runtime/src/Utilities/COMInterop/HResult.cs#L18): ```C# namespace Microsoft.Diagnostics.Runtime.Utilities { public struct HResult { public const int S_OK = 0; public const int S_FALSE = 1; public const int E_FAIL = unchecked((int)0x80004005); public const int E_INVALIDARG = unchecked((int)0x80070057); public const int E_NOTIMPL = unchecked((int)0x80004001); public const int E_NOINTERFACE = unchecked((int)0x80004002); public bool IsOK => Value == S_OK; public int Value { get; set; } public HResult(int hr) => Value = hr; ``` Linux x86 platform processes structure returning in common way, no matter how large the structure is. So native code expects ret value in eax, while QueryInterface expects additional implicit arg with reference to ret structure provided by caller to fill the structure with values and mov to eax reference to the structure.
Linux x86 asm for SOSInitializeByHost and ReversePInvoke
```asm Dump of assembler code for function SOSInitializeByHost: 0xac0335f0 <+0>: push ebp 0xac0335f1 <+1>: mov ebp,esp 0xac0335f3 <+3>: push ebx 0xac0335f4 <+4>: push edi 0xac0335f5 <+5>: push esi 0xac0335f6 <+6>: and esp,0xfffffff0 0xac0335f9 <+9>: sub esp,0x20 0xac0335fc <+12>: mov eax,DWORD PTR [ebp+0x8] 0xac0335ff <+15>: mov ecx,DWORD PTR gs:0x14 0xac033606 <+22>: call 0xac03360b| Author: | t-mustafin |
|---|---|
| Assignees: | - |
| Labels: | `area-CodeGen-coreclr`, `untriaged` |
| Milestone: | - |
jkotas
AaronRobinsonMSFT
leculver
MichalStrehovsky
Runtime related code has different HResult definitions: managed part defines HResult as
struct {int Value}while native part defines HResult asint. Generally it is different types and native<->managed conversion should be done explicitly. But in many cases this conversion is processed implicitly relying on expectation of same types representation on specific platform. It could produce bugs on some platforms in some cases.So happened on Linux x86 while
dotnet dump analyzeexecution. In particular example native SOSInitializeByHost functioncalls managed QueryInterface method
Native SOSInitializeByHost expects QueryInterface would return
int:while managed ReversePInvoke for QueryInterface returns
struct {int Value}:Linux x86 platform processes structure returning in common way, no matter how large the structure is. So native code expects ret value in eax, while QueryInterface expects additional implicit arg with reference to ret structure provided by caller to fill the structure with values and mov to eax reference to the structure.
Linux x86 asm for SOSInitializeByHost and ReversePInvoke
```asm Dump of assembler code for function SOSInitializeByHost: 0xac0335f0 <+0>: push ebp 0xac0335f1 <+1>: mov ebp,esp 0xac0335f3 <+3>: push ebx 0xac0335f4 <+4>: push edi 0xac0335f5 <+5>: push esi 0xac0335f6 <+6>: and esp,0xfffffff0 0xac0335f9 <+9>: sub esp,0x20 0xac0335fc <+12>: mov eax,DWORD PTR [ebp+0x8] 0xac0335ff <+15>: mov ecx,DWORD PTR gs:0x14 0xac033606 <+22>: call 0xac03360bOn other architectures such little structures are returned via standard return value registers (
eax,r0). This non-standard platform specific feature allows implicitly convert managed HResultstruct{int}to native HResultintanywhere except Linux x86. Feature demonstration on godbolt.I can propose fix for this particular case by https://github.com/t-mustafin/clrmd/commit/98016eb854860b3b4910d720a7dbe243d28eb5ba, but it changes clrmd Public API. Maybe exists another better solution?
cc @jkotas @jakobbotsch @BruceForstall @alpencolt