[patch] Fix for newer kernels with: t (tracing stop)

[patch] Fix for newer kernels with: t (tracing stop)

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

Hi,

I did provide wrong ptrace data which should fail on their write.
	error (_("Unexpected error setting hardware debug registers"));
But GDB did not print that error, only inferior did hang, because the data was
not written.

It is because this error/exception gets suppressed by:
linux_resume_one_lwp():
1578          if (!check_ptrace_stopped_lwp_gone (lp))
1579            throw_exception (ex);

Which happens because check_ptrace_stopped_lwp_gone()
expects 'T (tracing stop)' while recent Linux kernels
provide 't (tracing stop)' instad.
	What does lowercase t means in ps state code
	http://stackoverflow.com/questions/35895886/what-does-lowercase-t-means-in-ps-state-code

Found it on:
	kernel-4.4.6-301.fc23.aarch64
by:
	gdb/nat/aarch64-linux-hw-point.c
	-  ctrl |= ((1 << len) - 1) << 5;
	+  ctrl |= (((1 << len) - 1)&~1) << 5;

It does not change testsuite results on that F-23.aarch64 machine.
I see no real regessions on rawhide.x86_64 machine (with F-23 kernel) although
there were some fuzzy results I will need to check more.

OK for check-in?


Thanks,
Jan

[-- Attachment #2: tracestop.patch --]
[-- Type: text/plain, Size: 2465 bytes --]

gdb/ChangeLog
2016-06-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* nat/linux-procfs.c (linux_proc_pid_has_state): Add parameter state2.
	(linux_proc_pid_is_stopped): Update caller.
	(linux_proc_pid_is_trace_stopped_nowarn): Add 't (tracing stop)'.
	(linux_proc_pid_is_zombie_maybe_warn): Update caller.

--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -129,17 +129,20 @@ linux_proc_pid_is_gone (pid_t pid)
     }
 }
 
-/* Return non-zero if 'State' of /proc/PID/status contains STATE.  If
-   WARN, warn on failure to open the /proc file.  */
+/* Return non-zero if 'State' of /proc/PID/status contains STATE or STATE2.
+   STATE2 can be NULL.  If WARN, warn on failure to open the /proc file.  */
 
 static int
-linux_proc_pid_has_state (pid_t pid, const char *state, int warn)
+linux_proc_pid_has_state (pid_t pid, const char *state, const char *state2,
+			  int warn)
 {
   char buffer[100];
   int have_state;
 
   have_state = linux_proc_pid_get_state (pid, buffer, sizeof buffer, warn);
-  return (have_state > 0 && strstr (buffer, state) != NULL);
+  return (have_state > 0
+	  && (strstr (buffer, state) != NULL
+	      || (state2 != NULL && strstr (buffer, state2) != NULL)));
 }
 
 /* Detect `T (stopped)' in `/proc/PID/status'.
@@ -148,16 +151,18 @@ linux_proc_pid_has_state (pid_t pid, const char *state, int warn)
 int
 linux_proc_pid_is_stopped (pid_t pid)
 {
-  return linux_proc_pid_has_state (pid, "T (stopped)", 1);
+  return linux_proc_pid_has_state (pid, "T (stopped)", NULL, 1);
 }
 
-/* Detect `T (tracing stop)' in `/proc/PID/status'.
-   Other states including `T (stopped)' are reported as false.  */
+/* Detect `t (tracing stop)'  or `T (tracing stop)' (present in older
+   kernels) in `/proc/PID/status'.  Other states including `T (stopped)'
+   are reported as false.  */
 
 int
 linux_proc_pid_is_trace_stopped_nowarn (pid_t pid)
 {
-  return linux_proc_pid_has_state (pid, "T (tracing stop)", 1);
+  return linux_proc_pid_has_state (pid, "t (tracing stop)", "T (tracing stop)",
+                                   1);
 }
 
 /* Return non-zero if PID is a zombie.  If WARN, warn on failure to
@@ -166,7 +171,7 @@ linux_proc_pid_is_trace_stopped_nowarn (pid_t pid)
 static int
 linux_proc_pid_is_zombie_maybe_warn (pid_t pid, int warn)
 {
-  return linux_proc_pid_has_state (pid, "Z (zombie)", warn);
+  return linux_proc_pid_has_state (pid, "Z (zombie)", NULL, warn);
 }
 
 /* See linux-procfs.h declaration.  */

Re: [patch] Fix for newer kernels with: t (tracing stop)

[Pedro Alves]

On 06/04/2016 01:29 PM, Jan Kratochvil wrote:
> Hi,
> 
> I did provide wrong ptrace data which should fail on their write.
> 	error (_("Unexpected error setting hardware debug registers"));
> But GDB did not print that error, only inferior did hang, because the data was
> not written.
> 
> It is because this error/exception gets suppressed by:
> linux_resume_one_lwp():
> 1578          if (!check_ptrace_stopped_lwp_gone (lp))
> 1579            throw_exception (ex);
> 
> Which happens because check_ptrace_stopped_lwp_gone()
> expects 'T (tracing stop)' while recent Linux kernels
> provide 't (tracing stop)' instad.
> 	What does lowercase t means in ps state code
> 	http://stackoverflow.com/questions/35895886/what-does-lowercase-t-means-in-ps-state-code

Eh, I'm not sure how I ended up with "T (tracing stop)"
in the first place last year, as I think I was on Fedora 20,
and lowercase "t (tracing stop)" is around since 2009.

> 
> Found it on:
> 	kernel-4.4.6-301.fc23.aarch64
> by:
> 	gdb/nat/aarch64-linux-hw-point.c
> 	-  ctrl |= ((1 << len) - 1) << 5;
> 	+  ctrl |= (((1 << len) - 1)&~1) << 5;
> 
> It does not change testsuite results on that F-23.aarch64 machine.
> I see no real regessions on rawhide.x86_64 machine (with F-23 kernel) although
> there were some fuzzy results I will need to check more.
> 
> OK for check-in?

OK.

I wonder whether it wouldn't simplify things to parse the
state into some new enum lwp_state instead of the current scheme
of passing state strings around.  I may give that a try as follow up.

Thanks,
Pedro Alves

Re: [patch] Fix for newer kernels with: t (tracing stop)

[Pedro Alves]

On 07/22/2016 04:35 PM, Pedro Alves wrote:

> OK.
> 
> I wonder whether it wouldn't simplify things to parse the
> state into some new enum lwp_state instead of the current scheme
> of passing state strings around.  I may give that a try as follow up.
> 

Like this.  This one's against current master, however.  Turned out to
be easy/quick to try.  Untested though.  I'll rebase if you want to put
yours in first.

From 9be7dff19a73f6d64dcbb3d9cd7d965ca48c3ed6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 22 Jul 2016 16:10:00 +0100
Subject: [PATCH] Introduce enum proc_state

---
 gdb/nat/linux-procfs.c | 86 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 20 deletions(-)

diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index e2ea1d6..9dd3fd7 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -70,19 +70,67 @@ linux_proc_get_tracerpid_nowarn (pid_t lwpid)
   return linux_proc_get_int (lwpid, "TracerPid", 0);
 }
 
-/* Fill in BUFFER, a buffer with BUFFER_SIZE bytes with the 'State'
+/* Process states as discovered in the 'State' line of
+   /proc/PID/status.  Not all possible states are represented here,
+   only those that we care about.  */
+
+enum proc_state
+{
+  /* Some state we don't handle.  */
+  PROC_STATE_UNKNOWN,
+
+  /* Stopped on a signal.  */
+  PROC_STATE_STOPPED,
+
+  /* Tracing stop.  */
+  PROC_STATE_TRACING_STOP,
+
+  /* Dead.  */
+  PROC_STATE_DEAD,
+
+  /* Zombie.  */
+  PROC_STATE_ZOMBIE,
+};
+
+/* Parse an PROC_STATE out of STATE, a buffer with the 'State' line of
+   /proc/PID/status.  */
+
+static enum proc_state
+parse_proc_status_state (const char *state)
+{
+  switch (state[0])
+    {
+    case 't':
+      return PROC_STATE_TRACING_STOP;
+    case 'T':
+      /* Lowercase 't' was introduced in Linux 2.6.33.  */
+      if (strcmp (state, "T (tracing stop)") == 0)
+	return PROC_STATE_TRACING_STOP;
+      else
+	return PROC_STATE_STOPPED;
+    case 'X':
+      return PROC_STATE_DEAD;
+    case 'Z':
+      return PROC_STATE_ZOMBIE;
+    }
+
+  return PROC_STATE_UNKNOWN;
+}
+
+
+/* Fill in STATE, a buffer with BUFFER_SIZE bytes with the 'State'
    line of /proc/PID/status.  Returns -1 on failure to open the /proc
    file, 1 if the line is found, and 0 if not found.  If WARN, warn on
    failure to open the /proc file.  */
 
 static int
-linux_proc_pid_get_state (pid_t pid, char *buffer, size_t buffer_size,
-			  int warn)
+linux_proc_pid_get_state (pid_t pid, int warn, enum proc_state *state)
 {
   FILE *procfile;
   int have_state;
+  char buffer[100];
 
-  xsnprintf (buffer, buffer_size, "/proc/%d/status", (int) pid);
+  xsnprintf (buffer, sizeof buffer, "/proc/%d/status", (int) pid);
   procfile = gdb_fopen_cloexec (buffer, "r");
   if (procfile == NULL)
     {
@@ -92,10 +140,11 @@ linux_proc_pid_get_state (pid_t pid, char *buffer, size_t buffer_size,
     }
 
   have_state = 0;
-  while (fgets (buffer, buffer_size, procfile) != NULL)
+  while (fgets (buffer, sizeof (buffer), procfile) != NULL)
     if (startswith (buffer, "State:"))
       {
 	have_state = 1;
+	*state = parse_proc_status_state (buffer + sizeof ("State:") - 1);
 	break;
       }
   fclose (procfile);
@@ -107,10 +156,10 @@ linux_proc_pid_get_state (pid_t pid, char *buffer, size_t buffer_size,
 int
 linux_proc_pid_is_gone (pid_t pid)
 {
-  char buffer[100];
   int have_state;
+  enum proc_state state;
 
-  have_state = linux_proc_pid_get_state (pid, buffer, sizeof buffer, 0);
+  have_state = linux_proc_pid_get_state (pid, 0, &state);
   if (have_state < 0)
     {
       /* If we can't open the status file, assume the thread has
@@ -123,41 +172,38 @@ linux_proc_pid_is_gone (pid_t pid)
       return 0;
     }
   else
-    {
-      return (strstr (buffer, "Z (") != NULL
-	      || strstr (buffer, "X (") != NULL);
-    }
+    return (state == PROC_STATE_ZOMBIE || state == PROC_STATE_DEAD);
 }
 
 /* Return non-zero if 'State' of /proc/PID/status contains STATE.  If
    WARN, warn on failure to open the /proc file.  */
 
 static int
-linux_proc_pid_has_state (pid_t pid, const char *state, int warn)
+linux_proc_pid_has_state (pid_t pid, enum proc_state state, int warn)
 {
-  char buffer[100];
   int have_state;
+  enum proc_state cur_state;
 
-  have_state = linux_proc_pid_get_state (pid, buffer, sizeof buffer, warn);
-  return (have_state > 0 && strstr (buffer, state) != NULL);
+  have_state = linux_proc_pid_get_state (pid, warn, &cur_state);
+  return (have_state > 0 && cur_state == state);
 }
 
 /* Detect `T (stopped)' in `/proc/PID/status'.
-   Other states including `T (tracing stop)' are reported as false.  */
+   Other states including `t (tracing stop)' are reported as false.  */
 
 int
 linux_proc_pid_is_stopped (pid_t pid)
 {
-  return linux_proc_pid_has_state (pid, "T (stopped)", 1);
+  return linux_proc_pid_has_state (pid, PROC_STATE_STOPPED, 1);
 }
 
-/* Detect `T (tracing stop)' in `/proc/PID/status'.
+/* Detect `t (tracing stop)' in `/proc/PID/status'.
    Other states including `T (stopped)' are reported as false.  */
 
 int
 linux_proc_pid_is_trace_stopped_nowarn (pid_t pid)
 {
-  return linux_proc_pid_has_state (pid, "T (tracing stop)", 1);
+  return linux_proc_pid_has_state (pid, PROC_STATE_TRACING_STOP, 1);
 }
 
 /* Return non-zero if PID is a zombie.  If WARN, warn on failure to
@@ -166,7 +212,7 @@ linux_proc_pid_is_trace_stopped_nowarn (pid_t pid)
 static int
 linux_proc_pid_is_zombie_maybe_warn (pid_t pid, int warn)
 {
-  return linux_proc_pid_has_state (pid, "Z (zombie)", warn);
+  return linux_proc_pid_has_state (pid, PROC_STATE_ZOMBIE, warn);
 }
 
 /* See linux-procfs.h declaration.  */
-- 
2.5.5

Re: [patch] Fix for newer kernels with: t (tracing stop)

[Jan Kratochvil]

On Fri, 22 Jul 2016 18:04:04 +0200, Pedro Alves wrote:
> Like this.  This one's against current master, however.  Turned out to
> be easy/quick to try.  Untested though.  I'll rebase if you want to put
> yours in first.

Such refactorization is sure better.


Thanks,
Jan

[pushed 1/2] linux-procfs: Introduce enum proc_state

[Pedro Alves]

Parse the process's /proc/PID/status state into an enum instead of the
current scheme of passing state strings around.

gdb/ChangeLog:
2016-07-25  Pedro Alves  <palves@redhat.com>

	* nat/linux-procfs.c (enum proc_state): New enum.
	(parse_proc_status_state): New function.
	(linux_proc_pid_get_state): Replace output string buffer parameter
	with an output proc_state parameter.  Use parse_proc_status_state.
	(linux_proc_pid_is_gone): Adjust to use proc_state values.
	(linux_proc_pid_has_state): Change type of 'state' parameter; now
	an enum proc_state.  Adjust to linux_proc_pid_get_state interface
	change.
	(linux_proc_pid_is_stopped)
	(linux_proc_pid_is_trace_stopped_nowarn)
	(linux_proc_pid_is_zombie_maybe_warn): Adjust to
	linux_proc_pid_get_state interface change.
---
 gdb/ChangeLog          | 15 +++++++++
 gdb/nat/linux-procfs.c | 83 ++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 79 insertions(+), 19 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index db910a3..0840a69 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2016-07-25  Pedro Alves  <palves@redhat.com>
+
+	* nat/linux-procfs.c (enum proc_state): New enum.
+	(parse_proc_status_state): New function.
+	(linux_proc_pid_get_state): Replace output string buffer parameter
+	with an output proc_state parameter.  Use parse_proc_status_state.
+	(linux_proc_pid_is_gone): Adjust to use proc_state values.
+	(linux_proc_pid_has_state): Change type of 'state' parameter; now
+	an enum proc_state.  Adjust to linux_proc_pid_get_state interface
+	change.
+	(linux_proc_pid_is_stopped)
+	(linux_proc_pid_is_trace_stopped_nowarn)
+	(linux_proc_pid_is_zombie_maybe_warn): Adjust to
+	linux_proc_pid_get_state interface change.
+
 2016-07-25  Tim Wiederhake  <tim.wiederhake@intel.com>
 
 	* MAINTAINERS (Write After Approval): Add Tim Wiederhake
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index e2ea1d6..5d63e1b 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -70,19 +70,66 @@ linux_proc_get_tracerpid_nowarn (pid_t lwpid)
   return linux_proc_get_int (lwpid, "TracerPid", 0);
 }
 
-/* Fill in BUFFER, a buffer with BUFFER_SIZE bytes with the 'State'
+/* Process states as discovered in the 'State' line of
+   /proc/PID/status.  Not all possible states are represented here,
+   only those that we care about.  */
+
+enum proc_state
+{
+  /* Some state we don't handle.  */
+  PROC_STATE_UNKNOWN,
+
+  /* Stopped on a signal.  */
+  PROC_STATE_STOPPED,
+
+  /* Tracing stop.  */
+  PROC_STATE_TRACING_STOP,
+
+  /* Dead.  */
+  PROC_STATE_DEAD,
+
+  /* Zombie.  */
+  PROC_STATE_ZOMBIE,
+};
+
+/* Parse a PROC_STATE out of STATE, a buffer with the state found in
+   the 'State:' line of /proc/PID/status.  */
+
+static enum proc_state
+parse_proc_status_state (const char *state)
+{
+  state = skip_spaces_const (state);
+
+  switch (state[0])
+    {
+    case 'T':
+      if (strcmp (state, "T (tracing stop)") == 0)
+	return PROC_STATE_TRACING_STOP;
+      else
+	return PROC_STATE_STOPPED;
+    case 'X':
+      return PROC_STATE_DEAD;
+    case 'Z':
+      return PROC_STATE_ZOMBIE;
+    }
+
+  return PROC_STATE_UNKNOWN;
+}
+
+
+/* Fill in STATE, a buffer with BUFFER_SIZE bytes with the 'State'
    line of /proc/PID/status.  Returns -1 on failure to open the /proc
    file, 1 if the line is found, and 0 if not found.  If WARN, warn on
    failure to open the /proc file.  */
 
 static int
-linux_proc_pid_get_state (pid_t pid, char *buffer, size_t buffer_size,
-			  int warn)
+linux_proc_pid_get_state (pid_t pid, int warn, enum proc_state *state)
 {
   FILE *procfile;
   int have_state;
+  char buffer[100];
 
-  xsnprintf (buffer, buffer_size, "/proc/%d/status", (int) pid);
+  xsnprintf (buffer, sizeof (buffer), "/proc/%d/status", (int) pid);
   procfile = gdb_fopen_cloexec (buffer, "r");
   if (procfile == NULL)
     {
@@ -92,10 +139,11 @@ linux_proc_pid_get_state (pid_t pid, char *buffer, size_t buffer_size,
     }
 
   have_state = 0;
-  while (fgets (buffer, buffer_size, procfile) != NULL)
+  while (fgets (buffer, sizeof (buffer), procfile) != NULL)
     if (startswith (buffer, "State:"))
       {
 	have_state = 1;
+	*state = parse_proc_status_state (buffer + sizeof ("State:") - 1);
 	break;
       }
   fclose (procfile);
@@ -107,10 +155,10 @@ linux_proc_pid_get_state (pid_t pid, char *buffer, size_t buffer_size,
 int
 linux_proc_pid_is_gone (pid_t pid)
 {
-  char buffer[100];
   int have_state;
+  enum proc_state state;
 
-  have_state = linux_proc_pid_get_state (pid, buffer, sizeof buffer, 0);
+  have_state = linux_proc_pid_get_state (pid, 0, &state);
   if (have_state < 0)
     {
       /* If we can't open the status file, assume the thread has
@@ -123,23 +171,20 @@ linux_proc_pid_is_gone (pid_t pid)
       return 0;
     }
   else
-    {
-      return (strstr (buffer, "Z (") != NULL
-	      || strstr (buffer, "X (") != NULL);
-    }
+    return (state == PROC_STATE_ZOMBIE || state == PROC_STATE_DEAD);
 }
 
 /* Return non-zero if 'State' of /proc/PID/status contains STATE.  If
    WARN, warn on failure to open the /proc file.  */
 
 static int
-linux_proc_pid_has_state (pid_t pid, const char *state, int warn)
+linux_proc_pid_has_state (pid_t pid, enum proc_state state, int warn)
 {
-  char buffer[100];
   int have_state;
+  enum proc_state cur_state;
 
-  have_state = linux_proc_pid_get_state (pid, buffer, sizeof buffer, warn);
-  return (have_state > 0 && strstr (buffer, state) != NULL);
+  have_state = linux_proc_pid_get_state (pid, warn, &cur_state);
+  return (have_state > 0 && cur_state == state);
 }
 
 /* Detect `T (stopped)' in `/proc/PID/status'.
@@ -148,16 +193,16 @@ linux_proc_pid_has_state (pid_t pid, const char *state, int warn)
 int
 linux_proc_pid_is_stopped (pid_t pid)
 {
-  return linux_proc_pid_has_state (pid, "T (stopped)", 1);
+  return linux_proc_pid_has_state (pid, PROC_STATE_STOPPED, 1);
 }
 
-/* Detect `T (tracing stop)' in `/proc/PID/status'.
+/* Detect `t (tracing stop)' in `/proc/PID/status'.
    Other states including `T (stopped)' are reported as false.  */
 
 int
 linux_proc_pid_is_trace_stopped_nowarn (pid_t pid)
 {
-  return linux_proc_pid_has_state (pid, "T (tracing stop)", 1);
+  return linux_proc_pid_has_state (pid, PROC_STATE_TRACING_STOP, 1);
 }
 
 /* Return non-zero if PID is a zombie.  If WARN, warn on failure to
@@ -166,7 +211,7 @@ linux_proc_pid_is_trace_stopped_nowarn (pid_t pid)
 static int
 linux_proc_pid_is_zombie_maybe_warn (pid_t pid, int warn)
 {
-  return linux_proc_pid_has_state (pid, "Z (zombie)", warn);
+  return linux_proc_pid_has_state (pid, PROC_STATE_ZOMBIE, warn);
 }
 
 /* See linux-procfs.h declaration.  */
-- 
2.5.5

[pushed 2/2] linux-procfs: Handle lowercase "t (tracing stop)" state

[Pedro Alves]

Since Linux 2.6.33, /proc/PID/status shows "t (tracing stop)", with
lowercase 't'.  Because GDB is only expecting "T (tracing stop)", GDB
can incorrectly suppress errors in check_ptrace_stopped_lwp_gone:

 1578          if (!check_ptrace_stopped_lwp_gone (lp))
 1579            throw_exception (ex);

Ref: https://sourceware.org/ml/gdb-patches/2016-06/msg00072.html

2016-07-25  Pedro Alves  <palves@redhat.com>
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	* nat/linux-procfs.c (parse_proc_status_state): Handle lowercase
	't'.
---
 gdb/ChangeLog          | 6 ++++++
 gdb/nat/linux-procfs.c | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0840a69..c346a90 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,10 @@
 2016-07-25  Pedro Alves  <palves@redhat.com>
+	    Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* nat/linux-procfs.c (parse_proc_status_state): Handle lowercase
+	't'.
+
+2016-07-25  Pedro Alves  <palves@redhat.com>
 
 	* nat/linux-procfs.c (enum proc_state): New enum.
 	(parse_proc_status_state): New function.
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 5d63e1b..f00fe29 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -102,7 +102,10 @@ parse_proc_status_state (const char *state)
 
   switch (state[0])
     {
+    case 't':
+      return PROC_STATE_TRACING_STOP;
     case 'T':
+      /* Before Linux 2.6.33, tracing stop used uppercase T.  */
       if (strcmp (state, "T (tracing stop)") == 0)
 	return PROC_STATE_TRACING_STOP;
       else
-- 
2.5.5