[haiku-commits] Re: haiku: hrev43504 - in src/apps/debugger: . arch

  • From: Rene Gollent <anevilyak@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 15 Dec 2011 08:05:33 -0500

On Thu, Dec 15, 2011 at 2:53 AM, Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:
> Unless I'm much mistaken, this is the wrong CpuState. Rereading the function, 
> I have to admit that the naming used here isn't particularly good. The 
> previousCpuState/previousFrame (also in Architecture::CreateStackFrame() and 
> SpecificImageDebugInfo::CreateFrame()) seem to suggest that the CpuState 
> belongs to the StackFrame. That is not the case, though. We start with a 
> CpuState, create a StackFrame from it, and unwind the registers to get a 
> CpuState for the previous stack frame. Using the previous/next nomenclature 
> to mean the upper/lower or inner/outer frame, the 
> Architecture::CreateStackFrame() and SpecificImageDebugInfo::CreateFrame() 
> parameters should be named cpuState, _frame, and _previousCpuState, and the 
> local parameters here nextFrame and cpuState (the ones outside the loop) and 
> frame and previousCpuState inside the loop (as returned by 
> Architecture::CreateStackFrame() and SpecificImageDebugInfo::CreateFrame() 
> respectively).

Oops, you're right, what I did will wind up replicating the last frame
again upon resume. Fortunately that functionality isn't actually used
anywhere at this point, will fix it tonight, and update the naming as
suggested.

>
> Long story short, cpuState is not frame->GetCpuState(). In fact when creating 
> the stack trace, the last cpuState -- the one which needs to be used to 
> continue -- is deleted. I suggest to add a previousCpuState property to 
> StackTrace, which will be NULL, if the stack trace is complete.

Sounds like a plan, that'd also allow us to easily detect if we can
short circuit if asked to continue an already completed trace for some
reason.

Regards,

Rene

Other related posts: