[PATCH] S390: Defer PER info update until resume

[PATCH] S390: Defer PER info update until resume

[Andreas Arnez]

For multi-threaded inferiors on S390 GNU/Linux targets, GDB tried to
update the PER info via ptrace() in a newly attached thread before
assuring that the thread is stopped.  Depending on the timing, this
could lead to a GDB internal error.  The patch defers the PER info
update until just before resuming the thread.

gdb/ChangeLog:

	* s390-linux-nat.c (struct arch_lwp_info): New.
	(s390_fix_watch_points): Rename to...
	(s390_prepare_to_resume): ...this.  Skip the PER info update
	unless the watch points have changed.
	(s390_refresh_per_info, s390_new_thread): New functions.
	(s390_insert_watchpoint): Call s390_refresh_per_info instead of
	s390_fix_watch_points.
	(s390_remove_watchpoint): Likewise.
	(_initialize_s390_nat): Reflect renaming of s390_fix_watch_points.
	Register s390_prepare_to_resume.
---
 gdb/s390-linux-nat.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
index 367b610..9298bcc 100644
--- a/gdb/s390-linux-nat.c
+++ b/gdb/s390-linux-nat.c
@@ -46,6 +46,14 @@
 #define PTRACE_SETREGSET 0x4205
 #endif
 
+/* Per-thread arch-specific data.  */
+
+struct arch_lwp_info
+{
+  /* Non-zero if the thread's PER info must be re-written.  */
+  int per_info_changed;
+};
+
 static int have_regset_last_break = 0;
 static int have_regset_system_call = 0;
 static int have_regset_tdb = 0;
@@ -465,8 +473,10 @@ s390_stopped_by_watchpoint (struct target_ops *ops)
   return result;
 }
 
+/* Each time before resuming a thread, update its PER info.  */
+
 static void
-s390_fix_watch_points (struct lwp_info *lp)
+s390_prepare_to_resume (struct lwp_info *lp)
 {
   int tid;
 
@@ -476,6 +486,12 @@ s390_fix_watch_points (struct lwp_info *lp)
   CORE_ADDR watch_lo_addr = (CORE_ADDR)-1, watch_hi_addr = 0;
   struct watch_area *area;
 
+  if (lp->arch_private == NULL
+      || !lp->arch_private->per_info_changed)
+    return;
+
+  lp->arch_private->per_info_changed = 0;
+
   tid = ptid_get_lwp (lp->ptid);
   if (tid == 0)
     tid = ptid_get_pid (lp->ptid);
@@ -509,6 +525,30 @@ s390_fix_watch_points (struct lwp_info *lp)
     perror_with_name (_("Couldn't modify watchpoint status"));
 }
 
+/* Make sure that LP is stopped and mark its PER info as changed, so
+   the next resume will update it.  */
+
+static void
+s390_refresh_per_info (struct lwp_info *lp)
+{
+  if (lp->arch_private == NULL)
+    lp->arch_private = XCNEW (struct arch_lwp_info);
+
+  lp->arch_private->per_info_changed = 1;
+
+  if (!lp->stopped)
+    linux_stop_lwp (lp);
+}
+
+/* When attaching to a new thread, mark its PER info as changed.  */
+
+static void
+s390_new_thread (struct lwp_info *lp)
+{
+  lp->arch_private = XCNEW (struct arch_lwp_info);
+  lp->arch_private->per_info_changed = 1;
+}
+
 static int
 s390_insert_watchpoint (struct target_ops *self,
 			CORE_ADDR addr, int len, int type,
@@ -527,7 +567,7 @@ s390_insert_watchpoint (struct target_ops *self,
   watch_base = area;
 
   ALL_LWPS (lp)
-    s390_fix_watch_points (lp);
+    s390_refresh_per_info (lp);
   return 0;
 }
 
@@ -556,7 +596,7 @@ s390_remove_watchpoint (struct target_ops *self,
   xfree (area);
 
   ALL_LWPS (lp)
-    s390_fix_watch_points (lp);
+    s390_refresh_per_info (lp);
   return 0;
 }
 
@@ -699,5 +739,6 @@ _initialize_s390_nat (void)
 
   /* Register the target.  */
   linux_nat_add_target (t);
-  linux_nat_set_new_thread (t, s390_fix_watch_points);
+  linux_nat_set_new_thread (t, s390_new_thread);
+  linux_nat_set_prepare_to_resume (t, s390_prepare_to_resume);
 }
-- 
1.9.3

Re: [PATCH] S390: Defer PER info update until resume

[Ulrich Weigand]

Andreas Arnez wrote:

> For multi-threaded inferiors on S390 GNU/Linux targets, GDB tried to
> update the PER info via ptrace() in a newly attached thread before
> assuring that the thread is stopped.  Depending on the timing, this
> could lead to a GDB internal error.  The patch defers the PER info
> update until just before resuming the thread.
> 
> gdb/ChangeLog:
> 
> 	* s390-linux-nat.c (struct arch_lwp_info): New.
> 	(s390_fix_watch_points): Rename to...
> 	(s390_prepare_to_resume): ...this.  Skip the PER info update
> 	unless the watch points have changed.
> 	(s390_refresh_per_info, s390_new_thread): New functions.
> 	(s390_insert_watchpoint): Call s390_refresh_per_info instead of
> 	s390_fix_watch_points.
> 	(s390_remove_watchpoint): Likewise.
> 	(_initialize_s390_nat): Reflect renaming of s390_fix_watch_points.
> 	Register s390_prepare_to_resume.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com