More improvements to return value handling.

- Thread now has a data member indicating if a subroutine was
  executed during the last set of steps.
- ThreadHandler now sets the aforementioned state appropriate during
  Step Over/Step Out.
- Architecture::CreateStackTrace() now takes a parameter indicating
  whether return value retrieval is desired (based on aforementioned
  thread value). Adjust callers accordingly.
- DwarfImageDebugInfo: If return value retrieval is requested, loop
  backwards from the current IP to find the call instruction.

src/apps/debugger/arch/Architecture.cpp

@@ -94,8 +94,8 @@ Architecture::InitRegisterRules(CfaContext& context) const
 status_t
 Architecture::CreateStackTrace(Team* team,
 	ImageDebugInfoProvider* imageInfoProvider, CpuState* cpuState,
-	StackTrace*& _stackTrace, int32 maxStackDepth, bool useExistingTrace,
-	bool getFullFrameInfo)
+	StackTrace*& _stackTrace, bool getReturnValue, int32 maxStackDepth,
+	bool useExistingTrace, bool getFullFrameInfo)
 {
 	BReference<CpuState> cpuStateReference(cpuState);
 
@@ -163,7 +163,8 @@ Architecture::CreateStackTrace(Team* team,
 		if (function != NULL) {
 			status_t error = functionDebugInfo->GetSpecificImageDebugInfo()
 				->CreateFrame(image, function, cpuState, getFullFrameInfo,
-					frame, previousCpuState);
+					nextFrame == NULL ? getReturnValue : false, frame,
+					previousCpuState);
 			if (error != B_OK && error != B_UNSUPPORTED)
 				break;
 		}

src/apps/debugger/arch/Architecture.h

@@ -109,6 +109,7 @@ public:
 									ImageDebugInfoProvider* imageInfoProvider,
 									CpuState* cpuState,
 									StackTrace*& _stackTrace,
+									bool getReturnValue,
 									int32 maxStackDepth = -1,
 									bool useExistingTrace = false,
 									bool getFullFrameInfo = true);

src/apps/debugger/controllers/ThreadHandler.cpp

@@ -253,8 +253,8 @@ ThreadHandler::HandleThreadAction(uint32 action)
 
 	if (stackTrace == NULL && cpuState != NULL) {
 		if (fDebuggerInterface->GetArchitecture()->CreateStackTrace(
-				fThread->GetTeam(), this, cpuState, stackTrace, 1, false,
-				false) == B_OK) {
+				fThread->GetTeam(), this, cpuState, stackTrace, false, 1,
+				false, false) == B_OK) {
 			stackTraceReference.SetTo(stackTrace, true);
 		}
 	}
@@ -484,6 +484,7 @@ ThreadHandler::_DoStepOver(CpuState* cpuState)
 	TRACE_CONTROL("  subroutine call -- installing breakpoint at address "
 		"%#" B_PRIx64 "\n", info.Address() + info.Size());
 
+	fThread->SetExecutedSubroutine();
 	if (_InstallTemporaryBreakpoint(info.Address() + info.Size()) != B_OK)
 		return false;
 
@@ -565,9 +566,8 @@ ThreadHandler::_HandleBreakpointHitStep(CpuState* cpuState)
 
 			if (stackTrace == NULL && cpuState != NULL) {
 				if (fDebuggerInterface->GetArchitecture()->CreateStackTrace(
-						fThread->GetTeam(), this, cpuState, stackTrace, 1,
-						false, false)
-						== B_OK) {
+						fThread->GetTeam(), this, cpuState, stackTrace, false,
+						1, false, false) == B_OK) {
 					stackTraceReference.SetTo(stackTrace, true);
 				}
 			}
@@ -576,7 +576,7 @@ ThreadHandler::_HandleBreakpointHitStep(CpuState* cpuState)
 				// If we're not in the same frame we started in,
 				// keep executing.
 				if (frame != NULL && fPreviousFrameAddress
-						!= stackTrace->FrameAt(0)->FrameAddress()) {
+						!= frame->FrameAddress()) {
 					status_t error = _InstallTemporaryBreakpoint(
 						cpuState->InstructionPointer());
 					if (error != B_OK)
@@ -608,6 +608,7 @@ ThreadHandler::_HandleBreakpointHitStep(CpuState* cpuState)
 			// That's the return address, so we're done in theory,
 			// unless we're a recursive function. Check if we've actually
 			// exited the previous stack frame or not.
+			fThread->SetExecutedSubroutine();
 			target_addr_t framePointer = cpuState->StackFramePointer();
 			bool hasExitedFrame = fDebuggerInterface->GetArchitecture()
 				->StackGrowthDirection() == STACK_GROWTH_DIRECTION_POSITIVE
@@ -652,9 +653,8 @@ ThreadHandler::_HandleSingleStepStep(CpuState* cpuState)
 
 			if (stackTrace == NULL && cpuState != NULL) {
 				if (fDebuggerInterface->GetArchitecture()->CreateStackTrace(
-						fThread->GetTeam(), this, cpuState, stackTrace, 1,
-						false, false)
-						== B_OK) {
+						fThread->GetTeam(), this, cpuState, stackTrace, false,
+						1, false, false) == B_OK) {
 					stackTraceReference.SetTo(stackTrace, true);
 				}
 			}
@@ -680,8 +680,24 @@ ThreadHandler::_HandleSingleStepStep(CpuState* cpuState)
 		case STEP_OVER:
 		{
 			// If we have stepped out of the statement, we're done.
-			if (!fStepStatement->ContainsAddress(cpuState->InstructionPointer()))
+			if (!fStepStatement->ContainsAddress(cpuState->InstructionPointer())) {
+				StackTrace* stackTrace = fThread->GetStackTrace();
+				BReference<StackTrace> stackTraceReference(stackTrace);
+				if (stackTrace == NULL && cpuState != NULL) {
+					if (fDebuggerInterface->GetArchitecture()->CreateStackTrace(
+							fThread->GetTeam(), this, cpuState, stackTrace, false,
+							1, false, false) == B_OK) {
+						stackTraceReference.SetTo(stackTrace, true);
+					}
+				}
+
+				if (stackTrace != NULL && stackTrace->FrameAt(0)
+						->FrameAddress() != fPreviousFrameAddress) {
+					fThread->SetExecutedSubroutine();
+				}
+
 				return false;
+			}
 			return _DoStepOver(cpuState);
 		}
 

src/apps/debugger/debug_info/DebuggerImageDebugInfo.cpp

@@ -68,7 +68,7 @@ DebuggerImageDebugInfo::GetAddressSectionType(target_addr_t address)
 status_t
 DebuggerImageDebugInfo::CreateFrame(Image* image,
 	FunctionInstance* functionInstance, CpuState* cpuState,
-	bool getFullFrameInfo, StackFrame*& _previousFrame,
+	bool getFullFrameInfo, bool getReturnValue, StackFrame*& _previousFrame,
 	CpuState*& _previousCpuState)
 {
 	return B_UNSUPPORTED;

src/apps/debugger/debug_info/DebuggerImageDebugInfo.h

@@ -36,6 +36,7 @@ public:
 									FunctionInstance* functionInstance,
 									CpuState* cpuState,
 									bool getFullFrameInfo,
+									bool getReturnValue,
 									StackFrame*& _previousFrame,
 									CpuState*& _previousCpuState);
 	virtual	status_t			GetStatement(FunctionDebugInfo* function,

src/apps/debugger/debug_info/DwarfImageDebugInfo.cpp

@@ -53,6 +53,7 @@
 #include "StringUtils.h"
 #include "SymbolInfo.h"
 #include "TargetAddressRangeList.h"
+#include "Team.h"
 #include "TeamMemory.h"
 #include "Tracing.h"
 #include "TypeLookupConstraints.h"
@@ -521,7 +522,8 @@ DwarfImageDebugInfo::GetAddressSectionType(target_addr_t address)
 status_t
 DwarfImageDebugInfo::CreateFrame(Image* image,
 	FunctionInstance* functionInstance, CpuState* cpuState,
-	bool getFullFrameInfo, StackFrame*& _frame, CpuState*& _previousCpuState)
+	bool getFullFrameInfo, bool getReturnValue, StackFrame*& _frame,
+	CpuState*& _previousCpuState)
 {
 	DwarfFunctionDebugInfo* function = dynamic_cast<DwarfFunctionDebugInfo*>(
 		functionInstance->GetFunctionDebugInfo());
@@ -671,10 +673,7 @@ DwarfImageDebugInfo::CreateFrame(Image* image,
 			instructionPointer, functionInstance->Address() - fRelocationDelta,
 			subprogramEntry->Variables(), subprogramEntry->Blocks());
 
-		// determine if the previously executed instruction was a function
-		// call to see if we need to potentially retrieve a return value
-		// as well
-		if (instructionPointer > functionInstance->Address() - fRelocationDelta) {
+		if (getReturnValue) {
 			_CreateReturnValue(functionInstance, image, function, frame,
 				*stackFrameDebugInfo, instructionPointer);
 		}
@@ -1091,6 +1090,8 @@ DwarfImageDebugInfo::_CreateReturnValue(FunctionInstance* functionInstance,
 	Image* image, DwarfFunctionDebugInfo* function, StackFrame* frame,
 	DwarfStackFrameDebugInfo& factory, target_addr_t instructionPointer)
 {
+	// the thread just executed a subroutine, look for the last call
+	// instruction.
 	DisassembledCode* sourceCode = NULL;
 	target_size_t bufferSize = std::min(functionInstance->Size(),
 		(target_size_t)64 * 1024);
@@ -1114,59 +1115,74 @@ DwarfImageDebugInfo::_CreateReturnValue(FunctionInstance* functionInstance,
 		previousStatementAddress);
 	if (statement == NULL)
 		return B_BAD_VALUE;
-	previousStatementAddress = statement->CoveringAddressRange().Start() - 1;
-	statement = sourceCode->StatementAtAddress(
-		previousStatementAddress);
-	if (statement == NULL)
+
+	InstructionInfo info;
+	do {
+		TargetAddressRange range = statement->CoveringAddressRange();
+		result = fArchitecture->GetInstructionInfo(range.Start(), info);
+		if (result != B_OK)
+			return result;
+
+		if (info.Type() == INSTRUCTION_TYPE_SUBROUTINE_CALL)
+			break;
+
+		previousStatementAddress = statement->CoveringAddressRange().Start() - 1;
+		statement = sourceCode->StatementAtAddress(
+			previousStatementAddress);
+	} while (statement != NULL);
+
+	// we weren't able to find a subroutine call by stepping back
+	// so we can't retrieve a return value
+	if (info.Type() != INSTRUCTION_TYPE_SUBROUTINE_CALL)
+		return B_OK;
+
+	target_addr_t targetAddress = info.TargetAddress();
+	if (targetAddress == 0)
 		return B_BAD_VALUE;
 
-	TargetAddressRange range = statement->CoveringAddressRange();
-	InstructionInfo info;
-	if (fArchitecture->GetInstructionInfo(range.Start(), info) == B_OK
-		&& info.Type() == INSTRUCTION_TYPE_SUBROUTINE_CALL) {
-		target_addr_t targetAddress = info.TargetAddress();
-		if (targetAddress == 0)
+	if (!image->ContainsAddress(targetAddress)) {
+		// our current image doesn't contain the target function,
+		// locate the one which does.
+		image = image->GetTeam()->ImageByAddress(targetAddress);
+		if (image == NULL)
 			return B_BAD_VALUE;
+	}
 
-		if (image->ContainsAddress(targetAddress)) {
-			FunctionInstance* targetFunction;
-			if (targetAddress >= fPLTSectionStart && targetAddress < fPLTSectionEnd) {
-				// TODO: resolve actual target address in the PIC case
-				// and adjust targetAddress accordingly
+	FunctionInstance* targetFunction;
+	if (targetAddress >= fPLTSectionStart && targetAddress < fPLTSectionEnd) {
+		// TODO: resolve actual target address in the PIC case
+		// and adjust targetAddress accordingly
+	}
+	ImageDebugInfo* imageInfo = image->GetImageDebugInfo();
+	targetFunction = imageInfo->FunctionAtAddress(targetAddress);
+	if (targetFunction != NULL) {
+		DwarfFunctionDebugInfo* targetInfo =
+			dynamic_cast<DwarfFunctionDebugInfo*>(
+				targetFunction->GetFunctionDebugInfo());
+		if (targetInfo != NULL) {
+			DIESubprogram* subProgram = targetInfo->SubprogramEntry();
+			DIEType* returnType = subProgram->ReturnType();
+			if (returnType == NULL) {
+				// function doesn't return a value, we're done.
+				return B_OK;
 			}
-			ImageDebugInfo* imageInfo = image->GetImageDebugInfo();
-			targetFunction = imageInfo->FunctionAtAddress(targetAddress);
-			if (targetFunction != NULL) {
-				DwarfFunctionDebugInfo* targetInfo =
-					dynamic_cast<DwarfFunctionDebugInfo*>(
-						targetFunction->GetFunctionDebugInfo());
-				if (targetInfo != NULL) {
-					DIESubprogram* subProgram = targetInfo->SubprogramEntry();
-					DIEType* returnType = subProgram->ReturnType();
-					if (returnType == NULL) {
-						// function doesn't return a value, we're done.
-						return B_OK;
-					}
 
-					ValueLocation* location;
-					result = fArchitecture->GetReturnAddressLocation(frame,
-						returnType->ByteSize()->constant, location);
-					if (result != B_OK)
-						return result;
-					BReference<ValueLocation> locationReference(location,
-						true);
-					Variable* variable = NULL;
-					BReference<FunctionID> idReference(
-						targetFunction->GetFunctionID(), true);
-					result = factory.CreateReturnValue(idReference,
-						returnType, location, variable);
-					if (result != B_OK)
-						return result;
-					BReference<Variable> variableReference(variable, true);
-					if (!frame->AddLocalVariable(variable))
-						return B_NO_MEMORY;
-				}
-			}
+			ValueLocation* location;
+			result = fArchitecture->GetReturnAddressLocation(frame,
+				returnType->ByteSize()->constant, location);
+			if (result != B_OK)
+				return result;
+			BReference<ValueLocation> locationReference(location, true);
+			Variable* variable = NULL;
+			BReference<FunctionID> idReference(
+				targetFunction->GetFunctionID(), true);
+			result = factory.CreateReturnValue(idReference, returnType,
+				location, variable);
+			if (result != B_OK)
+				return result;
+			BReference<Variable> variableReference(variable, true);
+			if (!frame->AddLocalVariable(variable))
+				return B_NO_MEMORY;
 		}
 	}
 

src/apps/debugger/debug_info/DwarfImageDebugInfo.h

@@ -64,6 +64,7 @@ public:
 									FunctionInstance* functionInstance,
 									CpuState* cpuState,
 									bool getFullFrameInfo,
+									bool getReturnValue,
 									StackFrame*& _frame,
 									CpuState*& _previousCpuState);
 	virtual	status_t			GetStatement(FunctionDebugInfo* function,

src/apps/debugger/debug_info/SpecificImageDebugInfo.h

@@ -56,6 +56,7 @@ public:
 									FunctionInstance* functionInstance,
 									CpuState* cpuState,
 									bool getFullFrameInfo,
+									bool getReturnValue,
 									StackFrame*& _Frame,
 									CpuState*& _previousCpuState) = 0;
 										// returns reference to previous frame

src/apps/debugger/jobs/GetStackTraceJob.cpp

@@ -58,7 +58,7 @@ GetStackTraceJob::Do()
 	// get the stack trace
 	StackTrace* stackTrace;
 	status_t error = fArchitecture->CreateStackTrace(fThread->GetTeam(), this,
-		fCpuState, stackTrace);
+		fCpuState, stackTrace, fThread->ExecutedSubroutine());
 	if (error != B_OK)
 		return error;
 	BReference<StackTrace> stackTraceReference(stackTrace, true);

src/apps/debugger/model/Thread.cpp

@@ -17,6 +17,7 @@ Thread::Thread(Team* team, thread_id threadID)
 	fTeam(team),
 	fID(threadID),
 	fState(THREAD_STATE_UNKNOWN),
+	fExecutedSubroutine(false),
 	fStoppedReason(THREAD_STOPPED_UNKNOWN),
 	fCpuState(NULL),
 	fStackTrace(NULL)
@@ -68,6 +69,7 @@ Thread::SetState(uint32 state, uint32 reason, const BString& info)
 	if (fState != THREAD_STATE_STOPPED) {
 		SetCpuState(NULL);
 		SetStackTrace(NULL);
+		fExecutedSubroutine = false;
 	}
 
 	fTeam->NotifyThreadStateChanged(this);
@@ -108,3 +110,11 @@ Thread::SetStackTrace(StackTrace* trace)
 
 	fTeam->NotifyThreadStackTraceChanged(this);
 }
+
+
+void
+Thread::SetExecutedSubroutine()
+{
+	fExecutedSubroutine = true;
+}
+

src/apps/debugger/model/Thread.h

@@ -67,11 +67,16 @@ public:
 			StackTrace*			GetStackTrace() const	{ return fStackTrace; }
 			void				SetStackTrace(StackTrace* trace);
 
+			bool				ExecutedSubroutine() const
+									{ return fExecutedSubroutine; }
+			void				SetExecutedSubroutine();
+
 private:
 			Team*				fTeam;
 			thread_id			fID;
 			BString				fName;
 			uint32				fState;
+			bool				fExecutedSubroutine;
 			uint32				fStoppedReason;
 			BString				fStoppedReasonInfo;
 			CpuState*			fCpuState;