From 97448547841a58e3b74459d35a48c2b7978d97bc Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Wed, 31 Aug 2016 12:27:12 -0700 Subject: Fix funceval of a function returning 16 byte value type. (#6997) On Linux and OS X, structs less or equal to 16 bytes (instead of 8 bytes on Windows) are enregistered. Only 8 bytes is being passed back to debugger during a funceval of a property or method that returns a 16 byte value type. To fix this, the 16 byte return value (which is only used for enregistered structures of that size on xplat) needed to be plumbed from CallTargetWorker through the MethodDescCallSite macros to the func eval code. The func eval code needed to also deal with 16 byte results. NUMBER_RETURNVALUE_SLOTS is the number of ARG_SLOTs that will contain the maximum enregistered return value. CordbEval:m_result is now ARG_SLOT[NUMBER_RETURNVALUE_SLOTS]. CallTargetWorker is now passed a pointer and the count of bytes to/of the return value buffer. Minor fix to SOS SymbolReader function. --- src/ToolBox/SOS/NETCore/SymbolReader.cs | 31 ++++++++++++++++----------- src/debug/ee/debugger.cpp | 15 +++++++------ src/debug/ee/debugger.h | 2 +- src/debug/ee/funceval.cpp | 38 ++++++++++++++++----------------- src/vm/callhelpers.cpp | 36 +++++++++++++++++++++---------- src/vm/callhelpers.h | 25 ++++++++++++++++------ src/vm/interpreter.cpp | 5 +++-- src/vm/mngstdinterfaces.cpp | 2 +- 8 files changed, 94 insertions(+), 60 deletions(-) (limited to 'src') diff --git a/src/ToolBox/SOS/NETCore/SymbolReader.cs b/src/ToolBox/SOS/NETCore/SymbolReader.cs index bfd4c2bb7d..f4a3ce30d8 100644 --- a/src/ToolBox/SOS/NETCore/SymbolReader.cs +++ b/src/ToolBox/SOS/NETCore/SymbolReader.cs @@ -389,23 +389,30 @@ namespace SOS /// used by the gdb JIT support (not SOS). Does not support in-memory PEs or PDBs internal static bool GetInfoForMethod(string assemblyPath, int methodToken, ref MethodDebugInfo debugInfo) { - List points = null; - - if (!GetDebugInfoForMethod(assemblyPath, methodToken, out points)) + try { - return false; - } - var structSize = Marshal.SizeOf(); + List points = null; + + if (!GetDebugInfoForMethod(assemblyPath, methodToken, out points)) + { + return false; + } + var structSize = Marshal.SizeOf(); - debugInfo.size = points.Count; - var ptr = debugInfo.points; + debugInfo.size = points.Count; + var ptr = debugInfo.points; - foreach (var info in points) + foreach (var info in points) + { + Marshal.StructureToPtr(info, ptr, false); + ptr = (IntPtr)(ptr.ToInt64() + structSize); + } + return true; + } + catch { - Marshal.StructureToPtr(info, ptr, false); - ptr = (IntPtr)(ptr.ToInt64() + structSize); } - return true; + return false; } /// diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp index a9876abc35..a06811c817 100644 --- a/src/debug/ee/debugger.cpp +++ b/src/debug/ee/debugger.cpp @@ -1463,7 +1463,7 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval m_genericArgsNodeCount = pEvalInfo->genericArgsNodeCount; m_successful = false; m_argData = NULL; - m_result = 0; + memset(m_result, 0, sizeof(m_result)); m_md = NULL; m_resultType = TypeHandle(); m_aborting = FE_ABORT_NONE; @@ -1519,7 +1519,7 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, Thread * pThread, Thread::ThreadA m_successful = false; m_argData = NULL; m_targetCodeAddr = NULL; - m_result = 0; + memset(m_result, 0, sizeof(m_result)); m_md = NULL; m_resultType = TypeHandle(); m_aborting = FE_ABORT_NONE; @@ -10353,7 +10353,7 @@ void Debugger::FuncEvalComplete(Thread* pThread, DebuggerEval *pDE) if (CORDBUnrecoverableError(this)) return; - LOG((LF_CORDB, LL_INFO10000, "D::FEC: func eval complete pDE:%08x evalType:%d %s %s\n", + LOG((LF_CORDB, LL_INFO1000, "D::FEC: func eval complete pDE:%p evalType:%d %s %s\n", pDE, pDE->m_evalType, pDE->m_successful ? "Success" : "Fail", pDE->m_aborted ? "Abort" : "Completed")); @@ -10386,11 +10386,11 @@ void Debugger::FuncEvalComplete(Thread* pThread, DebuggerEval *pDE) ipce->FuncEvalComplete.funcEvalKey = pDE->m_funcEvalKey; ipce->FuncEvalComplete.successful = pDE->m_successful; ipce->FuncEvalComplete.aborted = pDE->m_aborted; - ipce->FuncEvalComplete.resultAddr = &(pDE->m_result); + ipce->FuncEvalComplete.resultAddr = pDE->m_result; ipce->FuncEvalComplete.vmAppDomain.SetRawPtr(pResultDomain); ipce->FuncEvalComplete.vmObjectHandle = pDE->m_vmObjectHandle; - LOG((LF_CORDB, LL_INFO10000, "D::FEC: TypeHandle is :%08x\n", pDE->m_resultType.AsPtr())); + LOG((LF_CORDB, LL_INFO1000, "D::FEC: TypeHandle is %p\n", pDE->m_resultType.AsPtr())); Debugger::TypeHandleToExpandedTypeInfo(pDE->m_retValueBoxing, // whether return values get boxed or not depends on the particular FuncEval we're doing... pResultDomain, @@ -10399,12 +10399,13 @@ void Debugger::FuncEvalComplete(Thread* pThread, DebuggerEval *pDE) _ASSERTE(ipce->FuncEvalComplete.resultType.elementType != ELEMENT_TYPE_VALUETYPE); - LOG((LF_CORDB, LL_INFO10000, "D::FEC: returned from call\n")); - // We must adjust the result address to point to the right place ipce->FuncEvalComplete.resultAddr = ArgSlotEndianessFixup((ARG_SLOT*)ipce->FuncEvalComplete.resultAddr, GetSizeForCorElementType(ipce->FuncEvalComplete.resultType.elementType)); + LOG((LF_CORDB, LL_INFO1000, "D::FEC: returned el %04x resultAddr %p\n", + ipce->FuncEvalComplete.resultType.elementType, ipce->FuncEvalComplete.resultAddr)); + m_pRCThread->SendIPCEvent(); #endif diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h index 4558468182..6368647946 100644 --- a/src/debug/ee/debugger.h +++ b/src/debug/ee/debugger.h @@ -3431,7 +3431,7 @@ public: BYTE *m_argData; MethodDesc *m_md; PCODE m_targetCodeAddr; - INT64 m_result; + ARG_SLOT m_result[NUMBER_RETURNVALUE_SLOTS]; TypeHandle m_resultType; SIZE_T m_arrayRank; FUNC_EVAL_ABORT_TYPE m_aborting; // Has an abort been requested, and what type. diff --git a/src/debug/ee/funceval.cpp b/src/debug/ee/funceval.cpp index d2e5576855..eb8950deab 100644 --- a/src/debug/ee/funceval.cpp +++ b/src/debug/ee/funceval.cpp @@ -2775,7 +2775,7 @@ void UnpackFuncEvalResult(DebuggerEval *pDE, { // We purposely do not morph nullables to be boxed Ts here because debugger EE's otherwise // have no way of creating true nullables that they need for their own purposes. - pDE->m_result = ObjToArgSlot(newObj); + pDE->m_result[0] = ObjToArgSlot(newObj); pDE->m_retValueBoxing = Debugger::AllBoxed; } else if (!RetValueType.IsNull()) @@ -2803,12 +2803,12 @@ void UnpackFuncEvalResult(DebuggerEval *pDE, { // box the primitive returned, retObject is a true nullable for nullabes, It will be Normalized later CopyValueClass(retObject->GetData(), - &(pDE->m_result), + pDE->m_result, RetValueType.GetMethodTable(), retObject->GetAppDomain()); } - pDE->m_result = ObjToArgSlot(retObject); + pDE->m_result[0] = ObjToArgSlot(retObject); pDE->m_retValueBoxing = Debugger::AllBoxed; } else @@ -2833,8 +2833,8 @@ void UnpackFuncEvalResult(DebuggerEval *pDE, IsElementTypeSpecial(retClassET)) { LOG((LF_CORDB, LL_EVERYTHING, "Creating strong handle for boxed DoNormalFuncEval result.\n")); - OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(ArgSlotToObj(pDE->m_result)); - pDE->m_result = (INT64)(LONG_PTR)oh; + OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(ArgSlotToObj(pDE->m_result[0])); + pDE->m_result[0] = (INT64)(LONG_PTR)oh; pDE->m_vmObjectHandle = VMPTR_OBJECTHANDLE::MakePtr(oh); } } @@ -2930,13 +2930,13 @@ void UnpackFuncEvalArguments(DebuggerEval *pDE, * None. * */ -void FuncEvalWrapper(MethodDescCallSite* pMDCS, DebuggerEval *pDE, ARG_SLOT *pArguments, BYTE *pCatcherStackAddr) +void FuncEvalWrapper(MethodDescCallSite* pMDCS, DebuggerEval *pDE, const ARG_SLOT *pArguments, BYTE *pCatcherStackAddr) { struct Param : NotifyOfCHFFilterWrapperParam { MethodDescCallSite* pMDCS; DebuggerEval *pDE; - ARG_SLOT *pArguments; + const ARG_SLOT *pArguments; }; Param param; @@ -2947,7 +2947,7 @@ void FuncEvalWrapper(MethodDescCallSite* pMDCS, DebuggerEval *pDE, ARG_SLOT *pAr PAL_TRY(Param *, pParam, ¶m) { - pParam->pDE->m_result = pParam->pMDCS->CallWithValueTypes_RetArgSlot(pParam->pArguments); + pParam->pMDCS->CallWithValueTypes_RetArgSlot(pParam->pArguments, pParam->pDE->m_result, sizeof(pParam->pDE->m_result)); } PAL_EXCEPT_FILTER(NotifyOfCHFFilterWrapper) { @@ -3003,13 +3003,12 @@ static void RecordFuncEvalException(DebuggerEval *pDE, // // This is the abort we sent down. // - pDE->m_result = NULL; + memset(pDE->m_result, 0, sizeof(pDE->m_result)); pDE->m_resultType = TypeHandle(); pDE->m_aborted = true; pDE->m_retValueBoxing = Debugger::NoValueTypeBoxing; LOG((LF_CORDB, LL_EVERYTHING, "D::FEHW - funceval abort exception.\n")); - } else { @@ -3022,11 +3021,11 @@ static void RecordFuncEvalException(DebuggerEval *pDE, // // The result is the exception object. // - pDE->m_result = ObjToArgSlot(ppException); + pDE->m_result[0] = ObjToArgSlot(ppException); pDE->m_resultType = ppException->GetTypeHandle(); - OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(ArgSlotToObj(pDE->m_result)); - pDE->m_result = (INT64)PTR_TO_CORDB_ADDRESS(oh); + OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(ArgSlotToObj(pDE->m_result[0])); + pDE->m_result[0] = (ARG_SLOT)PTR_TO_CORDB_ADDRESS(oh); pDE->m_vmObjectHandle = VMPTR_OBJECTHANDLE::MakePtr(oh); pDE->m_retValueBoxing = Debugger::NoValueTypeBoxing; @@ -3035,15 +3034,14 @@ static void RecordFuncEvalException(DebuggerEval *pDE, } else { - // // The result is the exception object. // - pDE->m_result = ObjToArgSlot(ppException); + pDE->m_result[0] = ObjToArgSlot(ppException); pDE->m_resultType = ppException->GetTypeHandle(); - OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(ArgSlotToObj(pDE->m_result)); - pDE->m_result = (INT64)(LONG_PTR)oh; + OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(ArgSlotToObj(pDE->m_result[0])); + pDE->m_result[0] = (ARG_SLOT)(LONG_PTR)oh; pDE->m_vmObjectHandle = VMPTR_OBJECTHANDLE::MakePtr(oh); pDE->m_retValueBoxing = Debugger::NoValueTypeBoxing; @@ -3597,7 +3595,7 @@ void FuncEvalHijackRealWorker(DebuggerEval *pDE, Thread* pThread, FuncEvalFrame* // Make a strong handle for the result. OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(newObj); - pDE->m_result = (INT64)(LONG_PTR)oh; + pDE->m_result[0] = (ARG_SLOT)(LONG_PTR)oh; pDE->m_vmObjectHandle = VMPTR_OBJECTHANDLE::MakePtr(oh); break; @@ -3627,7 +3625,7 @@ void FuncEvalHijackRealWorker(DebuggerEval *pDE, Thread* pThread, FuncEvalFrame* // Place the result in a strong handle to protect it from a collection. OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(newObj); - pDE->m_result = (INT64)(LONG_PTR)oh; + pDE->m_result[0] = (ARG_SLOT)(LONG_PTR)oh; pDE->m_vmObjectHandle = VMPTR_OBJECTHANDLE::MakePtr(oh); break; @@ -3673,7 +3671,7 @@ void FuncEvalHijackRealWorker(DebuggerEval *pDE, Thread* pThread, FuncEvalFrame* // Place the result in a strong handle to protect it from a collection. OBJECTHANDLE oh = pDE->m_thread->GetDomain()->CreateStrongHandle(newObj); - pDE->m_result = (INT64)(LONG_PTR)oh; + pDE->m_result[0] = (ARG_SLOT)(LONG_PTR)oh; pDE->m_vmObjectHandle = VMPTR_OBJECTHANDLE::MakePtr(oh); break; diff --git a/src/vm/callhelpers.cpp b/src/vm/callhelpers.cpp index 343b066e38..9152f71d79 100644 --- a/src/vm/callhelpers.cpp +++ b/src/vm/callhelpers.cpp @@ -335,9 +335,9 @@ extern Volatile g_fInExecuteMainMethod; //******************************************************************************* #ifdef FEATURE_INTERPRETER -ARG_SLOT MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, bool transitionToPreemptive) +void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *pReturnValue, int cbReturnValue, bool transitionToPreemptive) #else -ARG_SLOT MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments) +void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *pReturnValue, int cbReturnValue) #endif { // @@ -443,6 +443,18 @@ ARG_SLOT MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments) #ifdef _DEBUG { +#ifdef FEATURE_UNIX_AMD64_STRUCT_PASSING_ITF + // Validate that the return value is not too big for the buffer passed + if (m_pMD->GetMethodTable()->IsRegPassedStruct()) + { + TypeHandle thReturnValueType; + if (m_methodSig.GetReturnTypeNormalized(&thReturnValueType) == ELEMENT_TYPE_VALUETYPE) + { + _ASSERTE(cbReturnValue >= thReturnValueType.GetSize()); + } + } +#endif // FEATURE_UNIX_AMD64_STRUCT_PASSING_ITF + // The metasig should be reset _ASSERTE(m_methodSig.GetArgNum() == 0); @@ -637,20 +649,22 @@ ARG_SLOT MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments) memcpyNoGCRefs(pvRetBuff, &callDescrData.returnValue, sizeof(callDescrData.returnValue)); } - ARG_SLOT retval = *(ARG_SLOT *)(&callDescrData.returnValue); - -#if !defined(_WIN64) && BIGENDIAN + if (pReturnValue != NULL) { - GCX_FORBID(); + _ASSERTE(cbReturnValue <= sizeof(callDescrData.returnValue)); + memcpyNoGCRefs(pReturnValue, &callDescrData.returnValue, cbReturnValue); - if (!m_methodSig.Is64BitReturn()) +#if !defined(_WIN64) && BIGENDIAN { - retval >>= 32; + GCX_FORBID(); + + if (!m_methodSig.Is64BitReturn()) + { + pReturnValue[0] >>= 32; + } } - } #endif // !defined(_WIN64) && BIGENDIAN - - return retval; + } } void CallDefaultConstructor(OBJECTREF ref) diff --git a/src/vm/callhelpers.h b/src/vm/callhelpers.h index ae32005352..446105bfba 100644 --- a/src/vm/callhelpers.h +++ b/src/vm/callhelpers.h @@ -41,6 +41,8 @@ struct CallDescrData #endif }; +#define NUMBER_RETURNVALUE_SLOTS (ENREGISTERED_RETURNTYPE_MAXSIZE / sizeof(ARG_SLOT)) + #if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE) extern "C" void STDCALL CallDescrWorkerInternal(CallDescrData * pCallDescrData); @@ -131,9 +133,9 @@ private: #ifdef FEATURE_INTERPRETER public: - ARG_SLOT CallTargetWorker(const ARG_SLOT *pArguments, bool transitionToPreemptive = false); + void CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *pReturnValue, int cbReturnValue, bool transitionToPreemptive = false); #else - ARG_SLOT CallTargetWorker(const ARG_SLOT *pArguments); + void CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *pReturnValue, int cbReturnValue); #endif public: @@ -309,10 +311,21 @@ public: eltype == m_methodSig.GetReturnType()); \ } \ ARG_SLOT retval; \ - retval = CallTargetWorker(pArguments); \ + CallTargetWorker(pArguments, &retval, sizeof(retval)); \ return *(rettype *)ArgSlotEndianessFixup(&retval, sizeof(rettype)); \ } +#define MDCALLDEF_ARGSLOT(wrappedmethod, ext) \ + FORCEINLINE void wrappedmethod##ext (const ARG_SLOT* pArguments, ARG_SLOT *pReturnValue, int cbReturnValue) \ + { \ + WRAPPER_NO_CONTRACT; \ + { \ + GCX_FORBID(); /* arg array is not protected */ \ + } \ + CallTargetWorker(pArguments, pReturnValue, cbReturnValue); \ + /* Bigendian layout not support */ \ + } + #define MDCALLDEF_REFTYPE(wrappedmethod, permitvaluetypes, ext, ptrtype, reftype) \ FORCEINLINE reftype wrappedmethod##ext (const ARG_SLOT* pArguments) \ { \ @@ -322,7 +335,7 @@ public: CONSISTENCY_CHECK(MetaSig::RETOBJ == m_pMD->ReturnsObject(true)); \ } \ ARG_SLOT retval; \ - retval = CallTargetWorker(pArguments); \ + CallTargetWorker(pArguments, &retval, sizeof(retval)); \ return ObjectTo##reftype(*(ptrtype *) \ ArgSlotEndianessFixup(&retval, sizeof(ptrtype))); \ } @@ -336,7 +349,7 @@ public: FORCEINLINE void wrappedmethod (const ARG_SLOT* pArguments) \ { \ WRAPPER_NO_CONTRACT; \ - CallTargetWorker(pArguments); \ + CallTargetWorker(pArguments, NULL, 0); \ } #define MDCALLDEFF_STD_RETTYPES(wrappedmethod,permitvaluetypes) \ @@ -426,7 +439,7 @@ public: // XXX CallWithValueTypes_RetXXX(const ARG_SLOT* pArguments); MDCALLDEF_VOID( CallWithValueTypes, TRUE) - MDCALLDEF( CallWithValueTypes, TRUE, _RetArgSlot, ARG_SLOT, OTHER_ELEMENT_TYPE) + MDCALLDEF_ARGSLOT( CallWithValueTypes, _RetArgSlot) MDCALLDEF_REFTYPE( CallWithValueTypes, TRUE, _RetOBJECTREF, Object*, OBJECTREF) MDCALLDEF( CallWithValueTypes, TRUE, _RetOleColor, OLE_COLOR, OTHER_ELEMENT_TYPE) #undef OTHER_ELEMENT_TYPE diff --git a/src/vm/interpreter.cpp b/src/vm/interpreter.cpp index 198ed2b26a..a540cff0b0 100644 --- a/src/vm/interpreter.cpp +++ b/src/vm/interpreter.cpp @@ -9578,6 +9578,7 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T // This is the argument slot that will be used to hold the return value. ARG_SLOT retVal = 0; + _ASSERTE (NUMBER_RETURNVALUE_SLOTS == 1); // If the return type is a structure, then these will be initialized. CORINFO_CLASS_HANDLE retTypeClsHnd = NULL; @@ -9853,7 +9854,7 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T #if INTERP_ILCYCLE_PROFILE bool b = CycleTimer::GetThreadCyclesS(&startCycles); assert(b); #endif // INTERP_ILCYCLE_PROFILE - retVal = mdcs.CallTargetWorker(args); + mdcs.CallTargetWorker(args, &retVal, sizeof(retVal)); if (pCscd != NULL) { @@ -10323,7 +10324,7 @@ void Interpreter::CallI() // to be a managed calling convention.) MethodDesc* pStubContextMD = reinterpret_cast(m_stubContext); bool transitionToPreemptive = (pStubContextMD != NULL && !pStubContextMD->IsIL()); - retVal = mdcs.CallTargetWorker(args, transitionToPreemptive); + mdcs.CallTargetWorker(args, &retVal, sizeof(retVal), transitionToPreemptive); } // retVal is now vulnerable. GCX_FORBID(); diff --git a/src/vm/mngstdinterfaces.cpp b/src/vm/mngstdinterfaces.cpp index 9e7fcdb931..ade3fbd483 100644 --- a/src/vm/mngstdinterfaces.cpp +++ b/src/vm/mngstdinterfaces.cpp @@ -228,7 +228,7 @@ LPVOID MngStdItfBase::ForwardCallToManagedView( MethodDescCallSite mngItf(pMngItfMD, CRemotingServices::GetStubForInterfaceMethod(pMngItfMD)); // Call the stub with the args we were passed originally. - Result = (Object*)mngItf.CallWithValueTypes_RetArgSlot(pArgs); + Result = (Object*)mngItf.Call_RetArgSlot(pArgs); if (mngItf.GetMetaSig()->IsObjectRefReturnType()) { Lr.Result = ObjectToOBJECTREF(Result); -- cgit v1.2.3