Fix step with stackalloc (#27246) (#27351)

* Make ControllerStackInfo::m_returnFrame private

* Make ControllerStackInfo always capture a return frame

In case the active frame has no managed caller, capture
the unmanaged frame

* Fix step over stackalloc

Generalize handling of stack allocations and stepping

* Fix GetFunctionFromToken() argument checking

Check token type is a method before creating a CordbFunction.

Add extra assert to check for invalid tokens

Author: sdmaclea

src/debug/di/module.cpp

@@ -1550,7 +1550,8 @@ HRESULT CordbModule::GetFunctionFromToken(mdMethodDef token,
         RSLockHolder lockHolder(GetProcess()->GetProcessLock());
 
         // Check token is valid.
-        if ((token == mdMethodDefNil) || 
+        if ((token == mdMethodDefNil) ||
+            (TypeFromToken(token) != mdtMethodDef) ||
             (!GetMetaDataImporter()->IsValidToken(token)))
         {
             ThrowHR(E_INVALIDARG);

src/debug/di/rsfunction.cpp

@@ -49,6 +49,7 @@ CordbFunction::CordbFunction(CordbModule * m,
     m_methodSigParserCached = SigParser(NULL, 0);
 
     _ASSERTE(enCVersion >= CorDB_DEFAULT_ENC_FUNCTION_VERSION);
+    _ASSERTE(TypeFromToken(m_MDToken) == mdtMethodDef);
 }
 
 

src/debug/di/rspriv.h

@@ -5537,7 +5537,7 @@ class CordbFunction : public CordbBase,
     RSSmartPtr<CordbNativeCode> m_nativeCode;
 
     // Metadata Token for the IL function. Scoped to m_module.
-    mdMethodDef              m_MDToken;
+    const mdMethodDef        m_MDToken;
 
     // EnC version number of this instance
     SIZE_T                   m_dwEnCVersionNumber; 

src/debug/ee/controller.cpp

@@ -140,15 +140,15 @@ class StackTraceTicket
  *  FrameInfo m_activeFrame:   A FrameInfo
  *      describing the target frame.  This should always be valid after a
  *      call to GetStackInfo.
+ *
+ *  private:
+ *  bool m_activeFound:  Set to true if we found the target frame.
+ *  bool m_returnFound:  Set to true if we found the target's return frame.
  * 
  *  FrameInfo m_returnFrame:   A FrameInfo
  *      describing the frame above the target frame, if  target's
  *      return frame were found (call HasReturnFrame() to see if this is
  *      valid). Otherwise, this will be the same as m_activeFrame, above
- *
- * private:
- * bool m_activeFound:  Set to true if we found the target frame.
- * bool m_returnFound:  Set to true if we found the target's return frame.
  */
 class ControllerStackInfo
 {
@@ -165,7 +165,6 @@ class ControllerStackInfo
     bool                    m_targetFrameFound;
 
     FrameInfo               m_activeFrame;
-    FrameInfo               m_returnFrame;
 
     CorDebugChainReason     m_specialChainReason;
 
@@ -199,8 +198,9 @@ class ControllerStackInfo
     //bool ControllerStackInfo::HasReturnFrame()  Returns
     //      true if m_returnFrame is valid.  Returns false
     //      if m_returnFrame is set to m_activeFrame
-    bool HasReturnFrame() {LIMITED_METHOD_CONTRACT;  return m_returnFound; }
+    bool HasReturnFrame(bool allowUnmanaged = false) {LIMITED_METHOD_CONTRACT;  return m_returnFound && (allowUnmanaged || m_returnFrame.managed); }
 
+    FrameInfo& GetReturnFrame(bool allowUnmanaged = false) {LIMITED_METHOD_CONTRACT; return HasReturnFrame(allowUnmanaged) ? m_returnFrame : m_activeFrame; }
     // This function "undoes" an unwind, i.e. it takes the active frame (the current frame)
     // and sets it to be the return frame (the caller frame).  Currently it is only used by
     // the stepper to step out of an LCG method.  See DebuggerStepper::DetectHandleLCGMethods()
@@ -213,6 +213,7 @@ class ControllerStackInfo
 
     bool                    m_activeFound;
     bool                    m_returnFound;
+    FrameInfo               m_returnFrame;
 
     // A ridiculous flag that is targetting a very narrow fix at issue 650903
     // (4.5.1/Blue).  This is set for the duration of a stackwalk designed to

src/debug/ee/controller.h

@@ -585,7 +585,7 @@ void FrameInfo::AssertValid()
 // Get the DJI associated w/ this frame. This is a convenience function.
 // This is recommended over using MethodDescs because DJI's are version-aware.
 //-----------------------------------------------------------------------------
-DebuggerJitInfo * FrameInfo::GetJitInfoFromFrame()
+DebuggerJitInfo * FrameInfo::GetJitInfoFromFrame() const
 {
     CONTRACTL
     {

src/debug/ee/frameinfo.cpp

@@ -130,22 +130,22 @@ struct FrameInfo
 
     // Does this FrameInfo represent a method frame? (aka a frameless frame)
     // This may be combined w/ both StubFrames and ChainMarkers.
-    bool HasMethodFrame() { return md != NULL && !internal; }
+    bool HasMethodFrame() const { return md != NULL && !internal; }
 
     // Is this frame for a stub?
     // This is mutually exclusive w/ Chain Markers.
     // StubFrames may also have a method frame as a "hint". Ex, a stub frame for a 
     // M2U transition may have the Method for the Managed Wrapper for the unmanaged call.
     // Stub frames map to internal frames on the RS.  They use the same enum 
     // (CorDebugInternalFrameType) to represent the type of the frame.
-    bool HasStubFrame() { return eStubFrameType != STUBFRAME_NONE; }
+    bool HasStubFrame() const { return eStubFrameType != STUBFRAME_NONE; }
 
     // Does this FrameInfo mark the start of a new chain? (A Frame info may both
     // start a chain and represent a method)
-    bool HasChainMarker() { return chainReason != CHAIN_NONE; }
+    bool HasChainMarker() const { return chainReason != CHAIN_NONE; }
 
     // Helper functions for retrieving the DJI and the DMI
-    DebuggerJitInfo * GetJitInfoFromFrame();
+    DebuggerJitInfo * GetJitInfoFromFrame() const;
     DebuggerMethodInfo * GetMethodInfoFromFrameOrThrow();
 
     // Debug helper which nops in retail; and asserts invariants in debug.

src/debug/ee/frameinfo.h

@@ -48,7 +48,7 @@ struct REGDISPLAY_BASE {
     TADDR ControlPC;
 };
 
-inline PCODE GetControlPC(REGDISPLAY_BASE *pRD) {
+inline PCODE GetControlPC(const REGDISPLAY_BASE *pRD) {
     LIMITED_METHOD_DAC_CONTRACT;
     return (PCODE)(pRD->ControlPC);
 }