Fix funceval of a function returning 16 byte value type. (dotnet/coreclr#6997)
authorMike McLaughlin <mikem@microsoft.com>
Wed, 31 Aug 2016 19:27:12 +0000 (12:27 -0700)
committerGitHub <noreply@github.com>
Wed, 31 Aug 2016 19:27:12 +0000 (12:27 -0700)
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.

Commit migrated from https://github.com/dotnet/coreclr/commit/97448547841a58e3b74459d35a48c2b7978d97bc

src/coreclr/src/ToolBox/SOS/NETCore/SymbolReader.cs
src/coreclr/src/debug/ee/debugger.cpp
src/coreclr/src/debug/ee/debugger.h
src/coreclr/src/debug/ee/funceval.cpp
src/coreclr/src/vm/callhelpers.cpp
src/coreclr/src/vm/callhelpers.h
src/coreclr/src/vm/interpreter.cpp
src/coreclr/src/vm/mngstdinterfaces.cpp

index bfd4c2b..f4a3ce3 100644 (file)
@@ -389,23 +389,30 @@ namespace SOS
         /// <remarks>used by the gdb JIT support (not SOS). Does not support in-memory PEs or PDBs</remarks>
         internal static bool GetInfoForMethod(string assemblyPath, int methodToken, ref MethodDebugInfo debugInfo)
         {
-            List<DebugInfo> points = null;
-
-            if (!GetDebugInfoForMethod(assemblyPath, methodToken, out points))
+            try
             {
-                return false;
-            }
-            var structSize = Marshal.SizeOf<DebugInfo>();
+                List<DebugInfo> points = null;
+
+                if (!GetDebugInfoForMethod(assemblyPath, methodToken, out points))
+                {
+                    return false;
+                }
+                var structSize = Marshal.SizeOf<DebugInfo>();
 
-            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;
         }
 
         /// <summary>
index a9876ab..a06811c 100644 (file)
@@ -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
index 4558468..6368647 100644 (file)
@@ -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.
index d2e5576..eb8950d 100644 (file)
@@ -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, &param)
     {
-        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;
index 343b066..9152f71 100644 (file)
@@ -335,9 +335,9 @@ extern Volatile<LONG> 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)
index ae32005..446105b 100644 (file)
@@ -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
index 198ed2b..a540cff 100644 (file)
@@ -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<MethodDesc*>(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();
index 9e7fcdb..ade3fbd 100644 (file)
@@ -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);