Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix step with stackalloc #27246

Merged
merged 5 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/debug/di/module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you hit this error even without the runtime portion of the change, or it only shows up together with the runtime change? If it is the former I think this is fine. If it is the latter I'd be worried that the runtime change has broken something we don't fully understand and a bad token here happened to be one symptom of it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This occurred in the baseline too. It could be a separate
PR.

(!GetMetaDataImporter()->IsValidToken(token)))
{
ThrowHR(E_INVALIDARG);
Expand Down
1 change: 1 addition & 0 deletions src/debug/di/rsfunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down
2 changes: 1 addition & 1 deletion src/debug/di/rspriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
141 changes: 59 additions & 82 deletions src/debug/ee/controller.cpp

Large diffs are not rendered by default.

13 changes: 7 additions & 6 deletions src/debug/ee/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -165,7 +165,6 @@ class ControllerStackInfo
bool m_targetFrameFound;

FrameInfo m_activeFrame;
FrameInfo m_returnFrame;

CorDebugChainReason m_specialChainReason;

Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/debug/ee/frameinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
8 changes: 4 additions & 4 deletions src/debug/ee/frameinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/inc/regdisp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down