src/gdb ChangeLog amd64-linux-nat.c amd64fbsd- ...

[palves at sourceware dot org]

CVSROOT:	/cvs/src
Module name:	src
Changes by:	palves@sourceware.org	2013-02-13 14:59:49

Modified files:
	gdb            : ChangeLog amd64-linux-nat.c amd64fbsd-nat.c 
	                 i386-linux-nat.c i386-nat.c i386-nat.h 
	                 linux-fork.c linux-nat.c linux-nat.h 
	                 windows-nat.c 

Log message:
	[native x86 GNU/Linux] Access debug register mirror from the corresponding process.
	
	While reviewing the native AArch64 patch, I noticed a problem:
	
	On 02/06/2013 08:46 PM, Pedro Alves wrote:
	>
	>> > +static void
	>> > +aarch64_linux_prepare_to_resume (struct lwp_info *lwp)
	>> > +{
	>> > +  struct arch_lwp_info *info = lwp->arch_private;
	>> > +
	>> > +  /* NULL means this is the main thread still going through the shell,
	>> > +     or, no watchpoint has been set yet.  In that case, there's
	>> > +     nothing to do.  */
	>> > +  if (info == NULL)
	>> > +    return;
	>> > +
	>> > +  if (DR_HAS_CHANGED (info->dr_changed_bp)
	>> > +      || DR_HAS_CHANGED (info->dr_changed_wp))
	>> > +    {
	>> > +      int tid = GET_LWP (lwp->ptid);
	>> > +      struct aarch64_debug_reg_state *state = aarch64_get_debug_reg_state ();
	> Hmm.  This is always fetching the debug_reg_state of
	> the current inferior, but may not be the inferior of lwp.
	> I see the same bug on x86.  Sorry about that.  I'll fix it.
	
	A natural fix would be to make xxx_get_debug_reg_state take an
	inferior argument, but that doesn't work because of the case where we
	detach breakpoints/watchpoints from the child fork, at a time there's
	no inferior for the child fork at all.  We do a nasty hack in
	i386_inferior_data_get, but that relies on all callers pointing the
	current inferior to the correct inferior, which isn't actually being
	done by all callers, and I don't think we want to enforce that -- deep
	in the bowls of linux-nat.c, there are many cases we resume lwps
	behind the scenes, and it's be better to not have that code rely on
	global state (as it doesn't today).
	
	The fix is to decouple the watchpoints code from inferiors, making it
	track target processes instead.  This way, we can freely keep track of
	the watchpoint mirrors for these processes behind the core's back.
	Checkpoints also play dirty tricks with swapping the process behind
	the inferior, so they get special treatment too in the patch (which
	just amounts to calling a new hook).  Instead of the old hack in
	i386_inferior_data_get, where we returned a copy of the current
	inferior's debug registers mirror, as soon as we detect a fork in the
	target, we copy the debug register mirror from the parent to the child
	process.
	
	I don't have an old kernel handy to test, but I stepped through gdb doing
	the watchpoint removal in the fork child in the watchpoint-fork test
	seeing that the debug registers end up cleared in the child.
	
	I didn't find the need for linux_nat_iterate_watchpoint_lwps.  If
	we use plain iterate_over_lwps instead, what happens is that
	when removing watchpoints, that iterate_over_lwps doesn't actually
	iterate over anything, since the fork child is not added to the
	lwp list until later, at detach time, in linux_child_follow_fork.
	And if we don't iterate over that lwp, we don't mark its debug
	registers as needing update.  But linux_child_follow_fork takes
	care of doing that explicitly:
	
	child_lp = add_lwp (inferior_ptid);
	child_lp->stopped = 1;
	child_lp->last_resume_kind = resume_stop;
	make_cleanup (delete_lwp_cleanup, child_lp);
	
	/* CHILD_LP has new PID, therefore linux_nat_new_thread is not called for it.
	See i386_inferior_data_get for the Linux kernel specifics.
	Ensure linux_nat_prepare_to_resume will reset the hardware debug
	registers.  It is done by the linux_nat_new_thread call, which is
	being skipped in add_lwp above for the first lwp of a pid.  */
	gdb_assert (num_lwps (GET_PID (child_lp->ptid)) == 1);
	if (linux_nat_new_thread != NULL)
	linux_nat_new_thread (child_lp);
	
	if (linux_nat_prepare_to_resume != NULL)
	linux_nat_prepare_to_resume (child_lp);
	ptrace (PTRACE_DETACH, child_pid, 0, 0);
	
	so unless I'm missing something (quite possible) it ends up all
	the same.  But, the !detach-on-fork, and the "follow-fork child" paths
	should also call linux_nat_new_thread, and they don't presently.  It
	seems to me in those cases we're not clearing debug regs correctly
	when that's needed.  Instead of copying that bit that works around
	add_lwp bypassing the linux_nat_new_thread call, I thought it'd
	be better to add an add_initial_lwp call to be used in the case we
	really need to bypass linux_nat_new_thread, and make
	add_lwp always call linux_nat_new_thread.
	
	i386_cleanup_dregs is rewritten to forget about the current process
	debug mirrors, which takes cares of other i386 ports.  Only a couple
	of extra tweaks here and there were needed, as some targets wheren't
	actually calling i386_cleanup_dregs.
	
	Tested on Fedora 17 x86_64 -m64/-m32.
	
	GDBserver already fetches the i386_debug_reg_state from the right
	process, and, it doesn't handle forks at all, so no fix is needed over
	there.
	
	gdb/
	2013-02-13  Pedro Alves  <palves@redhat.com>
	
	* amd64-linux-nat.c (update_debug_registers_callback):
	Update comment.
	(amd64_linux_dr_set_control, amd64_linux_dr_set_addr): Use
	iterate_over_lwps.
	(amd64_linux_prepare_to_resume): Pass the lwp's pid to
	i386_debug_reg_state.
	(amd64_linux_new_fork): New function.
	(_initialize_amd64_linux_nat): Install amd64_linux_new_fork as
	linux_nat_new_fork hook, and i386_forget_process as
	linux_nat_forget_process hook.
	* i386-linux-nat.c (update_debug_registers_callback):
	Update comment.
	(amd64_linux_dr_set_control, amd64_linux_dr_set_addr): Use
	iterate_over_lwps.
	(i386_linux_prepare_to_resume): Pass the lwp's pid to
	i386_debug_reg_state.
	(i386_linux_new_fork): New function.
	(_initialize_i386_linux_nat): Install i386_linux_new_fork as
	linux_nat_new_fork hook, and i386_forget_process as
	linux_nat_forget_process hook.
	* i386-nat.c (i386_init_dregs): Delete.
	(i386_inferior_data, struct i386_inferior_data):
	Delete.
	(struct i386_process_info): New.
	(i386_process_list): New global.
	(i386_find_process_pid, i386_add_process, i386_process_info_get):
	New functions.
	(i386_inferior_data_get): Delete.
	(i386_process_info_get): New function.
	(i386_debug_reg_state): New parameter 'pid'.  Reimplement.
	(i386_forget_process): New function.
	(i386_cleanup_dregs): Rewrite.
	(i386_update_inferior_debug_regs, i386_insert_watchpoint)
	(i386_remove_watchpoint, i386_region_ok_for_watchpoint)
	(i386_stopped_data_address, i386_insert_hw_breakpoint)
	(i386_remove_hw_breakpoint): Adjust to pass the current process id
	to i386_debug_reg_state.
	(i386_use_watchpoints): Don't register inferior data.
	* i386-nat.h (i386_debug_reg_state): Add new 'pid' parameter, and
	adjust comment.
	(i386_forget_process): Declare.
	* linux-fork.c (delete_fork): Call linux_nat_forget_process.
	* linux-nat.c (linux_nat_new_fork, linux_nat_forget_process_hook):
	New static globals.
	(linux_child_follow_fork): Don't call linux_nat_new_thread here.
	(add_initial_lwp): New, factored out from ...
	(add_lwp): ... this.  Don't check the number of lwps before
	calling linux_nat_new_thread.
	(linux_nat_iterate_watchpoint_lwps): Delete.
	(linux_nat_attach): Use add_initial_lwp instead of add_lwp.
	(linux_handle_extended_wait): Call the linux_nat_new_fork hook on
	forks and vforks.
	(linux_nat_wait_1): Use add_initial_lwp instead of add_lwp for the
	initial lwp.
	(linux_nat_kill, linux_nat_mourn_inferior): Call
	linux_nat_forget_process.
	(linux_nat_set_new_fork, linux_nat_set_forget_process)
	(linux_nat_forget_process): New functions.
	* linux-nat.h (linux_nat_iterate_watchpoint_lwps_ftype): Delete
	type.
	(linux_nat_iterate_watchpoint_lwps): Delete declaration.
	(linux_nat_new_fork_ftype, linux_nat_forget_process_ftype): New
	types.
	(linux_nat_set_new_fork, linux_nat_set_forget_process)
	(linux_nat_forget_process): New declarations.
	
	* amd64fbsd-nat.c (super_mourn_inferior): New global.
	(amd64fbsd_mourn_inferior): New function.
	(_initialize_amd64fbsd_nat): Override to_mourn_inferior.
	* windows-nat.c (windows_detach): Call i386_cleanup_dregs.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15142&r2=1.15143
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/amd64-linux-nat.c.diff?cvsroot=src&r1=1.49&r2=1.50
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/amd64fbsd-nat.c.diff?cvsroot=src&r1=1.30&r2=1.31
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/i386-linux-nat.c.diff?cvsroot=src&r1=1.114&r2=1.115
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/i386-nat.c.diff?cvsroot=src&r1=1.44&r2=1.45
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/i386-nat.h.diff?cvsroot=src&r1=1.9&r2=1.10
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/linux-fork.c.diff?cvsroot=src&r1=1.42&r2=1.43
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/linux-nat.c.diff?cvsroot=src&r1=1.263&r2=1.264
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/linux-nat.h.diff?cvsroot=src&r1=1.49&r2=1.50
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/windows-nat.c.diff?cvsroot=src&r1=1.238&r2=1.239