This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Resubmit process record and replay, 7/10


My 2c.,

On Sunday 16 November 2008 08:23:12, teawater wrote:
> --- a/linux-nat.c
> +++ b/linux-nat.c
> @@ -50,6 +50,8 @@
> Â#include "event-loop.h"
> Â#include "event-top.h"
> Â
> +#include "record.h"
> +
> Â#ifdef HAVE_PERSONALITY
> Â# include <sys/personality.h>
> Â# if !HAVE_DECL_ADDR_NO_RANDOMIZE
> @@ -518,6 +520,115 @@ my_waitpid (int pid, int *status, int fl
> Â Âreturn ret;
> Â}
> Â

Can you please try moving all this blob...

> +extern struct bp_location *bp_location_chain;
> +static struct lwp_info * find_lwp_pid (ptid_t ptid);
> +static int
> +my_waitpid_record (int pid, int *status, int flags)
> +{
> + Âint ret;
> + Âstruct bp_location *bl;
> + Âstruct breakpoint *b;
> + ÂCORE_ADDR pc;
> + ÂCORE_ADDR decr_pc_after_break;
> + Âstruct lwp_info *lp;
> + Âint is_breakpoint = 1;
> +
> +wait_begin:
> + Âret = my_waitpid (pid, status, flags);
> + Âif (ret == -1)
> + Â Â{
> + Â Â Âreturn ret;
> + Â Â}
> +
> + Âif (ret == 0)
> + Â Â{
> + Â Â Âgoto wait_begin;
> + Â Â}
> +
> + Âif (WIFSTOPPED (*status) && WSTOPSIG (*status) == SIGTRAP)
> + Â Â{
> + Â Â Â/* Check if there is a breakpoint. Â*/
> + Â Â Âpc = 0;
> + Â Â Âregisters_changed ();
> + Â Â Âfor (bl = bp_location_chain; bl; bl = bl->global_next)
> +ÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂ Âb = bl->owner;
> +ÂÂÂÂÂÂÂ Âgdb_assert (b);
> +ÂÂÂÂÂÂÂ Âif (b->enable_state != bp_enabled
> +ÂÂÂÂÂÂÂ Â Â Â&& b->enable_state != bp_permanent)
> +ÂÂÂÂÂÂÂ Â Âcontinue;
> +ÂÂÂÂÂÂÂ Âif (!pc)
> +ÂÂÂÂÂÂÂ Â Â{
> +ÂÂÂÂÂÂÂ Â Â Âpc = regcache_read_pc (get_thread_regcache (pid_to_ptid (ret)));
> +ÂÂÂÂÂÂÂ Â Â}
> +ÂÂÂÂÂÂÂ Âswitch (b->type)
> +ÂÂÂÂÂÂÂ Â Â{
> +ÂÂÂÂÂÂÂ Â Âdefault:
> +ÂÂÂÂÂÂÂ Â Â Âif (bl->address == pc)
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Âgoto out;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ}
> +ÂÂÂÂÂÂÂ Â Â Âbreak;
> +
> +ÂÂÂÂÂÂÂ Â Âcase bp_watchpoint:
> +ÂÂÂÂÂÂÂ Â Â Â/*XXX teawater: I still not very clear how to deal with it. Â*/
> +ÂÂÂÂÂÂÂ Â Â Âgoto out;
> +ÂÂÂÂÂÂÂ Â Â Âbreak;
> +
> +ÂÂÂÂÂÂÂ Â Âcase bp_catchpoint:
> +ÂÂÂÂÂÂÂ Â Â Âgdb_assert (b->ops != NULL && b->ops->breakpoint_hit != NULL);
> +ÂÂÂÂÂÂÂ Â Â Âif (b->ops->breakpoint_hit (b))
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Âgoto out;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ}
> +ÂÂÂÂÂÂÂ Â Â Âbreak;
> +
> +ÂÂÂÂÂÂÂ Â Âcase bp_hardware_watchpoint:
> +ÂÂÂÂÂÂÂ Â Âcase bp_read_watchpoint:
> +ÂÂÂÂÂÂÂ Â Âcase bp_access_watchpoint:
> +ÂÂÂÂÂÂÂ Â Â Âif (STOPPED_BY_WATCHPOINT (0))
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Âgoto out;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ}
> +ÂÂÂÂÂÂÂ Â Â Âbreak;
> +ÂÂÂÂÂÂÂ Â Â}
> +ÂÂÂÂÂÂÂ}
> +
> + Â Â Âlp = find_lwp_pid (pid_to_ptid (ret));
> + Â Â Âif (lp)
> + Â Â Â Âlp->stopped = 1;
> +
> + Â Â Â/* record message */
> + Â Â Ârecord_message (current_gdbarch);
> +
> + Â Â Â/* resume program */
> + Â Â Âlinux_ops->to_resume (pid_to_ptid (ret), 1, TARGET_SIGNAL_0);
> + Â Â Âgoto wait_begin;
> + Â Â}
> +
> + Âis_breakpoint = 0;
> +
> +out:
> + Â/* Add gdbarch_decr_pc_after_break to pc because pc will be break at address
> + Â Â add gdbarch_decr_pc_after_break when inferior non-step execute. Â*/
> + Âif (is_breakpoint)
> + Â Â{
> + Â Â Âdecr_pc_after_break = gdbarch_decr_pc_after_break
> +ÂÂÂÂÂÂÂ(get_regcache_arch (get_thread_regcache (pid_to_ptid (ret))));
> + Â Â Âif (decr_pc_after_break)
> +ÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂ Âif (!pc)
> +ÂÂÂÂÂÂÂ Â Â{
> +ÂÂÂÂÂÂÂ Â Â Âpc = regcache_read_pc (get_thread_regcache (pid_to_ptid (ret)));
> +ÂÂÂÂÂÂÂ Â Â}
> +ÂÂÂÂÂÂÂ Âregcache_write_pc (get_thread_regcache (pid_to_ptid (ret)),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Â Â pc + decr_pc_after_break);
> +ÂÂÂÂÂÂÂ}
> + Â Â}
> +
> + Âreturn ret;
> +}
> +

... to the record target?  It seems to be interested in getting
*all* events, instead of letting linux_nat_wait filter some.

> Â/* Determine if PTRACE_O_TRACEFORK can be used to follow fork events.
> Â
> Â Â First, we try to enable fork tracing on ORIGINAL_PID. ÂIf this fails,
> @@ -2876,7 +2987,16 @@ retry:
> ÂÂÂÂÂÂÂÂ Â queued events. Â*/
> ÂÂÂÂÂÂÂÂlwpid = queued_waitpid (pid, &status, options);
> Â Â Â Âelse
> -ÂÂÂÂÂÂÂlwpid = my_waitpid (pid, &status, options);
> +ÂÂÂÂÂÂÂ{
> +ÂÂÂÂÂÂÂ Âif (RECORD_IS_USED && !record_resume_step)
> +ÂÂÂÂÂÂÂ Â Â{
> +ÂÂÂÂÂÂÂ Â Â Âlwpid = my_waitpid_record (pid, &status, options);
> +ÂÂÂÂÂÂÂ Â Â}
> +ÂÂÂÂÂÂÂ Âelse
> +ÂÂÂÂÂÂÂ Â Â{
> +ÂÂÂÂÂÂÂ Â Â Âlwpid = my_waitpid (pid, &status, options);
> +ÂÂÂÂÂÂÂ Â Â}
> +ÂÂÂÂÂÂÂ}
> Â

Could you do it by instead of calling my_waitpid_record here, make
sure that whatever comes out of my_waitpid results in returning from
linux_nat_wait?

Ideally, we should have a flags parameter in target_wait, like:

 -ptid_t target_wait (ptid_t, struct target_waitstatus *);
 +ptid_t target_wait (ptid_t, struct target_waitstatus *, int target_flags);

... process record would pass a special flag to
linux_nat_wait for this, but I'd be happy if you tried moving
most of the code to the record target, and kept using RECORD_IS_USED,
as I hinted at in another message a new target_is_recording_p () or
somesuch method.

Also, it seems most of the breakpoint checking code should be replaced
by one of the breakpoint_here style predicates exported by breakpoint.h.

-- 
Pedro Alves

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]