Skip to content

Guard remaining IErrorInfo usage under FEATURE_COMINTEROP #103510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 27, 2024
Merged

Guard remaining IErrorInfo usage under FEATURE_COMINTEROP #103510

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c668ca0
Guard GetErrorInfo and SetErrorInfo under COMINTEROP
huoyaoyuan Jun 15, 2024
782cacd
Guard more IErrorInfo usage
huoyaoyuan Jun 15, 2024
c50c0e1
Guard GetExceptionForHR usage
huoyaoyuan Jun 15, 2024
938bfa4
Guard RealCOMPlusThrowHR
huoyaoyuan Jun 15, 2024
6700f23
Delete dummy oaild.h
huoyaoyuan Jun 15, 2024
dfcbd57
Fix m_pErrorInfo initialization
huoyaoyuan Jun 15, 2024
5a000eb
Apply suggestions from code review
huoyaoyuan Jun 24, 2024
ef99f9b
Update comment in GetExceptionForHR
huoyaoyuan Jun 24, 2024
1279a35
Update contract
huoyaoyuan Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/coreclr/inc/ex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,10 @@ class Exception
virtual BOOL IsDomainBound() {return m_innerException!=NULL && m_innerException->IsDomainBound();} ;
virtual HRESULT GetHR() = 0;
virtual void GetMessage(SString &s);
#ifdef FEATURE_COMINTEROP
virtual IErrorInfo *GetErrorInfo() { LIMITED_METHOD_CONTRACT; return NULL; }
virtual HRESULT SetErrorInfo() { LIMITED_METHOD_CONTRACT; return S_OK; }
#endif
void SetInnerException(Exception * pInnerException) { LIMITED_METHOD_CONTRACT; m_innerException = pInnerException; }

// Dynamic type query for catchers
Expand Down Expand Up @@ -432,8 +434,10 @@ class COMException : public HRException
{
friend bool DebugIsEECxxExceptionPointer(void* pv);

#ifdef FEATURE_COMINTEROP
private:
IErrorInfo *m_pErrorInfo;
#endif

public:
COMException();
Expand Down Expand Up @@ -488,7 +492,9 @@ class SEHException : public Exception

// Virtual overrides
HRESULT GetHR();
#ifdef FEATURE_COMINTEROP
IErrorInfo *GetErrorInfo();
#endif
void GetMessage(SString &result);

protected:
Expand Down Expand Up @@ -533,7 +539,9 @@ class DelegatingException : public Exception
// Virtual overrides
virtual BOOL IsDomainBound() {return Exception::IsDomainBound() ||(m_delegatedException!=NULL && m_delegatedException->IsDomainBound());} ;
HRESULT GetHR();
#ifdef FEATURE_COMINTEROP
IErrorInfo *GetErrorInfo();
#endif
void GetMessage(SString &result);
virtual Exception *Clone();

Expand Down Expand Up @@ -1290,15 +1298,19 @@ inline HRMsgException::HRMsgException(HRESULT hr, SString const &s)
}

inline COMException::COMException()
: HRException(),
m_pErrorInfo(NULL)
: HRException()
#ifdef FEATURE_COMINTEROP
, m_pErrorInfo(NULL)
#endif
{
WRAPPER_NO_CONTRACT;
}

inline COMException::COMException(HRESULT hr)
: HRException(hr),
m_pErrorInfo(NULL)
: HRException(hr)
#ifdef FEATURE_COMINTEROP
, m_pErrorInfo(NULL)
#endif
{
LIMITED_METHOD_CONTRACT;
}
Expand Down
18 changes: 0 additions & 18 deletions src/coreclr/pal/inc/rt/oaidl.h
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/coreclr/pal/inc/rt/ole2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@

#include "objidl.h"
#include "servprov.h"
#include "oaidl.h"

1 change: 0 additions & 1 deletion src/coreclr/pal/inc/rt/oleauto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#ifndef _OLEAUTO_H_
#define _OLEAUTO_H_
#include "oaidl.h"

#ifndef BEGIN_INTERFACE
#define BEGIN_INTERFACE
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/pal/prebuilt/inc/metahost.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ typedef interface ICLRRuntimeInfo ICLRRuntimeInfo;

/* header files for imported files */
#include "unknwn.h"
#include "oaidl.h"
#include "ocidl.h"
#include "mscoree.h"

Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/utilcode/ex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,11 +811,13 @@ HRESULT SEHException::GetHR()
return m_exception.ExceptionCode;
}

#ifdef FEATURE_COMINTEROP
IErrorInfo *SEHException::GetErrorInfo()
{
LIMITED_METHOD_CONTRACT;
return NULL;
}
#endif

void SEHException::GetMessage(SString &string)
{
Expand Down Expand Up @@ -896,6 +898,7 @@ HRESULT DelegatingException::GetHR()

} // HRESULT DelegatingException::GetHR()

#ifdef FEATURE_COMINTEROP
//------------------------------------------------------------------------------
IErrorInfo *DelegatingException::GetErrorInfo()
{
Expand All @@ -909,6 +912,7 @@ IErrorInfo *DelegatingException::GetErrorInfo()
return pDelegate ? pDelegate->GetErrorInfo() : NULL;

} // IErrorInfo *DelegatingException::GetErrorInfo()
#endif

//------------------------------------------------------------------------------
void DelegatingException::GetMessage(SString &result)
Expand Down
16 changes: 4 additions & 12 deletions src/coreclr/vm/clrex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,18 +366,6 @@ IErrorInfo *CLRException::GetErrorInfo()
// return the IErrorInfo we got...
return pErrorInfo;
}
#else // FEATURE_COMINTEROP
IErrorInfo *CLRException::GetErrorInfo()
{
LIMITED_METHOD_CONTRACT;
return NULL;
}
HRESULT CLRException::SetErrorInfo()
{
LIMITED_METHOD_CONTRACT;

return S_OK;
}
#endif // FEATURE_COMINTEROP

void CLRException::GetMessage(SString &result)
Expand Down Expand Up @@ -698,13 +686,15 @@ OBJECTREF CLRException::GetThrowableFromException(Exception *pException)
}
else
{
#ifdef FEATURE_COMINTEROP
SafeComHolder<IErrorInfo> pErrInfo(pException->GetErrorInfo());

if (pErrInfo != NULL)
{
GetExceptionForHR(hr, pErrInfo, &oRetVal);
}
else
#endif
{
SString message;
pException->GetMessage(message);
Expand Down Expand Up @@ -938,12 +928,14 @@ HRESULT EEException::GetHR()
return EEException::GetHRFromKind(m_kind);
}

#ifdef FEATURE_COMINTEROP
IErrorInfo *EEException::GetErrorInfo()
{
LIMITED_METHOD_CONTRACT;

return NULL;
}
#endif

BOOL EEException::GetThrowableMessage(SString &result)
{
Expand Down
17 changes: 17 additions & 0 deletions src/coreclr/vm/clrex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ class CLRException : public Exception
}

HRESULT GetHR();
#ifdef FEATURE_COMINTEROP
IErrorInfo *GetErrorInfo();
HRESULT SetErrorInfo();
#endif

void GetMessage(SString &result);

Expand Down Expand Up @@ -222,7 +224,9 @@ class EEException : public CLRException

// Virtual overrides
HRESULT GetHR();
#ifdef FEATURE_COMINTEROP
IErrorInfo *GetErrorInfo();
#endif
void GetMessage(SString &result);
OBJECTREF CreateThrowable();

Expand Down Expand Up @@ -842,6 +846,7 @@ LONG CLRNoCatchHandler(EXCEPTION_POINTERS* pExceptionInfo, PVOID pv);
//
// Thus, the scoped use of FAULT_NOT_FATAL macro.
#undef EX_CATCH_HRESULT
#ifdef FEATURE_COMINTEROP
#define EX_CATCH_HRESULT(_hr) \
EX_CATCH \
{ \
Expand All @@ -857,6 +862,18 @@ LONG CLRNoCatchHandler(EXCEPTION_POINTERS* pExceptionInfo, PVOID pv);
_ASSERTE(FAILED(_hr)); \
} \
EX_END_CATCH(SwallowAllExceptions)
#else // FEATURE_COMINTEROP
#define EX_CATCH_HRESULT(_hr) \
EX_CATCH \
{ \
(_hr) = GET_EXCEPTION()->GetHR(); \
{ \
FAULT_NOT_FATAL(); \
} \
_ASSERTE(FAILED(_hr)); \
} \
EX_END_CATCH(SwallowAllExceptions)
#endif // FEATURE_COMINTEROP

#endif // !DACCESS_COMPILE

Expand Down
38 changes: 28 additions & 10 deletions src/coreclr/vm/excep.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2989,7 +2989,6 @@ void FreeExceptionData(ExceptionData *pedata)
if (pedata->bstrHelpFile)
SysFreeString(pedata->bstrHelpFile);
}
#endif // FEATURE_COMINTEROP

void GetExceptionForHR(HRESULT hr, IErrorInfo* pErrInfo, OBJECTREF* pProtectedThrowable)
{
Expand All @@ -3005,7 +3004,6 @@ void GetExceptionForHR(HRESULT hr, IErrorInfo* pErrInfo, OBJECTREF* pProtectedTh
// Initialize
*pProtectedThrowable = NULL;

#if defined(FEATURE_COMINTEROP)
if (pErrInfo != NULL)
{
// If this represents a managed object...
Expand Down Expand Up @@ -3039,7 +3037,6 @@ void GetExceptionForHR(HRESULT hr, IErrorInfo* pErrInfo, OBJECTREF* pProtectedTh
(*pProtectedThrowable) = ex.GetThrowable();
}
}
#endif // defined(FEATURE_COMINTEROP)

// If we made it here and we don't have an exception object, we didn't have a valid IErrorInfo
// so we'll create an exception based solely on the hresult.
Expand All @@ -3062,14 +3059,26 @@ void GetExceptionForHR(HRESULT hr, OBJECTREF* pProtectedThrowable)

// Get an IErrorInfo if one is available.
IErrorInfo *pErrInfo = NULL;
#ifdef FEATURE_COMINTEROP
if (SafeGetErrorInfo(&pErrInfo) != S_OK)
pErrInfo = NULL;
#endif // FEATURE_COMINTEROP

GetExceptionForHR(hr, pErrInfo, pProtectedThrowable);
}
#else
void GetExceptionForHR(HRESULT hr, OBJECTREF* pProtectedThrowable)
{
CONTRACTL
{
THROWS;
GC_TRIGGERS; // because of IErrorInfo
MODE_ANY;
}
CONTRACTL_END;

EEMessageException ex(hr);
(*pProtectedThrowable) = ex.GetThrowable();
}
#endif // FEATURE_COMINTEROP

//
// Maps a Win32 fault to a COM+ Exception enumeration code
Expand Down Expand Up @@ -11202,6 +11211,7 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowNonLocalized(RuntimeExceptionKind reKind,
//==========================================================================
// Throw a runtime exception based on an HResult
//==========================================================================
#ifdef FEATURE_COMINTEROP
VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, IErrorInfo* pErrInfo, Exception * pInnerException)
{
CONTRACTL
Expand All @@ -11224,7 +11234,6 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, IErrorInfo* pErrInfo, Exce
//_ASSERTE((hr != COR_E_EXECUTIONENGINE) ||
// !"ExecutionEngineException shouldn't be thrown. Use EEPolicy to failfast or a better exception. The caller of this function should modify their code.");

#ifdef FEATURE_COMINTEROP
// check for complus created IErrorInfo pointers
if (pErrInfo != NULL)
{
Expand All @@ -11238,9 +11247,7 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, IErrorInfo* pErrInfo, Exce
GCPROTECT_END ();
}
}
#endif // FEATURE_COMINTEROP

_ASSERTE((pErrInfo == NULL) || !"pErrInfo should always be null when FEATURE_COMINTEROP is disabled.");
if (pInnerException == NULL)
{
EX_THROW(EEMessageException, (hr));
Expand Down Expand Up @@ -11272,8 +11279,6 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr)
RealCOMPlusThrowHR(hr, (IErrorInfo*)NULL);
}


#ifdef FEATURE_COMINTEROP
VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, tagGetErrorInfo)
{
CONTRACTL
Expand All @@ -11293,6 +11298,19 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, tagGetErrorInfo)
// Throw the exception.
RealCOMPlusThrowHR(hr, pErrInfo);
}
#else // FEATURE_COMINTEROP
VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr)
{
CONTRACTL
{
THROWS;
DISABLED(GC_NOTRIGGER); // Must sanitize first pass handling to enable this
MODE_ANY;
}
CONTRACTL_END;

EX_THROW(EEMessageException, (hr));
}
#endif // FEATURE_COMINTEROP


Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/vm/excep.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,9 @@ VOID DECLSPEC_NORETURN RealCOMPlusThrow(RuntimeExceptionKind reKind, UINT resID
// passed as the first substitution string (%1).
//==========================================================================

#ifdef FEATURE_COMINTEROP
VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, IErrorInfo* pErrInfo, Exception * pInnerException = NULL);
#endif
VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr);
VOID DECLSPEC_NORETURN RealCOMPlusThrowHR(HRESULT hr, UINT resID, LPCWSTR wszArg1 = NULL, LPCWSTR wszArg2 = NULL,
LPCWSTR wszArg3 = NULL, LPCWSTR wszArg4 = NULL, LPCWSTR wszArg5 = NULL,
Expand Down Expand Up @@ -359,7 +361,9 @@ void ExceptionPreserveStackTrace(OBJECTREF throwable);
// Create an exception object for an HRESULT
//==========================================================================

#ifdef FEATURE_COMINTEROP
void GetExceptionForHR(HRESULT hr, IErrorInfo* pErrInfo, OBJECTREF* pProtectedThrowable);
#endif
void GetExceptionForHR(HRESULT hr, OBJECTREF* pProtectedThrowable);
HRESULT GetHRFromThrowable(OBJECTREF throwable);

Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/vm/marshalnative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,9 @@ extern "C" void QCALLTYPE MarshalNative_GetExceptionForHR(INT32 errorCode, LPVOI

BEGIN_QCALL;

#ifdef FEATURE_COMINTEROP
// Retrieve the IErrorInfo to use.
IErrorInfo* pErrorInfo = (IErrorInfo*)errorInfo;
#ifdef FEATURE_COMINTEROP
if (pErrorInfo == (IErrorInfo*)(-1))
{
pErrorInfo = NULL;
Expand All @@ -456,7 +456,11 @@ extern "C" void QCALLTYPE MarshalNative_GetExceptionForHR(INT32 errorCode, LPVOI

OBJECTREF exceptObj = NULL;
GCPROTECT_BEGIN(exceptObj);
#ifdef FEATURE_COMINTEROP
::GetExceptionForHR(errorCode, pErrorInfo, &exceptObj);
#else
::GetExceptionForHR(errorCode, &exceptObj);
#endif // FEATURE_COMINTEROP
retVal.Set(exceptObj);
GCPROTECT_END();

Expand Down