From 2460bf468bd81eb00457e5e0137c736ffa8ffda4 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Wed, 24 Jun 2009 22:10:07 +0000 Subject: [PATCH] * Changes that should already have been part of r31228: StackFrame and SourceView. * Fixed the information flow problem in Architecture::CreateStackTrace()/ ArchitectureX86::UpdateStackCpuState() by introducing a virtual UpdateStackFrameCpuState() which allows the architecture to update the CPU state it generated before after the function the state belongs to is known. That's where moving the instruction pointer to the previous instruction happens now. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@31229 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/apps/debugger/arch/Architecture.cpp | 10 ++- src/apps/debugger/arch/Architecture.h | 8 ++ .../debugger/arch/x86/ArchitectureX86.cpp | 74 +++++++++++-------- src/apps/debugger/arch/x86/ArchitectureX86.h | 5 ++ .../debugger/gui/team_window/SourceView.cpp | 5 +- src/apps/debugger/model/StackFrame.cpp | 10 +-- src/apps/debugger/model/StackFrame.h | 12 ++- 7 files changed, 78 insertions(+), 46 deletions(-) diff --git a/src/apps/debugger/arch/Architecture.cpp b/src/apps/debugger/arch/Architecture.cpp index 80b39052b3..a775d27202 100644 --- a/src/apps/debugger/arch/Architecture.cpp +++ b/src/apps/debugger/arch/Architecture.cpp @@ -52,6 +52,7 @@ Architecture::CreateStackTrace(Team* team, return B_NO_MEMORY; ObjectDeleter stackTraceDeleter(stackTrace); + bool architectureFrame = false; StackFrame* frame = NULL; while (cpuState != NULL) { @@ -78,6 +79,11 @@ Architecture::CreateStackTrace(Team* team, function = imageDebugInfo->FindFunction(instructionPointer); Reference functionReference(function, true); + // If the last frame had been created by the architecture, we update the + // CPU state. + if (architectureFrame) + UpdateStackFrameCpuState(frame, image, function, cpuState); + // create the frame using the debug info StackFrame* previousFrame = NULL; CpuState* previousCpuState = NULL; @@ -94,7 +100,9 @@ Architecture::CreateStackTrace(Team* team, frame == NULL, previousFrame, previousCpuState); if (error != B_OK) break; - } + architectureFrame = true; + } else + architectureFrame = false; cpuStateReference.SetTo(previousCpuState, true); diff --git a/src/apps/debugger/arch/Architecture.h b/src/apps/debugger/arch/Architecture.h index 607462fb7d..32801964df 100644 --- a/src/apps/debugger/arch/Architecture.h +++ b/src/apps/debugger/arch/Architecture.h @@ -43,6 +43,14 @@ public: // returns reference to previous frame // and CPU state; returned CPU state // can be NULL + virtual void UpdateStackFrameCpuState( + const StackFrame* frame, + Image* previousImage, + FunctionDebugInfo* previousFunction, + CpuState* previousCpuState) = 0; + // Called after a CreateStackFrame() + // with the image/function corresponding + // to the CPU state. virtual status_t DisassembleCode(FunctionDebugInfo* function, const void* buffer, size_t bufferSize, SourceCode*& _sourceCode) = 0; diff --git a/src/apps/debugger/arch/x86/ArchitectureX86.cpp b/src/apps/debugger/arch/x86/ArchitectureX86.cpp index 48f4f22b90..1e199cdbd5 100644 --- a/src/apps/debugger/arch/x86/ArchitectureX86.cpp +++ b/src/apps/debugger/arch/x86/ArchitectureX86.cpp @@ -127,35 +127,9 @@ ArchitectureX86::CreateStackFrame(Image* image, FunctionDebugInfo* function, frameType = STACK_FRAME_TYPE_SYSCALL; eip -= 2; // int 99, sysenter, and syscall all are 2 byte instructions - } else { + } else frameType = STACK_FRAME_TYPE_STANDARD; - // If this is not a top-frame, we offset eip to the previous (calling) - // instruction. - if (!isTopFrame && function != NULL && eip > function->Address()) { - size_t bufferSize = eip - function->Address(); - void* buffer = malloc(bufferSize); - if (buffer != NULL) { - ssize_t bytesRead = fDebuggerInterface->ReadMemory( - function->Address(), buffer, bufferSize); - if (bytesRead == (ssize_t)bufferSize) { - DisassemblerX86 disassembler; - target_addr_t instructionAddress; - target_size_t instructionSize; - if (disassembler.Init(function->Address(), - buffer, bufferSize) == B_OK - && disassembler.GetPreviousInstruction(eip, - instructionAddress, instructionSize) == B_OK) { - eip -= instructionSize; - cpuState->SetIntRegister(X86_REGISTER_EIP, eip); - } - } - - free(buffer); - } - } - } - // create the stack frame StackFrame* frame = new(std::nothrow) StackFrame(frameType, cpuState, framePointer, eip); @@ -176,11 +150,6 @@ ArchitectureX86::CreateStackFrame(Image* image, FunctionDebugInfo* function, frame->SetReturnAddress(frameData[1]); previousCpuState->SetIntRegister(X86_REGISTER_EBP, frameData[0]); previousCpuState->SetIntRegister(X86_REGISTER_EIP, frameData[1]); - // TODO: Actually it's the instruction before! We're currently - // offsetting it at the beginning of this method, but that's not - // correct, since for the previous stack frame there could be more - // debug info. Problem is that we don't have the function for the - // previous stack frame available at this point. } _previousFrame = frameReference.Detach(); @@ -189,6 +158,47 @@ ArchitectureX86::CreateStackFrame(Image* image, FunctionDebugInfo* function, } +void +ArchitectureX86::UpdateStackFrameCpuState(const StackFrame* frame, + Image* previousImage, FunctionDebugInfo* previousFunction, + CpuState* previousCpuState) +{ + // This is not a top frame, so we want to offset eip to the previous + // (calling) instruction. + CpuStateX86* cpuState = dynamic_cast(previousCpuState); + + // get eip + uint32 eip = cpuState->IntRegisterValue(X86_REGISTER_EIP); + if (previousFunction == NULL || eip <= previousFunction->Address()) + return; + target_addr_t functionAddresss = previousFunction->Address(); + + // allocate a buffer for the function code to disassemble + size_t bufferSize = eip - functionAddresss; + void* buffer = malloc(bufferSize); + if (buffer == NULL) + return; + MemoryDeleter bufferDeleter(buffer); + + // read the code + ssize_t bytesRead = fDebuggerInterface->ReadMemory(functionAddresss, buffer, + bufferSize); + if (bytesRead != (ssize_t)bufferSize) + return; + + // disassemble to get the previous instruction + DisassemblerX86 disassembler; + target_addr_t instructionAddress; + target_size_t instructionSize; + if (disassembler.Init(functionAddresss, buffer, bufferSize) == B_OK + && disassembler.GetPreviousInstruction(eip, instructionAddress, + instructionSize) == B_OK) { + eip -= instructionSize; + cpuState->SetIntRegister(X86_REGISTER_EIP, eip); + } +} + + status_t ArchitectureX86::DisassembleCode(FunctionDebugInfo* function, const void* buffer, size_t bufferSize, SourceCode*& _sourceCode) diff --git a/src/apps/debugger/arch/x86/ArchitectureX86.h b/src/apps/debugger/arch/x86/ArchitectureX86.h index abdd7bd019..adcc8eecea 100644 --- a/src/apps/debugger/arch/x86/ArchitectureX86.h +++ b/src/apps/debugger/arch/x86/ArchitectureX86.h @@ -28,6 +28,11 @@ public: CpuState* cpuState, bool isTopFrame, StackFrame*& _previousFrame, CpuState*& _previousCpuState); + virtual void UpdateStackFrameCpuState( + const StackFrame* frame, + Image* previousImage, + FunctionDebugInfo* previousFunction, + CpuState* previousCpuState); virtual status_t DisassembleCode(FunctionDebugInfo* function, const void* buffer, size_t bufferSize, SourceCode*& _sourceCode); diff --git a/src/apps/debugger/gui/team_window/SourceView.cpp b/src/apps/debugger/gui/team_window/SourceView.cpp index bf3355d236..ea3f327187 100644 --- a/src/apps/debugger/gui/team_window/SourceView.cpp +++ b/src/apps/debugger/gui/team_window/SourceView.cpp @@ -601,8 +601,11 @@ SourceView::MarkerView::_UpdateIPMarkers() if (line >= (uint32)LineCount()) continue; + bool isTopFrame = i == 0 + && frame->Type() != STACK_FRAME_TYPE_SYSCALL; + Marker* marker = new(std::nothrow) InstructionPointerMarker( - line, i == 0, frame == fStackFrame); + line, isTopFrame, frame == fStackFrame); if (marker == NULL || !fIPMarkers.AddItem(marker)) { delete marker; break; diff --git a/src/apps/debugger/model/StackFrame.cpp b/src/apps/debugger/model/StackFrame.cpp index 649d6c065e..96702b96d8 100644 --- a/src/apps/debugger/model/StackFrame.cpp +++ b/src/apps/debugger/model/StackFrame.cpp @@ -15,11 +15,12 @@ StackFrame::StackFrame(stack_frame_type type, CpuState* cpuState, - target_addr_t frameAddress) + target_addr_t frameAddress, target_addr_t instructionPointer) : fType(type), fCpuState(cpuState), fFrameAddress(frameAddress), + fInstructionPointer(instructionPointer), fReturnAddress(0), fImage(NULL), fFunction(NULL), @@ -39,13 +40,6 @@ StackFrame::~StackFrame() } -target_addr_t -StackFrame::InstructionPointer() const -{ - return fCpuState->InstructionPointer(); -} - - void StackFrame::SetReturnAddress(target_addr_t address) { diff --git a/src/apps/debugger/model/StackFrame.h b/src/apps/debugger/model/StackFrame.h index 9f954963ef..e727d88100 100644 --- a/src/apps/debugger/model/StackFrame.h +++ b/src/apps/debugger/model/StackFrame.h @@ -14,8 +14,8 @@ enum stack_frame_type { - STACK_FRAME_TYPE_TOP, // top-most frame - STACK_FRAME_TYPE_STANDARD, // non-top-most standard frame + STACK_FRAME_TYPE_SYSCALL, // syscall frame + STACK_FRAME_TYPE_STANDARD, // standard frame STACK_FRAME_TYPE_SIGNAL, // signal handler frame STACK_FRAME_TYPE_FRAMELESS // dummy frame for a frameless function }; @@ -42,14 +42,17 @@ public: public: StackFrame(stack_frame_type type, CpuState* cpuState, - target_addr_t frameAddress); + target_addr_t frameAddress, + target_addr_t instructionPointer); ~StackFrame(); stack_frame_type Type() const { return fType; } CpuState* GetCpuState() const { return fCpuState; } - target_addr_t InstructionPointer() const; target_addr_t FrameAddress() const { return fFrameAddress; } + target_addr_t InstructionPointer() const + { return fInstructionPointer; } + target_addr_t ReturnAddress() const { return fReturnAddress; } void SetReturnAddress(target_addr_t address); @@ -76,6 +79,7 @@ private: stack_frame_type fType; CpuState* fCpuState; target_addr_t fFrameAddress; + target_addr_t fInstructionPointer; target_addr_t fReturnAddress; Image* fImage; FunctionDebugInfo* fFunction;