* Re: frysk-core/frysk/proc Breakpoint.java IsaIA32. ... [not found] <20070115134847.31787.qmail@sourceware.org> @ 2007-08-22 15:11 ` Phil Muldoon 2007-08-22 18:08 ` Mark Wielaard 0 siblings, 1 reply; 3+ messages in thread From: Phil Muldoon @ 2007-08-22 15:11 UTC (permalink / raw) To: frysk mark@sourceware.org wrote: > CVSROOT: /cvs/frysk > Module name: frysk-core > Changes by: mark@sourceware.org 2007-01-15 13:48:47 > > Modified files: > frysk/proc : Breakpoint.java IsaIA32.java IsaX8664.java > LinuxIa32On64.java LinuxPtraceTaskState.java > TestTaskObserverInstruction.java > TestTaskObserverInstructionAndCode.java > > Log message: > Fixes bug #3676 > * Breakpoint.java (stepDone): Only set if still installed. > (isInstalled): new method. > (toString): Prettify. > * IsaIA32.java (isTaskStepped): Reset flag. > * IsaX8664.java (isTaskStepped): Likewise. > Mark, As we talked about on irc, and in addition to looking at this code in preparation for hardware watchpoints, can I make a small recommendation here? isTaskStepped(task) is a query, and should imo not be altering any debug registers after the fact. Can I propose a split into: isTaskStepped(task) and setTaskStepped(task) Does that make better sense? Regards Phil ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: frysk-core/frysk/proc Breakpoint.java IsaIA32. ... 2007-08-22 15:11 ` frysk-core/frysk/proc Breakpoint.java IsaIA32. Phil Muldoon @ 2007-08-22 18:08 ` Mark Wielaard 2007-08-22 21:47 ` Phil Muldoon 0 siblings, 1 reply; 3+ messages in thread From: Mark Wielaard @ 2007-08-22 18:08 UTC (permalink / raw) To: Phil Muldoon; +Cc: frysk Hi Phil, On Wed, 2007-08-22 at 10:11 -0500, Phil Muldoon wrote: > > Fixes bug #3676 > > * Breakpoint.java (stepDone): Only set if still installed. > > (isInstalled): new method. > > (toString): Prettify. > > * IsaIA32.java (isTaskStepped): Reset flag. > > * IsaX8664.java (isTaskStepped): Likewise. > > > As we talked about on irc, and in addition to looking at this code in > preparation for hardware watchpoints, can I make a small recommendation > here? > > isTaskStepped(task) is a query, and should imo not be altering any debug > registers after the fact. > > Can I propose a split into: > > isTaskStepped(task) and setTaskStepped(task) > > Does that make better sense? That does make sense. Although I don't know if someone really needs setTaskStepped(), so I wouldn't introduce it unless there was a real usage for it. The reason why the flag is reset here is mentioned in the original patch email: http://sourceware.org/ml/frysk/2007-q1/msg00024.html > There is one thing that changed in the semantics of Isa.isTaskStepped() > for x86 and x86_64 (and Ia32On64 has been added). That is that the > stepping flag in the d6 register is being reset because "[the d6] > register is never cleared by the processor and must be cleared by > software after the contents have been read". This means that we are now > doing a inferior visible change, but I don't see any way to get around > this. If the inferior would be using instruction stepping itself there > would be all kinds of interesting issues anyway. I do agree this might not be the most ideal place to reset this flag though. Maybe it should be moved into LinuxTask.sendContinue()? Cheers, Mark ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: frysk-core/frysk/proc Breakpoint.java IsaIA32. ... 2007-08-22 18:08 ` Mark Wielaard @ 2007-08-22 21:47 ` Phil Muldoon 0 siblings, 0 replies; 3+ messages in thread From: Phil Muldoon @ 2007-08-22 21:47 UTC (permalink / raw) To: Mark Wielaard; +Cc: frysk Mark Wielaard wrote: > I do agree this might not be the most ideal place to reset this flag > though. Maybe it should be moved into LinuxTask.sendContinue()? > > > Not sure where, but hopefully somewhere better than an a query function. Regards Phil ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-08-22 21:47 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20070115134847.31787.qmail@sourceware.org> 2007-08-22 15:11 ` frysk-core/frysk/proc Breakpoint.java IsaIA32. Phil Muldoon 2007-08-22 18:08 ` Mark Wielaard 2007-08-22 21:47 ` Phil Muldoon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).