From a9bec97e472fd70d446da9b0fb1eff28dd0f0549 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Thu, 15 Dec 2011 22:40:32 -0500 Subject: [PATCH 1/2] Add previous CPU state member and associated accessors. Used to store the unwound state of the CPU from this frame, which will form the actual state of the following frame. Needed in order to properly support continuation of unwinding from a partial stack trace. --- src/apps/debugger/model/StackFrame.cpp | 14 ++++++++++++++ src/apps/debugger/model/StackFrame.h | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/src/apps/debugger/model/StackFrame.cpp b/src/apps/debugger/model/StackFrame.cpp index 14f81ff705..6cb760c7d2 100644 --- a/src/apps/debugger/model/StackFrame.cpp +++ b/src/apps/debugger/model/StackFrame.cpp @@ -25,6 +25,7 @@ StackFrame::StackFrame(stack_frame_type type, CpuState* cpuState, : fType(type), fCpuState(cpuState), + fPreviousCpuState(NULL), fFrameAddress(frameAddress), fInstructionPointer(instructionPointer), fReturnAddress(0), @@ -49,6 +50,7 @@ StackFrame::~StackFrame() SetImage(NULL); SetFunction(NULL); + SetPreviousCpuState(NULL); fDebugInfo->ReleaseReference(); fCpuState->ReleaseReference(); @@ -80,6 +82,18 @@ StackFrame::Init() } +void +StackFrame::SetPreviousCpuState(CpuState* state) +{ + if (fPreviousCpuState != NULL) + fPreviousCpuState->ReleaseReference(); + + fPreviousCpuState = state; + + if (fPreviousCpuState != NULL) + fPreviousCpuState->AcquireReference(); +} + void StackFrame::SetReturnAddress(target_addr_t address) { diff --git a/src/apps/debugger/model/StackFrame.h b/src/apps/debugger/model/StackFrame.h index e4c2f975f7..0e30d53a13 100644 --- a/src/apps/debugger/model/StackFrame.h +++ b/src/apps/debugger/model/StackFrame.h @@ -55,6 +55,10 @@ public: target_addr_t InstructionPointer() const { return fInstructionPointer; } + CpuState* GetPreviousCpuState() const + { return fPreviousCpuState; } + void SetPreviousCpuState(CpuState* state); + target_addr_t ReturnAddress() const { return fReturnAddress; } void SetReturnAddress(target_addr_t address); @@ -89,6 +93,7 @@ private: private: stack_frame_type fType; CpuState* fCpuState; + CpuState* fPreviousCpuState; target_addr_t fFrameAddress; target_addr_t fInstructionPointer; target_addr_t fReturnAddress; From 26334a8a66a1365b8fbd135a9dacc8fe897b6d38 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Thu, 15 Dec 2011 22:43:38 -0500 Subject: [PATCH 2/2] Fix unwinding of partial stack traces. - Architecture::CreateStackTrace() now uses the last frame's PreviousCpuState() as the basis to continue unwinding when passed a partial trace to continue from, rather than the (incorrect) actual cpu state of that frame, which would have resulted in the last frame being duplicated in the trace. - Renamed variables to be more clear. --- src/apps/debugger/arch/Architecture.cpp | 30 +++++++++---------- src/apps/debugger/arch/Architecture.h | 2 +- .../debugger/arch/x86/ArchitectureX86.cpp | 5 ++-- .../debug_info/DwarfImageDebugInfo.cpp | 6 ++-- .../debugger/debug_info/DwarfImageDebugInfo.h | 2 +- .../debug_info/SpecificImageDebugInfo.h | 2 +- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/apps/debugger/arch/Architecture.cpp b/src/apps/debugger/arch/Architecture.cpp index 084b8ee690..c58be0e645 100644 --- a/src/apps/debugger/arch/Architecture.cpp +++ b/src/apps/debugger/arch/Architecture.cpp @@ -100,7 +100,7 @@ Architecture::CreateStackTrace(Team* team, StackTrace* stackTrace = NULL; ObjectDeleter stackTraceDeleter; - StackFrame* frame = NULL; + StackFrame* nextFrame = NULL; if (useExistingTrace) stackTrace = _stackTrace; @@ -115,8 +115,8 @@ Architecture::CreateStackTrace(Team* team, // if we're passed an already existing partial stack trace, // attempt to continue building it from where it left off. if (stackTrace->CountFrames() > 0) { - frame = stackTrace->FrameAt(stackTrace->CountFrames() - 1); - cpuState = frame->GetCpuState(); + nextFrame = stackTrace->FrameAt(stackTrace->CountFrames() - 1); + cpuState = nextFrame->GetPreviousCpuState(); } while (cpuState != NULL) { @@ -152,42 +152,42 @@ Architecture::CreateStackTrace(Team* team, // If the CPU state's instruction pointer is actually the return address // of the next frame, we let the architecture fix that. - if (frame != NULL - && frame->ReturnAddress() == cpuState->InstructionPointer()) { - UpdateStackFrameCpuState(frame, image, + if (nextFrame != NULL + && nextFrame->ReturnAddress() == cpuState->InstructionPointer()) { + UpdateStackFrameCpuState(nextFrame, image, functionDebugInfo, cpuState); } // create the frame using the debug info - StackFrame* previousFrame = NULL; + StackFrame* frame = NULL; CpuState* previousCpuState = NULL; if (function != NULL) { status_t error = functionDebugInfo->GetSpecificImageDebugInfo() - ->CreateFrame(image, function, cpuState, previousFrame, + ->CreateFrame(image, function, cpuState, frame, previousCpuState); if (error != B_OK && error != B_UNSUPPORTED) break; } // If we have no frame yet, let the architecture create it. - if (previousFrame == NULL) { + if (frame == NULL) { status_t error = CreateStackFrame(image, functionDebugInfo, - cpuState, frame == NULL, previousFrame, previousCpuState); + cpuState, nextFrame == NULL, frame, previousCpuState); if (error != B_OK) break; } cpuStateReference.SetTo(previousCpuState, true); - previousFrame->SetImage(image); - previousFrame->SetFunction(function); + frame->SetImage(image); + frame->SetFunction(function); - if (!stackTrace->AddFrame(previousFrame)) { - delete previousFrame; + if (!stackTrace->AddFrame(frame)) { + delete frame; return B_NO_MEMORY; } - frame = previousFrame; + frame = nextFrame; cpuState = previousCpuState; if (--maxStackDepth == 0) break; diff --git a/src/apps/debugger/arch/Architecture.h b/src/apps/debugger/arch/Architecture.h index 1f0d40b306..845af2e2f7 100644 --- a/src/apps/debugger/arch/Architecture.h +++ b/src/apps/debugger/arch/Architecture.h @@ -67,7 +67,7 @@ public: virtual status_t CreateStackFrame(Image* image, FunctionDebugInfo* function, CpuState* cpuState, bool isTopFrame, - StackFrame*& _previousFrame, + StackFrame*& _frame, CpuState*& _previousCpuState) = 0; // returns reference to previous frame // and CPU state; returned CPU state diff --git a/src/apps/debugger/arch/x86/ArchitectureX86.cpp b/src/apps/debugger/arch/x86/ArchitectureX86.cpp index 61269a7063..cc856cedce 100644 --- a/src/apps/debugger/arch/x86/ArchitectureX86.cpp +++ b/src/apps/debugger/arch/x86/ArchitectureX86.cpp @@ -265,7 +265,7 @@ ArchitectureX86::CreateCpuState(const void* cpuStateData, size_t size, status_t ArchitectureX86::CreateStackFrame(Image* image, FunctionDebugInfo* function, - CpuState* _cpuState, bool isTopFrame, StackFrame*& _previousFrame, + CpuState* _cpuState, bool isTopFrame, StackFrame*& _frame, CpuState*& _previousCpuState) { CpuStateX86* cpuState = dynamic_cast(_cpuState); @@ -382,11 +382,12 @@ ArchitectureX86::CreateStackFrame(Image* image, FunctionDebugInfo* function, previousCpuState->SetIntRegister(X86_REGISTER_EBP, previousFramePointer); previousCpuState->SetIntRegister(X86_REGISTER_EIP, returnAddress); + frame->SetPreviousCpuState(previousCpuState); } frame->SetReturnAddress(returnAddress); - _previousFrame = frameReference.Detach(); + _frame = frameReference.Detach(); _previousCpuState = previousCpuState; return B_OK; } diff --git a/src/apps/debugger/debug_info/DwarfImageDebugInfo.cpp b/src/apps/debugger/debug_info/DwarfImageDebugInfo.cpp index 45d13884be..7731e5ae88 100644 --- a/src/apps/debugger/debug_info/DwarfImageDebugInfo.cpp +++ b/src/apps/debugger/debug_info/DwarfImageDebugInfo.cpp @@ -495,7 +495,7 @@ DwarfImageDebugInfo::GetAddressSectionType(target_addr_t address) status_t DwarfImageDebugInfo::CreateFrame(Image* image, FunctionInstance* functionInstance, CpuState* cpuState, - StackFrame*& _previousFrame, CpuState*& _previousCpuState) + StackFrame*& _frame, CpuState*& _previousCpuState) { DwarfFunctionDebugInfo* function = dynamic_cast( functionInstance->GetFunctionDebugInfo()); @@ -634,9 +634,11 @@ DwarfImageDebugInfo::CreateFrame(Image* image, instructionPointer, functionInstance->Address() - fRelocationDelta, subprogramEntry->Variables(), subprogramEntry->Blocks()); - _previousFrame = frameReference.Detach(); + _frame = frameReference.Detach(); _previousCpuState = previousCpuStateReference.Detach(); + frame->SetPreviousCpuState(_previousCpuState); + return B_OK; } diff --git a/src/apps/debugger/debug_info/DwarfImageDebugInfo.h b/src/apps/debugger/debug_info/DwarfImageDebugInfo.h index e0ea796fc4..4fde30c3f3 100644 --- a/src/apps/debugger/debug_info/DwarfImageDebugInfo.h +++ b/src/apps/debugger/debug_info/DwarfImageDebugInfo.h @@ -61,7 +61,7 @@ public: virtual status_t CreateFrame(Image* image, FunctionInstance* functionInstance, CpuState* cpuState, - StackFrame*& _previousFrame, + StackFrame*& _frame, CpuState*& _previousCpuState); virtual status_t GetStatement(FunctionDebugInfo* function, target_addr_t address, diff --git a/src/apps/debugger/debug_info/SpecificImageDebugInfo.h b/src/apps/debugger/debug_info/SpecificImageDebugInfo.h index 2778c887f5..ee141340c2 100644 --- a/src/apps/debugger/debug_info/SpecificImageDebugInfo.h +++ b/src/apps/debugger/debug_info/SpecificImageDebugInfo.h @@ -53,7 +53,7 @@ public: virtual status_t CreateFrame(Image* image, FunctionInstance* functionInstance, CpuState* cpuState, - StackFrame*& _previousFrame, + StackFrame*& _Frame, CpuState*& _previousCpuState) = 0; // returns reference to previous frame // and CPU state; returned CPU state