Skip to content

Improve stack overflow stack trace pretty print #122243

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

Open
janvorli wants to merge 2 commits into
base: main
Choose a base branch
from
+65 −42
Open

Improve stack overflow stack trace pretty print #122243

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
509c6c6
Improve stack overflow stacktrace pretty print
janvorli Dec 5, 2025
285750d
Apply suggestions from code review
janvorli Dec 5, 2025
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
107 changes: 65 additions & 42 deletions src/coreclr/vm/eepolicy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,6 @@ class CallStackLogger
// MethodDescs of the stack frames, the TOS is at index 0
CDynArray<MethodDesc*> m_frames;

// Index of a stack frame where a possible repetition of frames starts
int m_commonStartIndex = -1;
// Length of the largest found repeated sequence of frames
int m_largestCommonStartLength = 0;
// Number of repetitions of the largest repeated sequence of frames
int m_largestCommonStartRepeat = 0;

StackWalkAction LogCallstackForLogCallbackWorker(CrawlFrame *pCF)
{
WRAPPER_NO_CONTRACT;
Expand All @@ -183,44 +176,14 @@ class CallStackLogger
}
}

MethodDesc *pMD = pCF->GetFunction();

if (m_commonStartIndex != -1)
{
// Some common frames were already found

if (m_frames[m_frames.Count() - m_commonStartIndex] != pMD)
{
// The frame being added is not part of the repeated sequence
if (m_frames.Count() / m_commonStartIndex >= 2)
{
// A sequence repeated at least twice was found. It is the largest one that was found so far
m_largestCommonStartLength = m_commonStartIndex;
m_largestCommonStartRepeat = m_frames.Count() / m_commonStartIndex;
}

m_commonStartIndex = -1;
}
}

if (m_commonStartIndex == -1)
{
if ((m_frames.Count() != 0) && (pMD == m_frames[0]))
{
// We have found a frame with the same MethodDesc as the frame at the top of the stack,
// possibly a new repeated sequence is starting.
m_commonStartIndex = m_frames.Count();
}
}

MethodDesc** itemPtr = m_frames.Append();
if (itemPtr == nullptr)
{
// Memory allocation failure
return SWA_ABORT;
}

*itemPtr = pMD;
*itemPtr = pCF->GetFunction();

return SWA_CONTINUE;
}
Expand Down Expand Up @@ -260,22 +223,82 @@ class CallStackLogger
{
WRAPPER_NO_CONTRACT;

if (m_largestCommonStartLength != 0)
// Length of the largest found repeated sequence of frames
int largestCommonLength = 0;
// Number of repetitions of the largest repeated sequence of frames
int largestCommonRepeat = 0;

// Start index of the repetition
int largestCommonStartOffset;
for (largestCommonStartOffset = 0; largestCommonStartOffset < m_frames.Count(); largestCommonStartOffset++)
{
// Index of a stack frame where a possible repetition of frames starts
int commonStartIndex = -1;
largestCommonLength = 0;
largestCommonRepeat = 0;

for (int i = largestCommonStartOffset; i < m_frames.Count(); i++)
Copy link
Member

@jkotas jkotas Dec 6, 2025

Choose a reason for hiding this comment

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

This looks like a quadratic algorithm. What's the worst-case delay that it can lead to in real-world apps?

Copy link
Member

@jkotas jkotas Dec 6, 2025

Choose a reason for hiding this comment

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

I believe there are O(N log N) algorithms to do this if this one is too slow.

Copy link
Member Author

@janvorli janvorli Dec 8, 2025

Choose a reason for hiding this comment

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

Yes, I am aware of the fact it is quadratic. I was also thinking of an option of setting a fixed limit to the outer loop so that in case we don't find a repeated sequence in that number of attempts (which would mean that there is no repeated sequence starting within that number of topmost frames), we would give up and show the whole stack.

As for the worst case delay, I've tried to run it with the repro you have shared in the #118218 and disabling the check for found repetition. That effectively simulates the worst case. Release build of your repro with tight loop has stack with 27084 frames at the time of the stack overflow and the time it took to run the full quadratic number of iterations was 0.16s on my Windows x64 machine with Release build of the runtime and release build of the test.

So given this result, I think it is 1ok to leave the quadratic algorithm in. If we wanted to be super safe, we could possibly add some high fixed limit of say ~1000 repetitions of the outer loop.

Copy link
Member

@jkotas jkotas Dec 8, 2025

Choose a reason for hiding this comment

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

Worth adding a comment about this?

{
MethodDesc* pMD = m_frames[i];
if (commonStartIndex != -1)
{
// Some common frames were already found

int commonLength = commonStartIndex - largestCommonStartOffset;
if (m_frames[i - commonLength] != pMD)
{
// The frame being added is not part of the repeated sequence
int commonRepeat = (i - largestCommonStartOffset) / commonLength;
if (commonRepeat >= 2)
{
// A sequence repeated at least twice was found. It is the largest one that was found so far
largestCommonLength = commonLength;
largestCommonRepeat = commonRepeat;
}

commonStartIndex = -1;
}
}

if (commonStartIndex == -1)
{
if ((i != largestCommonStartOffset) && (pMD == m_frames[largestCommonStartOffset]))
{
// We have found a frame with the same MethodDesc as the frame at the start of the repetition search (index largestCommonStartOffset),
// possibly a new repeated sequence is starting.
commonStartIndex = i;
}
}
}

if (largestCommonRepeat != 0)
{
// A repeated sequence of frames was identified
break;
}
}

for (int i = 0; i < largestCommonStartOffset; i++)
{
PrintFrame(i, pWordAt);
}

if (largestCommonLength != 0)
{
SmallStackSString repeatStr;
repeatStr.AppendPrintf("Repeated %d times:\n", m_largestCommonStartRepeat);
repeatStr.AppendPrintf("Repeated %d times:\n", largestCommonRepeat);

PrintToStdErrW(repeatStr.GetUnicode());

PrintToStdErrA("--------------------------------\n");
for (int i = 0; i < m_largestCommonStartLength; i++)
for (int i = largestCommonStartOffset; i < largestCommonStartOffset + largestCommonLength; i++)
{
PrintFrame(i, pWordAt);
}
PrintToStdErrA("--------------------------------\n");
}

for (int i = m_largestCommonStartLength * m_largestCommonStartRepeat; i < m_frames.Count(); i++)
for (int i = largestCommonLength * largestCommonRepeat + largestCommonStartOffset; i < m_frames.Count(); i++)
{
PrintFrame(i, pWordAt);
}
Expand Down
Loading