Repository: dotnet/runtime
File: src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.cpp (C++)

Code before:
	CrawlFrame *frame,
	EventPipeStackContents *stack_contents);

static
void
walk_managed_stack_for_threads (
	ep_rt_thread_handle_t sampling_thread,
	EventPipeEvent *sampling_event);

static
StackWalkAction
stack_walk_callback (
	CrawlFrame *frame,
	EventPipeStackContents *stack_contents)
{
	STATIC_CONTRACT_NOTHROW;
	EP_ASSERT (frame != NULL);
	EP_ASSERT (stack_contents != NULL);

	// Get the IP.
	UINT_PTR control_pc = (UINT_PTR)frame->GetRegisterSet ()->ControlPC;
	
	{
		// At the PrestubMethodFrame, the ControlPC is not valid. Instead use the address of the Prestub.
		control_pc = 0;
	}
	if (control_pc == 0) {
#ifdef DEBUG
		if (ep_stack_contents_get_length (stack_contents) == 0) {
			// This happens for pinvoke stubs on the top of the stack.
		}
		else if (!frame->IsFrameless() && frame->GetFrame()->GetFrameIdentifier() == FrameIdentifier::PrestubMethodFrame) {
			// This happens for PrestubMethodFrame when we have interpreter frames on the stack
			// In non-interpreter scenarios the control_pc is not set to anything meaningful, but reporting it doesn't seem to cause problems.
		}
		else {
			EP_ASSERT (!"Unexpected null ControlPC in stack walk callback");
		}
#endif
		// With FUNCTIONSONLY flag, we may hit frames without a meaningful control_pc, but with a valid MethodDesc.
		// there is no point in reporting those frames as ep_stack_contents_append doesn't actually record the function
		// in a Frame in release builds, it only records the control_pc.
		return SWA_CONTINUE;
	}

	EP_ASSERT (control_pc != 0);

	// Add the IP to the captured stack.
	ep_stack_contents_append (stack_contents, control_pc, frame->GetFunction ());

	// Continue the stack walk.

Review comment (bug):
The empty block at lines 53-56 unconditionally sets control_pc to 0 for all frames. This appears to be incorrect logic. Based on the comment inside the block and the subsequent handling of control_pc == 0, this condition should only set control_pc to 0 for PrestubMethodFrame, not for all frames. The missing condition check will cause all frames to be skipped in the stack walk, breaking the functionality.

Code after:
	CrawlFrame *frame,
	EventPipeStackContents *stack_contents);

static
void
walk_managed_stack_for_threads (
	ep_rt_thread_handle_t sampling_thread,
	EventPipeEvent *sampling_event);

static
StackWalkAction
stack_walk_callback (
	CrawlFrame *frame,
	EventPipeStackContents *stack_contents)
{
	STATIC_CONTRACT_NOTHROW;
	EP_ASSERT (frame != NULL);
	EP_ASSERT (stack_contents != NULL);

	// Get the IP.
	UINT_PTR control_pc = (UINT_PTR)frame->GetRegisterSet ()->ControlPC;
	
	if (!frame->IsFrameless() && frame->GetFrame()->GetFrameIdentifier() == FrameIdentifier::PrestubMethodFrame) {
		// At the PrestubMethodFrame, the ControlPC is not valid. Since the eventpipe stackwalk is actually only based on the ip, skip this frame.
		return SWA_CONTINUE;
	}
	if (control_pc == 0) {
#ifdef DEBUG
		if (ep_stack_contents_get_length (stack_contents) == 0) {
			// This happens for pinvoke stubs on the top of the stack.
		}
		else {
			EP_ASSERT (!"Unexpected null ControlPC in stack walk callback");
		}
#endif
		// With FUNCTIONSONLY flag, we may hit frames without a meaningful control_pc, but with a valid MethodDesc.
		// There is no point in reporting those frames as ep_stack_contents_append doesn't actually record the function
		// in a Frame in release builds, it only records the control_pc.
		return SWA_CONTINUE;
	}

	EP_ASSERT (control_pc != 0);

	// Add the IP to the captured stack.
	ep_stack_contents_append (stack_contents, control_pc, frame->GetFunction ());

	// Continue the stack walk.
	return SWA_CONTINUE;
}

bool