Repository: dotnet/runtime
File: src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs (C#)

Code before:

    int ISOSDacInterface.GetPrivateBinPaths(ClrDataAddress appDomain, int count, char* paths, uint* pNeeded)
    {
        // Method is not supported on CoreCLR
        int hr = HResults.E_NOTIMPL;

#if DEBUG
        if (_legacyImpl is not null)
        {
            int hrLocal = _legacyImpl.GetPrivateBinPaths(appDomain, count, paths, pNeeded);
            Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
        }
#endif

        return hr;
    }
    int ISOSDacInterface.GetRCWData(ClrDataAddress addr, void* data)
        => _legacyImpl is not null ? _legacyImpl.GetRCWData(addr, data) : HResults.E_NOTIMPL;
    int ISOSDacInterface.GetRCWInterfaces(ClrDataAddress rcw, uint count, void* interfaces, uint* pNeeded)
        => _legacyImpl is not null ? _legacyImpl.GetRCWInterfaces(rcw, count, interfaces, pNeeded) : HResults.E_NOTIMPL;
    int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint* pNeeded)
        => _legacyImpl is not null ? _legacyImpl.GetRegisterName(regName, count, buffer, pNeeded) : HResults.E_NOTIMPL;
    int ISOSDacInterface.GetStackLimits(ClrDataAddress threadPtr, ClrDataAddress* lower, ClrDataAddress* upper, ClrDataAddress* fp)
    {
        if (threadPtr == 0 || (lower == null && upper == null && fp == null))
            return HResults.E_INVALIDARG;

        int hr = HResults.S_OK;
        try
        {
            Contracts.IThread contract = _target.Contracts.Thread;
            Contracts.StackLimitData stackLimitData = contract.GetStackLimitData(threadPtr.ToTargetPointer(_target));

            if (lower != null)
                *lower = stackLimitData.StackBase.ToClrDataAddress(_target);

            if (upper != null)
                *upper = stackLimitData.StackLimit.ToClrDataAddress(_target);

            if (fp != null)
                *fp = stackLimitData.FrameAddress.ToClrDataAddress(_target);
        }
        catch (global::System.Exception ex)
        {
            hr = ex.HResult;
        }

#if DEBUG
        if (_legacyImpl is not null)
        {
            ClrDataAddress lowerLocal, upperLocal, fpLocal;

Review comment (bug):
Please leave only `int hr = HResults.S_OK` before the try catch and place the invalid arg check inside. That way, if we have invalid args, we can throw an `ArgumentException`, catch it, and subsequently do the debug check instead of immediately returning.

Code after:

    int ISOSDacInterface.GetPrivateBinPaths(ClrDataAddress appDomain, int count, char* paths, uint* pNeeded)
    {
        // Method is not supported on CoreCLR
        int hr = HResults.E_NOTIMPL;

#if DEBUG
        if (_legacyImpl is not null)
        {
            int hrLocal = _legacyImpl.GetPrivateBinPaths(appDomain, count, paths, pNeeded);
            Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}");
        }
#endif

        return hr;
    }
    int ISOSDacInterface.GetRCWData(ClrDataAddress addr, void* data)
        => _legacyImpl is not null ? _legacyImpl.GetRCWData(addr, data) : HResults.E_NOTIMPL;
    int ISOSDacInterface.GetRCWInterfaces(ClrDataAddress rcw, uint count, void* interfaces, uint* pNeeded)
        => _legacyImpl is not null ? _legacyImpl.GetRCWInterfaces(rcw, count, interfaces, pNeeded) : HResults.E_NOTIMPL;
    int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint* pNeeded)
        => _legacyImpl is not null ? _legacyImpl.GetRegisterName(regName, count, buffer, pNeeded) : HResults.E_NOTIMPL;
    int ISOSDacInterface.GetStackLimits(ClrDataAddress threadPtr, ClrDataAddress* lower, ClrDataAddress* upper, ClrDataAddress* fp)
    {
        int hr = HResults.S_OK;
        try
        {
            if (threadPtr == 0 || (lower == null && upper == null && fp == null))
                throw new ArgumentException();

            Contracts.IThread contract = _target.Contracts.Thread;
            TargetPointer stackBase, stackLimit, frameAddress;
            contract.GetStackLimitData(threadPtr.ToTargetPointer(_target), out stackBase, out stackLimit, out frameAddress);

            if (lower != null)
                *lower = stackBase.ToClrDataAddress(_target);

            if (upper != null)
                *upper = stackLimit.ToClrDataAddress(_target);

            if (fp != null)
                *fp = frameAddress.ToClrDataAddress(_target);
        }
        catch (global::System.Exception ex)
        {
            hr = ex.HResult;
        }

#if DEBUG
        if (_legacyImpl is not null)
        {