[patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Jan Kratochvil]

Hi,

in some cases ia64 can generate SIGILL instead of SIGTRAP.  Guessing it is
a CPU bug instead of Linux kernel bug (but I may be wrong).
	https://bugzilla.redhat.com/show_bug.cgi?id=615538
	http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ia64-sigill.c?cvsroot=systemtap

Anyway FSF GDB already contains code for handling such cases (and not only
SIGILL).  Just the time it gets executed the breakpoint which generated the
signal may be already deleted and GDB stops with:
	Program received signal SIGILL, Illegal instruction.

I do not think it can be solved fully target-independent.  linux-nat.c already
does some postponing of signals using
	kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (lp->status));
and the moment linux-nat.c will read back such postponed signal its breakpoint
may be already deleted (and thus it will never get to target-independent parts
of GDB such as infrun.c).

Moreover I think the check could be more target specific as it could more
precisely check for siginfo_t.si_addr etc.  But there is currently no list of
targets and their SIGany->SIGTRAP conversions list so I would certainly break
compatibility with those arches.

Therefore I kept the arch-independent check in place but extended it also into
linux-nat.c.  BTW this patch works on GNU/Linux fine even with the
handle_inferior_event call completely removed.

Tested only with these patches applied (IMO they do not make a difference):
	[patch] Fix linux-nat.c {,lp->}status typo
	http://sourceware.org/ml/gdb-patches/2010-07/msg00270.html
	[patch] Fix linux-nat.c new_lp dropped status
	http://sourceware.org/ml/gdb-patches/2010-07/msg00272.html
No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
No regressions on ia64-rhel55-linux-gnu.


Thanks,
Jan


gdb/
2010-07-19  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* inferior.h (stop_signal_is_trap): New prototype.
	* infrun.c (handle_inferior_event): Move the `Treating signal as
	SIGTRAP' code ...
	(stop_signal_is_trap): .... into a new function.
	* linux-nat.c
	(linux_nat_post_attach_wait) <WSTOPSIG (status) != SIGSTOP>: New
	variable target_signal.  Call stop_signal_is_trap.
	(linux_handle_extended_wait) <WSTOPSIG (status) != SIGSTOP>: Likewise.
	(wait_lwp): Call stop_signal_is_trap.
	(linux_nat_filter_event): Likewise.

gdb/testsuite/
2010-07-19  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/ia64-sigill.exp: New file.
	* gdb.threads/ia64-sigill.c: New file.

--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -263,6 +263,9 @@ extern void error_is_running (void);
 /* Calls error_is_running if the current thread is running.  */
 extern void ensure_not_running (void);
 
+extern int stop_signal_is_trap (ptid_t ptid,
+				enum target_signal target_signal);
+
 void set_step_info (struct frame_info *frame, struct symtab_and_line sal);
 
 /* From infcmd.c */
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2926,6 +2926,31 @@ handle_syscall_event (struct execution_control_state *ecs)
   return 1;
 }
 
+/* Return 1 for signals caused by the debugger.  Return 0 for signals
+   that have to do with the program's own actions.  Note that breakpoint insns
+   may cause SIGTRAP or SIGILL or SIGEMT, depending on the operating system
+   version.  Here we detect when a SIGILL or SIGEMT is really a breakpoint and
+   change it to SIGTRAP.  We do something similar for SIGSEGV, since a SIGSEGV
+   will be generated when we're trying to execute a breakpoint instruction on
+   a non-executable stack.  This happens for call dummy breakpoints for
+   architectures like SPARC that place call dummies on the stack.  */
+
+int
+stop_signal_is_trap (ptid_t ptid, enum target_signal target_signal)
+{
+  if (target_signal == TARGET_SIGNAL_ILL
+      || target_signal == TARGET_SIGNAL_SEGV
+      || target_signal == TARGET_SIGNAL_EMT)
+    {
+      struct regcache *regcache = get_thread_regcache (ptid);
+
+      if (breakpoint_inserted_here_p (get_regcache_aspace (regcache),
+				      regcache_read_pc (regcache)))
+	return 1;
+    }
+  return 0;
+}
+
 /* Given an execution control state that has been freshly filled in
    by an event from the inferior, figure out what it means and take
    appropriate action.  */
@@ -2996,31 +3021,13 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   breakpoint_retire_moribund ();
 
-  /* First, distinguish signals caused by the debugger from signals
-     that have to do with the program's own actions.  Note that
-     breakpoint insns may cause SIGTRAP or SIGILL or SIGEMT, depending
-     on the operating system version.  Here we detect when a SIGILL or
-     SIGEMT is really a breakpoint and change it to SIGTRAP.  We do
-     something similar for SIGSEGV, since a SIGSEGV will be generated
-     when we're trying to execute a breakpoint instruction on a
-     non-executable stack.  This happens for call dummy breakpoints
-     for architectures like SPARC that place call dummies on the
-     stack.  */
   if (ecs->ws.kind == TARGET_WAITKIND_STOPPED
-      && (ecs->ws.value.sig == TARGET_SIGNAL_ILL
-	  || ecs->ws.value.sig == TARGET_SIGNAL_SEGV
-	  || ecs->ws.value.sig == TARGET_SIGNAL_EMT))
+      && stop_signal_is_trap (ecs->ptid, ecs->ws.value.sig))
     {
-      struct regcache *regcache = get_thread_regcache (ecs->ptid);
-
-      if (breakpoint_inserted_here_p (get_regcache_aspace (regcache),
-				      regcache_read_pc (regcache)))
-	{
-	  if (debug_infrun)
-	    fprintf_unfiltered (gdb_stdlog,
-				"infrun: Treating signal as SIGTRAP\n");
-	  ecs->ws.value.sig = TARGET_SIGNAL_TRAP;
-	}
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: Treating signal as SIGTRAP\n");
+      ecs->ws.value.sig = TARGET_SIGNAL_TRAP;
     }
 
   /* Mark the non-executing threads accordingly.  In all-stop, all
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1410,11 +1410,22 @@ linux_nat_post_attach_wait (ptid_t ptid, int first, int *cloned,
 
   if (WSTOPSIG (status) != SIGSTOP)
     {
+      enum target_signal target_signal;
+	  
       *signalled = 1;
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
 			    "LNPAW: Received %s after attaching\n",
 			    status_to_str (status));
+
+      target_signal = target_signal_from_host (WSTOPSIG (status));
+      if (stop_signal_is_trap (ptid, target_signal))
+	{
+	  if (debug_linux_nat)
+	    fprintf_unfiltered (gdb_stdlog,
+				"LNPAW: Treating signal as SIGTRAP\n");
+	  status = W_STOPCODE (SIGTRAP);
+	}
     }
 
   return status;
@@ -2222,6 +2233,8 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
 
 	  if (WSTOPSIG (status) != SIGSTOP)
 	    {
+	      enum target_signal target_signal;
+
 	      /* This can happen if someone starts sending signals to
 		 the new thread before it gets a chance to run, which
 		 have a lower number than SIGSTOP (e.g. SIGUSR1).
@@ -2231,6 +2244,15 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
 		 the other signal on to the thread below.  */
 
 	      new_lp->signalled = 1;
+
+	      target_signal = target_signal_from_host (WSTOPSIG (status));
+	      if (stop_signal_is_trap (new_lp->ptid, target_signal))
+		{
+		  if (debug_linux_nat)
+		    fprintf_unfiltered (gdb_stdlog,
+					"LHEW: Treating signal as SIGTRAP\n");
+		  status = W_STOPCODE (SIGTRAP);
+		}
 	    }
 	  else
 	    status = 0;
@@ -2433,6 +2455,15 @@ wait_lwp (struct lwp_info *lp)
 	return wait_lwp (lp);
     }
 
+  if (WIFSTOPPED (status)
+      && stop_signal_is_trap (lp->ptid,
+			      target_signal_from_host (WSTOPSIG (status))))
+    {
+      if (debug_linux_nat)
+	fprintf_unfiltered (gdb_stdlog, "WL: Treating signal as SIGTRAP\n");
+      status = W_STOPCODE (SIGTRAP);
+    }
+
   return status;
 }
 
@@ -3065,6 +3096,16 @@ linux_nat_filter_event (int lwpid, int status, int options)
 	return NULL;
     }
 
+  if (WIFSTOPPED (status)
+      && stop_signal_is_trap (lp->ptid,
+			      target_signal_from_host (WSTOPSIG (status))))
+    {
+      if (debug_linux_nat)
+	fprintf_unfiltered (gdb_stdlog,
+			    "LLW: Treating signal as SIGTRAP\n");
+      status = W_STOPCODE (SIGTRAP);
+    }
+
   if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
     {
       /* Save the trap's siginfo in case we need it later.  */
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.c
@@ -0,0 +1,360 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <stdio.h>
+#include <limits.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <unistd.h>
+#include <asm/unistd.h>
+
+#define gettid() syscall (__NR_gettid)
+
+/* Terminate always in the main task, it can lock up with SIGSTOPped GDB
+   otherwise.  */
+#define TIMEOUT (gettid () == getpid() ? 10 : 15)
+
+static pid_t thread1_tid;
+static pthread_cond_t thread1_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread1_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pid_t thread2_tid;
+static pthread_cond_t thread2_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread2_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pthread_mutex_t terminate_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+/* Do not use alarm as it would create a ptrace event which would hang up us if
+   we are being traced by GDB which we stopped ourselves.  */
+
+static void timed_mutex_lock (pthread_mutex_t *mutex)
+{
+  int i;
+  struct timespec start, now;
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      i = pthread_mutex_trylock (mutex);
+      if (i == 0)
+	return;
+      assert (i == EBUSY);
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for internal lock!\n");
+  exit (EXIT_FAILURE);
+}
+
+static void *
+thread_func (void *threadno_voidp)
+{
+  int threadno = (intptr_t) threadno_voidp;
+  int i;
+
+  switch (threadno)
+  {
+    case 1:
+      timed_mutex_lock (&thread1_tid_mutex);
+
+      /* THREAD1_TID_MUTEX must be already locked to avoid race.  */
+      thread1_tid = gettid ();
+
+      i = pthread_cond_signal (&thread1_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread1_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    case 2:
+      timed_mutex_lock (&thread2_tid_mutex);
+
+      /* THREAD2_TID_MUTEX must be already locked to avoid race.  */
+      thread2_tid = gettid ();
+
+      i = pthread_cond_signal (&thread2_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread2_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    default:
+      assert (0);
+  }
+
+#ifdef __ia64__
+  asm volatile ("label:\n"
+		"nop.m 0\n"
+		"nop.i 0\n"
+		"nop.b 0\n");
+#endif
+  /* break-here */
+
+  /* Be sure the "t (tracing stop)" test can proceed for both threads.  */
+  timed_mutex_lock (&terminate_mutex);
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  return NULL;
+}
+
+static const char *
+proc_string (const char *filename, const char *line)
+{
+  FILE *f;
+  static char buf[LINE_MAX];
+  size_t line_len = strlen (line);
+
+  f = fopen (filename, "r");
+  if (f == NULL)
+    {
+      fprintf (stderr, "fopen (\"%s\") for \"%s\": %s\n", filename, line,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  while (errno = 0, fgets (buf, sizeof (buf), f))
+    {
+      char *s;
+
+      s = strchr (buf, '\n');
+      assert (s != NULL);
+      *s = 0;
+
+      if (strncmp (buf, line, line_len) != 0)
+	continue;
+
+      if (fclose (f))
+	{
+	  fprintf (stderr, "fclose (\"%s\") for \"%s\": %s\n", filename, line,
+		   strerror (errno));
+	  exit (EXIT_FAILURE);
+	}
+
+      return &buf[line_len];
+    }
+  if (errno != 0)
+    {
+      fprintf (stderr, "fgets (\"%s\": %s\n", filename, strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  fprintf (stderr, "\"%s\": No line \"%s\" found.\n", filename, line);
+  exit (EXIT_FAILURE);
+}
+
+static unsigned long
+proc_ulong (const char *filename, const char *line)
+{
+  const char *s = proc_string (filename, line);
+  long retval;
+  char *end;
+
+  errno = 0;
+  retval = strtol (s, &end, 10);
+  if (retval < 0 || retval >= LONG_MAX || (end && *end))
+    {
+      fprintf (stderr, "\"%s\":\"%s\": %ld, %s\n", filename, line, retval,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  return retval;
+}
+
+static void
+state_wait (pid_t process, const char *wanted)
+{
+  char *filename;
+  int i;
+  struct timespec start, now;
+  const char *state;
+
+  i = asprintf (&filename, "/proc/%lu/status", (unsigned long) process);
+  assert (i > 0);
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      state = proc_string (filename, "State:\t");
+
+      /* torvalds/linux-2.6.git 464763cf1c6df632dccc8f2f4c7e50163154a2c0
+	 has changed "T (tracing stop)" to "t (tracing stop)".  Make the GDB
+	 testcase backward compatible with older Linux kernels.  */
+      if (strcmp (state, "T (tracing stop)") == 0)
+	state = "t (tracing stop)";
+
+      if (strcmp (state, wanted) == 0)
+	{
+	  free (filename);
+	  return;
+	}
+
+      if (sched_yield ())
+	{
+	  perror ("sched_yield()");
+	  exit (EXIT_FAILURE);
+	}
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for PID %lu \"%s\" (now it is \"%s\")!\n",
+	   (unsigned long) process, wanted, state);
+  exit (EXIT_FAILURE);
+}
+
+static volatile pid_t tracer = 0;
+static pthread_t thread1, thread2;
+
+static void
+cleanup (void)
+{
+  printf ("Resuming GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      int i;
+      int tracer_save = tracer;
+
+      tracer = 0;
+
+      i = kill (tracer_save, SIGCONT);
+      assert (i == 0);
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  int i;
+  int standalone = 0;
+
+  if (argc == 2 && strcmp (argv[1], "-s") == 0)
+    standalone = 1;
+  else
+    assert (argc == 1);
+
+  setbuf (stdout, NULL);
+
+  timed_mutex_lock (&thread1_tid_mutex);
+  timed_mutex_lock (&thread2_tid_mutex);
+
+  timed_mutex_lock (&terminate_mutex);
+
+  i = pthread_create (&thread1, NULL, thread_func, (void *) (intptr_t) 1);
+  assert (i == 0);
+
+  i = pthread_create (&thread2, NULL, thread_func, (void *) (intptr_t) 2);
+  assert (i == 0);
+
+  if (!standalone)
+    {
+      tracer = proc_ulong ("/proc/self/status", "TracerPid:\t");
+      if (tracer == 0)
+	{
+	  fprintf (stderr, "The testcase must be run by GDB!\n");
+	  exit (EXIT_FAILURE);
+	}
+      if (tracer != getppid ())
+	{
+	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
+	  exit (EXIT_FAILURE);
+	}
+    }
+
+  /* SIGCONT our debugger in the case of our crash as we would deadlock
+     otherwise.  */
+
+  atexit (cleanup);
+
+  printf ("Stopping GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      i = kill (tracer, SIGSTOP);
+      assert (i == 0);
+      state_wait (tracer, "T (stopped)");
+    }
+
+  /* Threads are now waiting at timed_mutex_lock (thread1_tid_mutex) and so
+     they could not trigger the breakpoint before GDB gets unstopped later.
+     Threads get resumed at pthread_cond_wait below.  Use `while' loops for
+     protection against spurious pthread_cond_wait wakeups.  */
+
+  printf ("Waiting till the threads initialize their TIDs.\n");
+
+  while (thread1_tid == 0)
+    {
+      i = pthread_cond_wait (&thread1_tid_cond, &thread1_tid_mutex);
+      assert (i == 0);
+    }
+
+  while (thread2_tid == 0)
+    {
+      i = pthread_cond_wait (&thread2_tid_cond, &thread2_tid_mutex);
+      assert (i == 0);
+    }
+
+  printf ("Thread 1 TID = %lu, thread 2 TID = %lu, PID = %lu.\n",
+	  (unsigned long) thread1_tid, (unsigned long) thread2_tid,
+	  (unsigned long) getpid ());
+
+  printf ("Waiting till the threads get trapped by the breakpoint.\n");
+
+  if (tracer)
+    {
+      /* s390x-unknown-linux-gnu will fail with "R (running)".  */
+
+      state_wait (thread1_tid, "t (tracing stop)");
+
+      state_wait (thread2_tid, "t (tracing stop)");
+    }
+
+  cleanup ();
+
+  printf ("Joining the threads.\n");
+
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  i = pthread_join (thread1, NULL);
+  assert (i == 0);
+
+  i = pthread_join (thread2, NULL);
+  assert (i == 0);
+
+  printf ("Exiting.\n");	/* break-at-exit */
+
+  return EXIT_SUCCESS;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.exp
@@ -0,0 +1,76 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test SIGILL generated by some special cases of breakpoints on ia64.  Problem
+# was SIGILL being stored in non-current thread for later retrieval when its
+# breakpoint has been already deleted.  moribund locations are not active in
+# the default all-stop mode.
+
+set testfile "ia64-sigill"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" ${binfile} executable [list debug additional_flags=-lrt]] != "" } {
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    return -1
+}
+
+set test "info addr label"
+gdb_test_multiple $test $test {
+    -re "Symbol \"label\" is at 0x\[0-9a-f\]+0 in .*\r\n$gdb_prompt $" {
+	# Verify the label really starts at the start of ia64 bundle.
+	pass $test
+
+	# ia64 generates SIGILL for breakpoint at B slot of an MIB bundle.
+	gdb_test "break *label+2" {Breakpoint [0-9]+ at 0x[0-9a-f]+2:.*}
+    }
+    -re "No symbol \"label\" in current context\\.\r\n$gdb_prompt $" {
+	pass $test
+
+	# Either this target never generates non-SIGTRAP signals or they do
+	# not depend on the breakpoint address.  Try any address.
+	gdb_breakpoint [gdb_get_line_number "break-here"]
+    }
+}
+
+gdb_test_no_output {set $sigill_bpnum=$bpnum}
+
+gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+
+gdb_test_no_output "set debug infrun 1"
+
+# The ia64 SIGILL signal is visible only in the lin-lwp debug.
+gdb_test_no_output "set debug lin-lwp 1"
+
+gdb_test "continue" "Breakpoint \[0-9\]+,( .* in)? thread_func .*"
+
+gdb_test_no_output {delete $sigill_bpnum}
+
+set test "continue for the pending signal"
+gdb_test_multiple "continue" $test {
+    -re "Breakpoint \[0-9\]+, .*break-at-exit.*\r\n$gdb_prompt $" {
+	# Breakpoint has been skipped in the other thread.
+	pass $test
+    }
+    -re "Program received signal .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+}

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Tristan Gingold]

On Jul 19, 2010, at 10:58 AM, Jan Kratochvil wrote:

> Hi,
> 
> in some cases ia64 can generate SIGILL instead of SIGTRAP.  Guessing it is
> a CPU bug instead of Linux kernel bug (but I may be wrong).
> 	https://bugzilla.redhat.com/show_bug.cgi?id=615538
> 	http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ia64-sigill.c?cvsroot=systemtap

This is not a bug but a feature: the immediate of break.b is ignored by the processor.

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Jan Kratochvil]

On Mon, 19 Jul 2010 11:04:26 +0200, Tristan Gingold wrote:
> On Jul 19, 2010, at 10:58 AM, Jan Kratochvil wrote:
> > in some cases ia64 can generate SIGILL instead of SIGTRAP.  Guessing it is
> > a CPU bug instead of Linux kernel bug (but I may be wrong).
> > 	https://bugzilla.redhat.com/show_bug.cgi?id=615538
> > 	http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ia64-sigill.c?cvsroot=systemtap
> 
> This is not a bug but a feature: the immediate of break.b is ignored by the processor.

You are right, Intel IA64 manual says:
	For the b_unit_form, imm21 is ignored and the value zero is placed in
	the Interruption Immediate control register (IIM).

And kernel/arch/ia64/kernel/traps.c ia64_bad_break():
              case 0: /* unknown error (used by GCC for __builtin_abort()) */
		[...]
                sig = SIGILL; code = ILL_ILLOPC;

And it really reports SIGILL even for example for:
...0:       10 00 00 00 01 00       [MIB]       nop.m 0x0
...6:       00 00 00 00 00 00                   break.i 0x0
...c:       00 00 00 20                         nop.b 0x0

Thanks for the explanation (embarassingly I did read the IA64 manual first).

(The GDB patch part meaning has not changed by this mail.)


Jan

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Tristan Gingold]

On Jul 19, 2010, at 11:19 AM, Jan Kratochvil wrote:

> On Mon, 19 Jul 2010 11:04:26 +0200, Tristan Gingold wrote:
>> On Jul 19, 2010, at 10:58 AM, Jan Kratochvil wrote:
>>> in some cases ia64 can generate SIGILL instead of SIGTRAP.  Guessing it is
>>> a CPU bug instead of Linux kernel bug (but I may be wrong).
>>> 	https://bugzilla.redhat.com/show_bug.cgi?id=615538
>>> 	http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ia64-sigill.c?cvsroot=systemtap
>> 
>> This is not a bug but a feature: the immediate of break.b is ignored by the processor.
> 
> You are right, Intel IA64 manual says:
> 	For the b_unit_form, imm21 is ignored and the value zero is placed in
> 	the Interruption Immediate control register (IIM).

Yes, this is an amazing weirdness, which might be difficult to work-around before tukwila.

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Pedro Alves]

On Monday 19 July 2010 09:58:17, Jan Kratochvil wrote:
> Hi,
> 
> in some cases ia64 can generate SIGILL instead of SIGTRAP.  Guessing it is
> a CPU bug instead of Linux kernel bug (but I may be wrong).
> 	https://bugzilla.redhat.com/show_bug.cgi?id=615538
> 	http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/ia64-sigill.c?cvsroot=systemtap
> 
> Anyway FSF GDB already contains code for handling such cases (and not only
> SIGILL).  Just the time it gets executed the breakpoint which generated the
> signal may be already deleted and GDB stops with:
> 	Program received signal SIGILL, Illegal instruction.
> 
> I do not think it can be solved fully target-independent.  linux-nat.c already
> does some postponing of signals using
> 	kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (lp->status));
> and the moment linux-nat.c will read back such postponed signal its breakpoint
> may be already deleted (and thus it will never get to target-independent parts
> of GDB such as infrun.c).

That's a general problem with "real" SIGTRAP breakpoints as well.  linux-nat.c
handles this by "cancelling" the breakpoint, so that it is hit again later
on resume.  "cancelling" just means something like:

 Since we're not reporting the event to the user right now, just ignore
 it.  If the breakpoint is still inserted later,
 the next time we ask the target for an event (on resume), the inferior
 will trap on the same breakpoint again; we'll report it then, if so.

I don't see why this can't be extended to handle SIGILL breakpoints too
(with the difference that there's no PC adjustment required, you just
discard the signal).

Simply swapping SIGILL for SIGTRAP without accounting for the PC
adjustment that GDB will try to do on the SIGTRAP (thinking it was a real
SIGTRAP) makes me a bit nervous.

-- 
Pedro Alves

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Jan Kratochvil]

On Tue, 20 Jul 2010 15:28:58 +0200, Pedro Alves wrote:
> That's a general problem with "real" SIGTRAP breakpoints as well.  linux-nat.c
> handles this by "cancelling" the breakpoint, so that it is hit again later
> on resume.

Thanks for the pointer, yes, the needed code is already in place as I see.


> I don't see why this can't be extended to handle SIGILL breakpoints too
> (with the difference that there's no PC adjustment required, you just
> discard the signal).
> 
> Simply swapping SIGILL for SIGTRAP without accounting for the PC
> adjustment that GDB will try to do on the SIGTRAP (thinking it was a real
> SIGTRAP) makes me a bit nervous.

Extended it.

This SIGTRAP->SIGILL case happens only on ia64 and ia64 does not use any
set_gdbarch_decr_pc_after_break at all, PC stays on the breakpoint bundle+slot
in both the SIGTRAP and SIGILL case.

You are right it is arch-specific.  On i386 I checked SIGILL is never
generated (only in some fpu-emulated code).  So I checked s390x-linux-gnu::
	SIGILL on opcode 0xb29e
	si_addr = 0x800009a4
	.psw.addr = 0x800009a8
	instr at = 0x800009a4
	.psw.addr - instr == 4

	SIGTRAP on opcode 0x0001
	si_addr = (nil)
	.psw.addr = 0x800009a6
	instr at = 0x800009a4
	.psw.addr - instr == 2
	GDB really has: set_gdbarch_decr_pc_after_break (gdbarch, 2);

That "infrun: Treating signal as SIGTRAP\n" code I left in place now.
It should get disabled on arches where we know it is not needed but let it be
a different bug / mail thread.

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.
No regressions on ia64-rhel55-linux-gnu.


Thanks,
Jan


gdb/
2010-07-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* ia64-linux-nat.c (ia64_linux_cancel_breakpoint): New function.
	(_initialize_ia64_linux_nat): Install it.
	* linux-nat.c (cancel_breakpoint): Move the body to ...
	(linux_nat_cancel_breakpoint_when_signalled): ... a new function. Set
	LP->STATUS to 0 already here.
	(linux_nat_cancel_breakpoint_check_sigtrap)
	(linux_nat_cancel_breakpoint, linux_nat_set_cancel_breakpoint): New.
	(cancel_breakpoints_callback): Move the comment in front of the
	function.  Call linux_nat_cancel_breakpoint.
	(linux_nat_wait_1): Move the signals check and LP->STATUS reset into
	linux_nat_cancel_breakpoint.
	* linux-nat.h (linux_nat_cancel_breakpoint_when_signalled)
	(linux_nat_set_cancel_breakpoint): New prototypes.

gdb/testsuite/
2010-07-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/ia64-sigill.exp: New file.
	* gdb.threads/ia64-sigill.c: New file.

--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -809,6 +809,26 @@ ia64_linux_xfer_partial (struct target_ops *ops,
 			     offset, len);
 }
 
+/* For break.b instruction ia64 CPU forgets the immediate value and generates
+   SIGILL with ILL_ILLOPC instead of more common SIGTRAP with TRAP_BRKPT.  */
+
+static int
+ia64_linux_cancel_breakpoint (struct lwp_info *lp)
+{
+  /* We check for lp->waitstatus in addition to lp->status, because we can
+     have pending process exits recorded in lp->status
+     and W_EXITCODE(0,0) == 0.  We should probably have an additional
+     lp->status_p flag.  */
+
+  if (! (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
+	 && WIFSTOPPED (lp->status)
+	 && (WSTOPSIG (lp->status) == SIGTRAP
+	     || WSTOPSIG (lp->status) == SIGILL)))
+    return 0;
+
+  return linux_nat_cancel_breakpoint_when_signalled (lp);
+}
+
 void _initialize_ia64_linux_nat (void);
 
 void
@@ -848,4 +868,5 @@ _initialize_ia64_linux_nat (void)
   /* Register the target.  */
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, ia64_linux_new_thread);
+  linux_nat_set_cancel_breakpoint (t, ia64_linux_cancel_breakpoint);
 }
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2837,18 +2836,20 @@ select_event_lwp_callback (struct lwp_info *lp, void *data)
   return 0;
 }
 
-static int
-cancel_breakpoint (struct lwp_info *lp)
-{
-  /* Arrange for a breakpoint to be hit again later.  We don't keep
-     the SIGTRAP status and don't forward the SIGTRAP signal to the
-     LWP.  We will handle the current event, eventually we will resume
-     this LWP, and this breakpoint will trap again.
+/* Arrange for a breakpoint to be hit again later.  We don't keep the SIGTRAP
+   status and don't forward the SIGTRAP signal to the LWP.  We will handle the
+   current event, eventually we will resume this LWP, and this breakpoint will
+   trap again.
 
-     If we do not do this, then we run the risk that the user will
-     delete or disable the breakpoint, but the LWP will have already
-     tripped on it.  */
+   If we do not do this, then we run the risk that the user will delete or
+   disable the breakpoint, but the LWP will have already tripped on it.
+   
+   This function must be called with LP->STATUS signal already verified as
+   valid for a breakpoint.  */
 
+int
+linux_nat_cancel_breakpoint_when_signalled (struct lwp_info *lp)
+{
   struct regcache *regcache = get_thread_regcache (lp->ptid);
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   CORE_ADDR pc;
@@ -2865,11 +2866,52 @@ cancel_breakpoint (struct lwp_info *lp)
       if (gdbarch_decr_pc_after_break (gdbarch))
 	regcache_write_pc (regcache, pc);
 
+      /* Throw away the SIGTRAP.  */
+      lp->status = 0;
       return 1;
     }
   return 0;
 }
 
+/* Check we hit a breakpoint by checking the SIGTRAP signal.  */
+
+static int
+linux_nat_cancel_breakpoint_check_sigtrap (struct lwp_info *lp)
+{
+  /* We check for lp->waitstatus in addition to lp->status, because we can
+     have pending process exits recorded in lp->status
+     and W_EXITCODE(0,0) == 0.  We should probably have an additional
+     lp->status_p flag.  */
+
+  if (! (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
+         && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP))
+    return 0;
+
+  return linux_nat_cancel_breakpoint_when_signalled (lp);
+}
+
+static int (*linux_nat_cancel_breakpoint) (struct lwp_info *lp)
+  = linux_nat_cancel_breakpoint_check_sigtrap;
+
+/* Register an arch-specific alternative breakpoint hit check.  */
+
+void
+linux_nat_set_cancel_breakpoint (struct target_ops *t,
+                                 int (*func) (struct lwp_info *lp))
+{
+  linux_nat_cancel_breakpoint = func;
+}
+
+/* If a LWP other than the LWP that we're reporting an event for has hit a GDB
+   breakpoint (as opposed to some random trap signal), then just arrange for
+   it to hit it again later.  We don't keep the SIGTRAP status and don't
+   forward the SIGTRAP signal to the LWP.  We will handle the current event,
+   eventually we will resume all LWPs, and this one will get its breakpoint
+   trap again.
+
+   If we do not do this, then we run the risk that the user will delete or
+   disable the breakpoint, but the LWP will have already tripped on it.  */
+
 static int
 cancel_breakpoints_callback (struct lwp_info *lp, void *data)
 {
@@ -2879,23 +2921,7 @@ cancel_breakpoints_callback (struct lwp_info *lp, void *data)
   if (lp == event_lp)
     return 0;
 
-  /* If a LWP other than the LWP that we're reporting an event for has
-     hit a GDB breakpoint (as opposed to some random trap signal),
-     then just arrange for it to hit it again later.  We don't keep
-     the SIGTRAP status and don't forward the SIGTRAP signal to the
-     LWP.  We will handle the current event, eventually we will resume
-     all LWPs, and this one will get its breakpoint trap again.
-
-     If we do not do this, then we run the risk that the user will
-     delete or disable the breakpoint, but the LWP will have already
-     tripped on it.  */
-
-  if (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-      && lp->status != 0
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP
-      && cancel_breakpoint (lp))
-    /* Throw away the SIGTRAP.  */
-    lp->status = 0;
+  linux_nat_cancel_breakpoint (lp);
 
   return 0;
 }
@@ -3410,14 +3436,8 @@ retry:
 			 core before this one is handled.  All-stop
 			 always cancels breakpoint hits in all
 			 threads.  */
-		      if (non_stop
-			  && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-			  && WSTOPSIG (lp->status) == SIGTRAP
-			  && cancel_breakpoint (lp))
+		      if (non_stop && linux_nat_cancel_breakpoint (lp))
 			{
-			  /* Throw away the SIGTRAP.  */
-			  lp->status = 0;
-
 			  if (debug_linux_nat)
 			    fprintf (stderr,
 				     "LLW: LWP %ld hit a breakpoint while waiting "
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -172,3 +172,10 @@ struct siginfo *linux_nat_get_siginfo (ptid_t ptid);
 
 /* Compute and return the processor core of a given thread.  */
 int linux_nat_core_of_thread_1 (ptid_t ptid);
+
+/* Arrange for a breakpoint to be hit again later.   */
+int linux_nat_cancel_breakpoint_when_signalled (struct lwp_info *lp);
+
+/* Register an arch-specific alternative breakpoint hit check.  */
+void linux_nat_set_cancel_breakpoint (struct target_ops *t,
+				      int (*func) (struct lwp_info *lp));
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.c
@@ -0,0 +1,360 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <stdio.h>
+#include <limits.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <unistd.h>
+#include <asm/unistd.h>
+
+#define gettid() syscall (__NR_gettid)
+
+/* Terminate always in the main task, it can lock up with SIGSTOPped GDB
+   otherwise.  */
+#define TIMEOUT (gettid () == getpid() ? 10 : 15)
+
+static pid_t thread1_tid;
+static pthread_cond_t thread1_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread1_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pid_t thread2_tid;
+static pthread_cond_t thread2_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread2_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pthread_mutex_t terminate_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+/* Do not use alarm as it would create a ptrace event which would hang up us if
+   we are being traced by GDB which we stopped ourselves.  */
+
+static void timed_mutex_lock (pthread_mutex_t *mutex)
+{
+  int i;
+  struct timespec start, now;
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      i = pthread_mutex_trylock (mutex);
+      if (i == 0)
+	return;
+      assert (i == EBUSY);
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for internal lock!\n");
+  exit (EXIT_FAILURE);
+}
+
+static void *
+thread_func (void *threadno_voidp)
+{
+  int threadno = (intptr_t) threadno_voidp;
+  int i;
+
+  switch (threadno)
+  {
+    case 1:
+      timed_mutex_lock (&thread1_tid_mutex);
+
+      /* THREAD1_TID_MUTEX must be already locked to avoid race.  */
+      thread1_tid = gettid ();
+
+      i = pthread_cond_signal (&thread1_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread1_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    case 2:
+      timed_mutex_lock (&thread2_tid_mutex);
+
+      /* THREAD2_TID_MUTEX must be already locked to avoid race.  */
+      thread2_tid = gettid ();
+
+      i = pthread_cond_signal (&thread2_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread2_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    default:
+      assert (0);
+  }
+
+#ifdef __ia64__
+  asm volatile ("label:\n"
+		"nop.m 0\n"
+		"nop.i 0\n"
+		"nop.b 0\n");
+#endif
+  /* break-here */
+
+  /* Be sure the "t (tracing stop)" test can proceed for both threads.  */
+  timed_mutex_lock (&terminate_mutex);
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  return NULL;
+}
+
+static const char *
+proc_string (const char *filename, const char *line)
+{
+  FILE *f;
+  static char buf[LINE_MAX];
+  size_t line_len = strlen (line);
+
+  f = fopen (filename, "r");
+  if (f == NULL)
+    {
+      fprintf (stderr, "fopen (\"%s\") for \"%s\": %s\n", filename, line,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  while (errno = 0, fgets (buf, sizeof (buf), f))
+    {
+      char *s;
+
+      s = strchr (buf, '\n');
+      assert (s != NULL);
+      *s = 0;
+
+      if (strncmp (buf, line, line_len) != 0)
+	continue;
+
+      if (fclose (f))
+	{
+	  fprintf (stderr, "fclose (\"%s\") for \"%s\": %s\n", filename, line,
+		   strerror (errno));
+	  exit (EXIT_FAILURE);
+	}
+
+      return &buf[line_len];
+    }
+  if (errno != 0)
+    {
+      fprintf (stderr, "fgets (\"%s\": %s\n", filename, strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  fprintf (stderr, "\"%s\": No line \"%s\" found.\n", filename, line);
+  exit (EXIT_FAILURE);
+}
+
+static unsigned long
+proc_ulong (const char *filename, const char *line)
+{
+  const char *s = proc_string (filename, line);
+  long retval;
+  char *end;
+
+  errno = 0;
+  retval = strtol (s, &end, 10);
+  if (retval < 0 || retval >= LONG_MAX || (end && *end))
+    {
+      fprintf (stderr, "\"%s\":\"%s\": %ld, %s\n", filename, line, retval,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  return retval;
+}
+
+static void
+state_wait (pid_t process, const char *wanted)
+{
+  char *filename;
+  int i;
+  struct timespec start, now;
+  const char *state;
+
+  i = asprintf (&filename, "/proc/%lu/status", (unsigned long) process);
+  assert (i > 0);
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      state = proc_string (filename, "State:\t");
+
+      /* torvalds/linux-2.6.git 464763cf1c6df632dccc8f2f4c7e50163154a2c0
+	 has changed "T (tracing stop)" to "t (tracing stop)".  Make the GDB
+	 testcase backward compatible with older Linux kernels.  */
+      if (strcmp (state, "T (tracing stop)") == 0)
+	state = "t (tracing stop)";
+
+      if (strcmp (state, wanted) == 0)
+	{
+	  free (filename);
+	  return;
+	}
+
+      if (sched_yield ())
+	{
+	  perror ("sched_yield()");
+	  exit (EXIT_FAILURE);
+	}
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for PID %lu \"%s\" (now it is \"%s\")!\n",
+	   (unsigned long) process, wanted, state);
+  exit (EXIT_FAILURE);
+}
+
+static volatile pid_t tracer = 0;
+static pthread_t thread1, thread2;
+
+static void
+cleanup (void)
+{
+  printf ("Resuming GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      int i;
+      int tracer_save = tracer;
+
+      tracer = 0;
+
+      i = kill (tracer_save, SIGCONT);
+      assert (i == 0);
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  int i;
+  int standalone = 0;
+
+  if (argc == 2 && strcmp (argv[1], "-s") == 0)
+    standalone = 1;
+  else
+    assert (argc == 1);
+
+  setbuf (stdout, NULL);
+
+  timed_mutex_lock (&thread1_tid_mutex);
+  timed_mutex_lock (&thread2_tid_mutex);
+
+  timed_mutex_lock (&terminate_mutex);
+
+  i = pthread_create (&thread1, NULL, thread_func, (void *) (intptr_t) 1);
+  assert (i == 0);
+
+  i = pthread_create (&thread2, NULL, thread_func, (void *) (intptr_t) 2);
+  assert (i == 0);
+
+  if (!standalone)
+    {
+      tracer = proc_ulong ("/proc/self/status", "TracerPid:\t");
+      if (tracer == 0)
+	{
+	  fprintf (stderr, "The testcase must be run by GDB!\n");
+	  exit (EXIT_FAILURE);
+	}
+      if (tracer != getppid ())
+	{
+	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
+	  exit (EXIT_FAILURE);
+	}
+    }
+
+  /* SIGCONT our debugger in the case of our crash as we would deadlock
+     otherwise.  */
+
+  atexit (cleanup);
+
+  printf ("Stopping GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      i = kill (tracer, SIGSTOP);
+      assert (i == 0);
+      state_wait (tracer, "T (stopped)");
+    }
+
+  /* Threads are now waiting at timed_mutex_lock (thread1_tid_mutex) and so
+     they could not trigger the breakpoint before GDB gets unstopped later.
+     Threads get resumed at pthread_cond_wait below.  Use `while' loops for
+     protection against spurious pthread_cond_wait wakeups.  */
+
+  printf ("Waiting till the threads initialize their TIDs.\n");
+
+  while (thread1_tid == 0)
+    {
+      i = pthread_cond_wait (&thread1_tid_cond, &thread1_tid_mutex);
+      assert (i == 0);
+    }
+
+  while (thread2_tid == 0)
+    {
+      i = pthread_cond_wait (&thread2_tid_cond, &thread2_tid_mutex);
+      assert (i == 0);
+    }
+
+  printf ("Thread 1 TID = %lu, thread 2 TID = %lu, PID = %lu.\n",
+	  (unsigned long) thread1_tid, (unsigned long) thread2_tid,
+	  (unsigned long) getpid ());
+
+  printf ("Waiting till the threads get trapped by the breakpoint.\n");
+
+  if (tracer)
+    {
+      /* s390x-unknown-linux-gnu will fail with "R (running)".  */
+
+      state_wait (thread1_tid, "t (tracing stop)");
+
+      state_wait (thread2_tid, "t (tracing stop)");
+    }
+
+  cleanup ();
+
+  printf ("Joining the threads.\n");
+
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  i = pthread_join (thread1, NULL);
+  assert (i == 0);
+
+  i = pthread_join (thread2, NULL);
+  assert (i == 0);
+
+  printf ("Exiting.\n");	/* break-at-exit */
+
+  return EXIT_SUCCESS;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.exp
@@ -0,0 +1,76 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test SIGILL generated by some special cases of breakpoints on ia64.  Problem
+# was SIGILL being stored in non-current thread for later retrieval when its
+# breakpoint has been already deleted.  moribund locations are not active in
+# the default all-stop mode.
+
+set testfile "ia64-sigill"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" ${binfile} executable [list debug additional_flags=-lrt]] != "" } {
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    return -1
+}
+
+set test "info addr label"
+gdb_test_multiple $test $test {
+    -re "Symbol \"label\" is at 0x\[0-9a-f\]+0 in .*\r\n$gdb_prompt $" {
+	# Verify the label really starts at the start of ia64 bundle.
+	pass $test
+
+	# ia64 generates SIGILL for breakpoint at B slot of an MIB bundle.
+	gdb_test "break *label+2" {Breakpoint [0-9]+ at 0x[0-9a-f]+2:.*}
+    }
+    -re "No symbol \"label\" in current context\\.\r\n$gdb_prompt $" {
+	pass $test
+
+	# Either this target never generates non-SIGTRAP signals or they do
+	# not depend on the breakpoint address.  Try any address.
+	gdb_breakpoint [gdb_get_line_number "break-here"]
+    }
+}
+
+gdb_test_no_output {set $sigill_bpnum=$bpnum}
+
+gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+
+gdb_test_no_output "set debug infrun 1"
+
+# The ia64 SIGILL signal is visible only in the lin-lwp debug.
+gdb_test_no_output "set debug lin-lwp 1"
+
+gdb_test "continue" "Breakpoint \[0-9\]+,( .* in)? thread_func .*"
+
+gdb_test_no_output {delete $sigill_bpnum}
+
+set test "continue for the pending signal"
+gdb_test_multiple "continue" $test {
+    -re "Breakpoint \[0-9\]+, .*break-at-exit.*\r\n$gdb_prompt $" {
+	# Breakpoint has been skipped in the other thread.
+	pass $test
+    }
+    -re "Program received signal .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+}

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Pedro Alves]

On Friday 23 July 2010 23:19:35, Jan Kratochvil wrote:

> This SIGTRAP->SIGILL case happens only on ia64 and ia64 does not use any
> set_gdbarch_decr_pc_after_break at all, PC stays on the breakpoint bundle+slot
> in both the SIGTRAP and SIGILL case.
> 
> You are right it is arch-specific.  On i386 I checked SIGILL is never
> generated (only in some fpu-emulated code).  So I checked s390x-linux-gnu::
> 	SIGILL on opcode 0xb29e
> 	si_addr = 0x800009a4
> 	.psw.addr = 0x800009a8
> 	instr at = 0x800009a4
> 	.psw.addr - instr == 4

Oh?  Does this mean the PC is left pointing _after_ the instruction
that caused the SIGILL?

I think you still need to audit other bits in linux-nat.c for SIGTRAP bkpts
handling.  E.g., see stop_wait_callback.  (For extra correctness,
count_events_callback, and the select_event_lwp_callback functions would
be relaxed too.)  I wasn't previously suggesting to make this ia64 arch
specific, which made fixing these other places too easier.  Notice how
gdbserver/linux-low.c also considers non-sigtrap bkpts (and it was a recent
change, needed for ARM thumb2 kernels that hadn't learned about the
breakpoint insns yet, IIRC).

> --- a/gdb/ia64-linux-nat.c
> +++ b/gdb/ia64-linux-nat.c
> @@ -809,6 +809,26 @@ ia64_linux_xfer_partial (struct target_ops *ops,
>  			     offset, len);
>  }
>  
> +/* For break.b instruction ia64 CPU forgets the immediate value and generates
> +   SIGILL with ILL_ILLOPC instead of more common SIGTRAP with TRAP_BRKPT.  */
> +
> +static int
> +ia64_linux_cancel_breakpoint (struct lwp_info *lp)
> +{
> +  /* We check for lp->waitstatus in addition to lp->status, because we can
> +     have pending process exits recorded in lp->status
> +     and W_EXITCODE(0,0) == 0.  We should probably have an additional
> +     lp->status_p flag.  */
> +
> +  if (! (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
> +	 && WIFSTOPPED (lp->status)
> +	 && (WSTOPSIG (lp->status) == SIGTRAP
> +	     || WSTOPSIG (lp->status) == SIGILL)))
> +    return 0;
> +
> +  return linux_nat_cancel_breakpoint_when_signalled (lp);

If this stays, please add a comment above this call mentioning that
we can safely call this function even for SIGILL, since
decr_pc_after_break is 0 on ia64.  (Alternatively, recode to avoid
the assumption)  The point I'm making is that PC adjustment is
always only necessary for sigtraps, never other signals (and is in fact
wrong for other signals).

-- 
Pedro Alves

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Jan Kratochvil]

On Sun, 25 Jul 2010 00:26:04 +0200, Pedro Alves wrote:
> On Friday 23 July 2010 23:19:35, Jan Kratochvil wrote:
> 
> > This SIGTRAP->SIGILL case happens only on ia64 and ia64 does not use any
> > set_gdbarch_decr_pc_after_break at all, PC stays on the breakpoint bundle+slot
> > in both the SIGTRAP and SIGILL case.
> > 
> > You are right it is arch-specific.  On i386 I checked SIGILL is never
> > generated (only in some fpu-emulated code).  So I checked s390x-linux-gnu::
> > 	SIGILL on opcode 0xb29e
> > 	si_addr = 0x800009a4
> > 	.psw.addr = 0x800009a8
> > 	instr at = 0x800009a4
> > 	.psw.addr - instr == 4
> 
> Oh?  Does this mean the PC is left pointing _after_ the instruction
> that caused the SIGILL?

Yes.  It probably corresponds to:

390 - z/Architecture Principles of Operation (assembly manual)
http://publibz.boulder.ibm.com/epubs/pdf/a2278325.pdf
6-6 Instruction-Length Code
The instruction-length code (ILC) occupies two bit positions and provides the 
length of the last instruction executed.  It permits identifying the
instruction causing the interruption when the instruction address in the old
PSW designates the next sequential instruction.
                   ^^^^

set_gdbarch_decr_pc_after_break = 2 is still OK there:

gdbarch_decr_pc_after_break:
This function shall return the amount by which to decrement the PC after the
program encounters a breakpoint.
                   ^^^^^^^^^^^^

As I see it means GDB-placed-breakpoint.  When GDB hits x86_64 random "int3"
it does not decrement PC (which may be for a discussion if it should but
currently it does not by design).  GDB does not intentionally place SIGILL
generating instruction on s390x so it does not need to recognize ILC.


> I think you still need to audit other bits in linux-nat.c for SIGTRAP bkpts
> handling.  E.g., see stop_wait_callback.  (For extra correctness,
> count_events_callback, and the select_event_lwp_callback functions would
> be relaxed too.)  I wasn't previously suggesting to make this ia64 arch
> specific, which made fixing these other places too easier.  Notice how
> gdbserver/linux-low.c also considers non-sigtrap bkpts (and it was a recent
> change, needed for ARM thumb2 kernels that hadn't learned about the
> breakpoint insns yet, IIRC).

Are we talking about?
	RFC: Updates support for breakpoints that generate SIGILL
	http://sourceware.org/ml/gdb-patches/2010-01/msg00619.html

As it is for arm*.c which does not use set_gdbarch_decr_pc_after_break I do
not want to predict how SIGILL vs. gdbarch_decr_pc_after_break behaves.

Which arch uses gdbarch_decr_pc_after_break && generates a non-SIGTRAP signal
on a GDB-placed breakpoint?  It is probably none of the arches I target (i686,
x86_64, ppc{,64}, s390{,x}, ia64, possibly arm).


> If this stays, please add a comment above this call mentioning that
> we can safely call this function even for SIGILL, since
> decr_pc_after_break is 0 on ia64.

Done.


> (Alternatively, recode to avoid the assumption)

Sorry I do not try to code anything without an iron to test it.


> The point I'm making is that PC adjustment is always only necessary for
> sigtraps, never other signals (and is in fact wrong for other signals).

I believe you mean the adjustment is necessary for GDB-placed instructions
only.

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.
Tested the new testcase on ia64-rhel55-linux-gnu.

OK to check-in?


Thanks,
Jan


gdb/
2010-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* ia64-linux-nat.c (ia64_linux_status_is_event): New function.
	(_initialize_ia64_linux_nat): Install it.
	* linux-nat.c (sigtrap_is_event, linux_nat_status_is_event)
	(linux_nat_set_status_is_event): New.
	(stop_wait_callback, count_events_callback, select_event_lwp_callback)
	cancel_breakpoints_callback, linux_nat_filter_event)
	(linux_nat_wait_1): Use linux_nat_status_is_event.
	* linux-nat.h (linux_nat_set_status_is_event): New prototype.

gdb/testsuite/
2010-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/ia64-sigill.exp: New file.
	* gdb.threads/ia64-sigill.c: New file.

--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -809,6 +809,18 @@ ia64_linux_xfer_partial (struct target_ops *ops,
 			     offset, len);
 }
 
+/* For break.b instruction ia64 CPU forgets the immediate value and generates
+   SIGILL with ILL_ILLOPC instead of more common SIGTRAP with TRAP_BRKPT.
+   ia64 does not use gdbarch_decr_pc_after_break so we do not have to make any
+   difference for the signals here.  */
+
+static int
+ia64_linux_status_is_event (int status)
+{
+  return WIFSTOPPED (status) && (WSTOPSIG (status) == SIGTRAP
+				 || WSTOPSIG (status) == SIGILL);
+}
+
 void _initialize_ia64_linux_nat (void);
 
 void
@@ -848,4 +860,5 @@ _initialize_ia64_linux_nat (void)
   /* Register the target.  */
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, ia64_linux_new_thread);
+  linux_nat_set_status_is_event (t, ia64_linux_status_is_event);
 }
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2605,6 +2605,29 @@ linux_nat_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   return lp->stopped_data_address_p;
 }
 
+/* Commonly any breakpoint / watchpoint generate only SIGTRAP.  */
+
+static int
+sigtrap_is_event (int status)
+{
+  return WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP;
+}
+
+/* SIGTRAP-like events recognizer.  */
+
+static int (*linux_nat_status_is_event) (int status) = sigtrap_is_event;
+
+/* Set altarnative SIGTRAP-like events recognizer.  If
+   breakpoint_inserted_here_p there then gdbarch_decr_pc_after_break will be
+   applied.  */
+
+void
+linux_nat_set_status_is_event (struct target_ops *t,
+			       int (*status_is_event) (int status))
+{
+  linux_nat_status_is_event = status_is_event;
+}
+
 /* Wait until LP is stopped.  */
 
 static int
@@ -2645,7 +2668,7 @@ stop_wait_callback (struct lwp_info *lp, void *data)
 
       if (WSTOPSIG (status) != SIGSTOP)
 	{
-	  if (WSTOPSIG (status) == SIGTRAP)
+	  if (linux_nat_status_is_event (status))
 	    {
 	      /* If a LWP other than the LWP that we're reporting an
 	         event for has hit a GDB breakpoint (as opposed to
@@ -2801,7 +2824,7 @@ count_events_callback (struct lwp_info *lp, void *data)
 
   /* Count only resumed LWPs that have a SIGTRAP event pending.  */
   if (lp->status != 0 && lp->resumed
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP)
+      && linux_nat_status_is_event (lp->status))
     (*count)++;
 
   return 0;
@@ -2829,7 +2852,7 @@ select_event_lwp_callback (struct lwp_info *lp, void *data)
 
   /* Select only resumed LWPs that have a SIGTRAP event pending. */
   if (lp->status != 0 && lp->resumed
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP)
+      && linux_nat_status_is_event (lp->status))
     if ((*selector)-- == 0)
       return 1;
 
@@ -2891,7 +2914,7 @@ cancel_breakpoints_callback (struct lwp_info *lp, void *data)
 
   if (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
       && lp->status != 0
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP
+      && linux_nat_status_is_event (lp->status)
       && cancel_breakpoint (lp))
     /* Throw away the SIGTRAP.  */
     lp->status = 0;
@@ -3064,7 +3087,7 @@ linux_nat_filter_event (int lwpid, int status, int options)
 	return NULL;
     }
 
-  if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
+  if (linux_nat_status_is_event (status))
     {
       /* Save the trap's siginfo in case we need it later.  */
       save_siginfo (lp);
@@ -3411,7 +3434,7 @@ retry:
 			 threads.  */
 		      if (non_stop
 			  && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-			  && WSTOPSIG (lp->status) == SIGTRAP
+			  && linux_nat_status_is_event (lp->status)
 			  && cancel_breakpoint (lp))
 			{
 			  /* Throw away the SIGTRAP.  */
@@ -3627,7 +3650,7 @@ retry:
   else
     lp->resumed = 0;
 
-  if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
+  if (linux_nat_status_is_event (status))
     {
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -172,3 +172,7 @@ struct siginfo *linux_nat_get_siginfo (ptid_t ptid);
 
 /* Compute and return the processor core of a given thread.  */
 int linux_nat_core_of_thread_1 (ptid_t ptid);
+
+/* Set altarnative SIGTRAP-like events recognizer.  */
+void linux_nat_set_status_is_event (struct target_ops *t,
+				    int (*status_is_event) (int status));
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.c
@@ -0,0 +1,360 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <stdio.h>
+#include <limits.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <unistd.h>
+#include <asm/unistd.h>
+
+#define gettid() syscall (__NR_gettid)
+
+/* Terminate always in the main task, it can lock up with SIGSTOPped GDB
+   otherwise.  */
+#define TIMEOUT (gettid () == getpid() ? 10 : 15)
+
+static pid_t thread1_tid;
+static pthread_cond_t thread1_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread1_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pid_t thread2_tid;
+static pthread_cond_t thread2_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread2_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pthread_mutex_t terminate_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+/* Do not use alarm as it would create a ptrace event which would hang up us if
+   we are being traced by GDB which we stopped ourselves.  */
+
+static void timed_mutex_lock (pthread_mutex_t *mutex)
+{
+  int i;
+  struct timespec start, now;
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      i = pthread_mutex_trylock (mutex);
+      if (i == 0)
+	return;
+      assert (i == EBUSY);
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for internal lock!\n");
+  exit (EXIT_FAILURE);
+}
+
+static void *
+thread_func (void *threadno_voidp)
+{
+  int threadno = (intptr_t) threadno_voidp;
+  int i;
+
+  switch (threadno)
+  {
+    case 1:
+      timed_mutex_lock (&thread1_tid_mutex);
+
+      /* THREAD1_TID_MUTEX must be already locked to avoid race.  */
+      thread1_tid = gettid ();
+
+      i = pthread_cond_signal (&thread1_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread1_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    case 2:
+      timed_mutex_lock (&thread2_tid_mutex);
+
+      /* THREAD2_TID_MUTEX must be already locked to avoid race.  */
+      thread2_tid = gettid ();
+
+      i = pthread_cond_signal (&thread2_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread2_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    default:
+      assert (0);
+  }
+
+#ifdef __ia64__
+  asm volatile ("label:\n"
+		"nop.m 0\n"
+		"nop.i 0\n"
+		"nop.b 0\n");
+#endif
+  /* break-here */
+
+  /* Be sure the "t (tracing stop)" test can proceed for both threads.  */
+  timed_mutex_lock (&terminate_mutex);
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  return NULL;
+}
+
+static const char *
+proc_string (const char *filename, const char *line)
+{
+  FILE *f;
+  static char buf[LINE_MAX];
+  size_t line_len = strlen (line);
+
+  f = fopen (filename, "r");
+  if (f == NULL)
+    {
+      fprintf (stderr, "fopen (\"%s\") for \"%s\": %s\n", filename, line,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  while (errno = 0, fgets (buf, sizeof (buf), f))
+    {
+      char *s;
+
+      s = strchr (buf, '\n');
+      assert (s != NULL);
+      *s = 0;
+
+      if (strncmp (buf, line, line_len) != 0)
+	continue;
+
+      if (fclose (f))
+	{
+	  fprintf (stderr, "fclose (\"%s\") for \"%s\": %s\n", filename, line,
+		   strerror (errno));
+	  exit (EXIT_FAILURE);
+	}
+
+      return &buf[line_len];
+    }
+  if (errno != 0)
+    {
+      fprintf (stderr, "fgets (\"%s\": %s\n", filename, strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  fprintf (stderr, "\"%s\": No line \"%s\" found.\n", filename, line);
+  exit (EXIT_FAILURE);
+}
+
+static unsigned long
+proc_ulong (const char *filename, const char *line)
+{
+  const char *s = proc_string (filename, line);
+  long retval;
+  char *end;
+
+  errno = 0;
+  retval = strtol (s, &end, 10);
+  if (retval < 0 || retval >= LONG_MAX || (end && *end))
+    {
+      fprintf (stderr, "\"%s\":\"%s\": %ld, %s\n", filename, line, retval,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  return retval;
+}
+
+static void
+state_wait (pid_t process, const char *wanted)
+{
+  char *filename;
+  int i;
+  struct timespec start, now;
+  const char *state;
+
+  i = asprintf (&filename, "/proc/%lu/status", (unsigned long) process);
+  assert (i > 0);
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      state = proc_string (filename, "State:\t");
+
+      /* torvalds/linux-2.6.git 464763cf1c6df632dccc8f2f4c7e50163154a2c0
+	 has changed "T (tracing stop)" to "t (tracing stop)".  Make the GDB
+	 testcase backward compatible with older Linux kernels.  */
+      if (strcmp (state, "T (tracing stop)") == 0)
+	state = "t (tracing stop)";
+
+      if (strcmp (state, wanted) == 0)
+	{
+	  free (filename);
+	  return;
+	}
+
+      if (sched_yield ())
+	{
+	  perror ("sched_yield()");
+	  exit (EXIT_FAILURE);
+	}
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for PID %lu \"%s\" (now it is \"%s\")!\n",
+	   (unsigned long) process, wanted, state);
+  exit (EXIT_FAILURE);
+}
+
+static volatile pid_t tracer = 0;
+static pthread_t thread1, thread2;
+
+static void
+cleanup (void)
+{
+  printf ("Resuming GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      int i;
+      int tracer_save = tracer;
+
+      tracer = 0;
+
+      i = kill (tracer_save, SIGCONT);
+      assert (i == 0);
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  int i;
+  int standalone = 0;
+
+  if (argc == 2 && strcmp (argv[1], "-s") == 0)
+    standalone = 1;
+  else
+    assert (argc == 1);
+
+  setbuf (stdout, NULL);
+
+  timed_mutex_lock (&thread1_tid_mutex);
+  timed_mutex_lock (&thread2_tid_mutex);
+
+  timed_mutex_lock (&terminate_mutex);
+
+  i = pthread_create (&thread1, NULL, thread_func, (void *) (intptr_t) 1);
+  assert (i == 0);
+
+  i = pthread_create (&thread2, NULL, thread_func, (void *) (intptr_t) 2);
+  assert (i == 0);
+
+  if (!standalone)
+    {
+      tracer = proc_ulong ("/proc/self/status", "TracerPid:\t");
+      if (tracer == 0)
+	{
+	  fprintf (stderr, "The testcase must be run by GDB!\n");
+	  exit (EXIT_FAILURE);
+	}
+      if (tracer != getppid ())
+	{
+	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
+	  exit (EXIT_FAILURE);
+	}
+    }
+
+  /* SIGCONT our debugger in the case of our crash as we would deadlock
+     otherwise.  */
+
+  atexit (cleanup);
+
+  printf ("Stopping GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      i = kill (tracer, SIGSTOP);
+      assert (i == 0);
+      state_wait (tracer, "T (stopped)");
+    }
+
+  /* Threads are now waiting at timed_mutex_lock (thread1_tid_mutex) and so
+     they could not trigger the breakpoint before GDB gets unstopped later.
+     Threads get resumed at pthread_cond_wait below.  Use `while' loops for
+     protection against spurious pthread_cond_wait wakeups.  */
+
+  printf ("Waiting till the threads initialize their TIDs.\n");
+
+  while (thread1_tid == 0)
+    {
+      i = pthread_cond_wait (&thread1_tid_cond, &thread1_tid_mutex);
+      assert (i == 0);
+    }
+
+  while (thread2_tid == 0)
+    {
+      i = pthread_cond_wait (&thread2_tid_cond, &thread2_tid_mutex);
+      assert (i == 0);
+    }
+
+  printf ("Thread 1 TID = %lu, thread 2 TID = %lu, PID = %lu.\n",
+	  (unsigned long) thread1_tid, (unsigned long) thread2_tid,
+	  (unsigned long) getpid ());
+
+  printf ("Waiting till the threads get trapped by the breakpoint.\n");
+
+  if (tracer)
+    {
+      /* s390x-unknown-linux-gnu will fail with "R (running)".  */
+
+      state_wait (thread1_tid, "t (tracing stop)");
+
+      state_wait (thread2_tid, "t (tracing stop)");
+    }
+
+  cleanup ();
+
+  printf ("Joining the threads.\n");
+
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  i = pthread_join (thread1, NULL);
+  assert (i == 0);
+
+  i = pthread_join (thread2, NULL);
+  assert (i == 0);
+
+  printf ("Exiting.\n");	/* break-at-exit */
+
+  return EXIT_SUCCESS;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/ia64-sigill.exp
@@ -0,0 +1,76 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test SIGILL generated by some special cases of breakpoints on ia64.  Problem
+# was SIGILL being stored in non-current thread for later retrieval when its
+# breakpoint has been already deleted.  moribund locations are not active in
+# the default all-stop mode.
+
+set testfile "ia64-sigill"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" ${binfile} executable [list debug additional_flags=-lrt]] != "" } {
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    return -1
+}
+
+set test "info addr label"
+gdb_test_multiple $test $test {
+    -re "Symbol \"label\" is at 0x\[0-9a-f\]+0 in .*\r\n$gdb_prompt $" {
+	# Verify the label really starts at the start of ia64 bundle.
+	pass $test
+
+	# ia64 generates SIGILL for breakpoint at B slot of an MIB bundle.
+	gdb_test "break *label+2" {Breakpoint [0-9]+ at 0x[0-9a-f]+2:.*}
+    }
+    -re "No symbol \"label\" in current context\\.\r\n$gdb_prompt $" {
+	pass $test
+
+	# Either this target never generates non-SIGTRAP signals or they do
+	# not depend on the breakpoint address.  Try any address.
+	gdb_breakpoint [gdb_get_line_number "break-here"]
+    }
+}
+
+gdb_test_no_output {set $sigill_bpnum=$bpnum}
+
+gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+
+gdb_test_no_output "set debug infrun 1"
+
+# The ia64 SIGILL signal is visible only in the lin-lwp debug.
+gdb_test_no_output "set debug lin-lwp 1"
+
+gdb_test "continue" "Breakpoint \[0-9\]+,( .* in)? thread_func .*"
+
+gdb_test_no_output {delete $sigill_bpnum}
+
+set test "continue for the pending signal"
+gdb_test_multiple "continue" $test {
+    -re "Breakpoint \[0-9\]+, .*break-at-exit.*\r\n$gdb_prompt $" {
+	# Breakpoint has been skipped in the other thread.
+	pass $test
+    }
+    -re "Program received signal .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+}

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint [cleanup]

[Jan Kratochvil]

Hi Pedro,

add-on patch to make the code less magic a bit.

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.
Tested gdb.threads/ia64-sigill.exp on ia64-rhel55-linux-gnu.


Thanks,
Jan


gdb/
2010-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-nat.c (linux_nat_lp_status_is_event): New function.
	(count_events_callback, select_event_lwp_callback)
	(cancel_breakpoints_callback, linux_nat_wait_1): Use it.

--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2617,6 +2617,20 @@ sigtrap_is_event (int status)
 
 static int (*linux_nat_status_is_event) (int status) = sigtrap_is_event;
 
+/* Check for SIGTRAP-like events in LP.  */
+
+static int
+linux_nat_lp_status_is_event (struct lwp_info *lp)
+{
+  /* We check for lp->waitstatus in addition to lp->status, because we can
+     have pending process exits recorded in lp->status
+     and W_EXITCODE(0,0) == 0.  We should probably have an additional
+     lp->status_p flag.  */
+
+  return (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
+	  && linux_nat_status_is_event (lp->status));
+}
+
 /* Set altarnative SIGTRAP-like events recognizer.  If
    breakpoint_inserted_here_p there then gdbarch_decr_pc_after_break will be
    applied.  */
@@ -2823,8 +2837,7 @@ count_events_callback (struct lwp_info *lp, void *data)
   gdb_assert (count != NULL);
 
   /* Count only resumed LWPs that have a SIGTRAP event pending.  */
-  if (lp->status != 0 && lp->resumed
-      && linux_nat_status_is_event (lp->status))
+  if (lp->resumed && linux_nat_lp_status_is_event (lp))
     (*count)++;
 
   return 0;
@@ -2851,8 +2864,7 @@ select_event_lwp_callback (struct lwp_info *lp, void *data)
   gdb_assert (selector != NULL);
 
   /* Select only resumed LWPs that have a SIGTRAP event pending. */
-  if (lp->status != 0 && lp->resumed
-      && linux_nat_status_is_event (lp->status))
+  if (lp->resumed && linux_nat_lp_status_is_event (lp))
     if ((*selector)-- == 0)
       return 1;
 
@@ -2912,9 +2924,7 @@ cancel_breakpoints_callback (struct lwp_info *lp, void *data)
      delete or disable the breakpoint, but the LWP will have already
      tripped on it.  */
 
-  if (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-      && lp->status != 0
-      && linux_nat_status_is_event (lp->status)
+  if (linux_nat_lp_status_is_event (lp)
       && cancel_breakpoint (lp))
     /* Throw away the SIGTRAP.  */
     lp->status = 0;
@@ -3433,8 +3443,7 @@ retry:
 			 always cancels breakpoint hits in all
 			 threads.  */
 		      if (non_stop
-			  && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-			  && linux_nat_status_is_event (lp->status)
+			  && linux_nat_lp_status_is_event (lp)
 			  && cancel_breakpoint (lp))
 			{
 			  /* Throw away the SIGTRAP.  */

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Pedro Alves]

On Sunday 25 July 2010 19:52:17, Jan Kratochvil wrote:
> On Sun, 25 Jul 2010 00:26:04 +0200, Pedro Alves wrote:
> > On Friday 23 July 2010 23:19:35, Jan Kratochvil wrote:
> > 
> > > This SIGTRAP->SIGILL case happens only on ia64 and ia64 does not use any
> > > set_gdbarch_decr_pc_after_break at all, PC stays on the breakpoint bundle+slot
> > > in both the SIGTRAP and SIGILL case.
> > > 
> > > You are right it is arch-specific.  On i386 I checked SIGILL is never
> > > generated (only in some fpu-emulated code).  So I checked s390x-linux-gnu::
> > > 	SIGILL on opcode 0xb29e
> > > 	si_addr = 0x800009a4
> > > 	.psw.addr = 0x800009a8
> > > 	instr at = 0x800009a4
> > > 	.psw.addr - instr == 4
> > 
> > Oh?  Does this mean the PC is left pointing _after_ the instruction
> > that caused the SIGILL?
> 
> Yes.  It probably corresponds to:
> 
> 390 - z/Architecture Principles of Operation (assembly manual)
> http://publibz.boulder.ibm.com/epubs/pdf/a2278325.pdf
> 6-6 Instruction-Length Code
> The instruction-length code (ILC) occupies two bit positions and provides the 
> length of the last instruction executed.  It permits identifying the
> instruction causing the interruption when the instruction address in the old
> PSW designates the next sequential instruction.
>                    ^^^^

I guess I'm surprised.  Doesn't s390 have a variable length encoding?  If
the processor just tried to execute an illegal instruction, I'm not seeing
what does exactly mean the length of the bad instruction, and that "next"
ends up pointing at a real instruction boundary.  Oh, well, whatever.
Good to know that we need to be careful with this.  Thanks for the pointer.

> 
> set_gdbarch_decr_pc_after_break = 2 is still OK there:

> GDB does not intentionally place SIGILL
> generating instruction on s390x so it does not need to recognize ILC.

Right.  decr_pc_after_break generally models the PC offset to the breakpoint
instruction, when the processor finishes executing a breakpoint trap, not
random exception modes.  If ever needed, we might pass the signal/event
as argument to gdbarch_decr_pc_after_break.  And let the callback consult
siginfo too...  We'll cross that bridge when we come to it.

> > I think you still need to audit other bits in linux-nat.c for SIGTRAP bkpts
> > handling.  E.g., see stop_wait_callback.  (For extra correctness,
> > count_events_callback, and the select_event_lwp_callback functions would
> > be relaxed too.)  I wasn't previously suggesting to make this ia64 arch
> > specific, which made fixing these other places too easier.  Notice how
> > gdbserver/linux-low.c also considers non-sigtrap bkpts (and it was a recent
> > change, needed for ARM thumb2 kernels that hadn't learned about the
> > breakpoint insns yet, IIRC).
> 
> Are we talking about?
> 	RFC: Updates support for breakpoints that generate SIGILL
> 	http://sourceware.org/ml/gdb-patches/2010-01/msg00619.html
> 
> As it is for arm*.c which does not use set_gdbarch_decr_pc_after_break I do
> not want to predict how SIGILL vs. gdbarch_decr_pc_after_break behaves.

It's for arm, but it's implemented in the arch-independent linux-low.c.
If your check on s390 is good, than that might have broken (rare ...) corner
cases on s390, like a SIGILL reported for an instruction set at the address
consecutive to an inserted software breakpoint.

> Which arch uses gdbarch_decr_pc_after_break && generates a non-SIGTRAP signal
> on a GDB-placed breakpoint?  It is probably none of the arches I target (i686,
> x86_64, ppc{,64}, s390{,x}, ia64, possibly arm).

I don't have a list.  Some archs may generate that only on some
situations (or used to and no longer do on more up to date versions).
I wouldn't know how all targets/archs behave in the call dummies on a
non-executable stack scenario, for example.  In such case, the target
doesn't actually execute the trap, and reports an exception (that results
in say, SIGSEGV), so gdbarch_decr_pc_after_break doesn't really apply.

From infrun.c:

  /* First, distinguish signals caused by the debugger from signals
     that have to do with the program's own actions.  Note that
     breakpoint insns may cause SIGTRAP or SIGILL or SIGEMT, depending
     on the operating system version.  Here we detect when a SIGILL or
     SIGEMT is really a breakpoint and change it to SIGTRAP.  We do
     something similar for SIGSEGV, since a SIGSEGV will be generated
     when we're trying to execute a breakpoint instruction on a
     non-executable stack.  This happens for call dummy breakpoints
     for architectures like SPARC that place call dummies on the
     stack.  */

According to <http://en.wikipedia.org/wiki/SIGEMT>
> On the PDP-11 architecture, EMT, TRAP, BPT and IOT were instructions used for
> program traps. EMT was for system software, TRAP for programmer use, BPT for 
> debugging, and IOT for I/O routines. Currently EMT does not have a standardised usage.

> > If this stays, please add a comment above this call mentioning that
> > we can safely call this function even for SIGILL, since
> > decr_pc_after_break is 0 on ia64.
> 
> Done.

Thanks.

> 
> 
> > (Alternatively, recode to avoid the assumption)
> 
> Sorry I do not try to code anything without an iron to test it.

What I meant was to have your new code not depend on decr_pc_after
break being 0, you don't need any other system to try that on.
It meant, to not have any code path in the SIGILL
case that calls gdbarch_decr_pc_after_break at all. Conceptually,
something like:

if (sig == SIGTRAP && software_breakpoint_inserted_at (PC - decr_pc_after_breakpoint))
 {
    rewind pc, discard signal
    return 1
 }
else if (sig == SIGILL && software_breakpoint_inserted_at (PC))
 {
    discard signal
    return 1
 }
return 0


> OK to check-in?

Okay.  Please fix the spelling of "alternative".

-- 
Pedro Alves

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint [cleanup]

[Pedro Alves]

On Sunday 25 July 2010 19:55:12, Jan Kratochvil wrote:

> gdb/
> 2010-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* linux-nat.c (linux_nat_lp_status_is_event): New function.
> 	(count_events_callback, select_event_lwp_callback)
> 	(cancel_breakpoints_callback, linux_nat_wait_1): Use it.

Looks okay.  A few comments below, but I'm feeling passive today,
and I'll let this go in unmodified.

> +static int
> +linux_nat_lp_status_is_event (struct lwp_info *lp)

Static helper functions that aren't a part of the target_ops
interface (as opposed to linux_nat_resume, say), or external,
don't usually take the "linux_nat_" prefix, so you could shorten
the function name.

I'm not particularly fond of the function names.  The reason is
that:

- linux_nat_status_is_event

    Returns true for breakpoint events, and other SIGTRAP events.
    Returns false for other kinds of events (random signals), though
    we could call these events too.

- linux_nat_lp_status_is_event

    Returns true for breakpoint events, and other SIGTRAP events
    Returns false for other kinds of events (random signals), and
    fork/exec/clone/exit events, though we could call these events too.

Note the subtle differences.

> +{
> +  /* We check for lp->waitstatus in addition to lp->status, because we can
> +     have pending process exits recorded in lp->status
> +     and W_EXITCODE(0,0) == 0.  We should probably have an additional
> +     lp->status_p flag.  */
> +
> +  return (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
> +	  && linux_nat_status_is_event (lp->status));
> +}
> +
>  /* Set altarnative SIGTRAP-like events recognizer.  If
>     breakpoint_inserted_here_p there then gdbarch_decr_pc_after_break will be
>     applied.  */
> @@ -2823,8 +2837,7 @@ count_events_callback (struct lwp_info *lp, void *data)
>    gdb_assert (count != NULL);
>  
>    /* Count only resumed LWPs that have a SIGTRAP event pending.  */
> -  if (lp->status != 0 && lp->resumed
> -      && linux_nat_status_is_event (lp->status))
> +  if (lp->resumed && linux_nat_lp_status_is_event (lp))
>      (*count)++;

This appears to be changing behaviour (the new waitstatus check within
the new function), but I doubt it makes a difference.  I'm pointing
it out, because you're presenting the patch as cleanup.  I actually wonder
why we only select SIGTRAP-like events instead of all reportable events
(as opposed to our own SIGSTOPs).

>  
>    return 0;
> @@ -2851,8 +2864,7 @@ select_event_lwp_callback (struct lwp_info *lp, void *data)
>    gdb_assert (selector != NULL);
>  
>    /* Select only resumed LWPs that have a SIGTRAP event pending. */
> -  if (lp->status != 0 && lp->resumed
> -      && linux_nat_status_is_event (lp->status))
> +  if (lp->resumed && linux_nat_lp_status_is_event (lp))
>      if ((*selector)-- == 0)
>        return 1;

(Same comment here.)

>  
> @@ -2912,9 +2924,7 @@ cancel_breakpoints_callback (struct lwp_info *lp, void *data)
>       delete or disable the breakpoint, but the LWP will have already
>       tripped on it.  */
>  
> -  if (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
> -      && lp->status != 0
> -      && linux_nat_status_is_event (lp->status)
> +  if (linux_nat_lp_status_is_event (lp)
>        && cancel_breakpoint (lp))
>      /* Throw away the SIGTRAP.  */
>      lp->status = 0;
> @@ -3433,8 +3443,7 @@ retry:
>  			 always cancels breakpoint hits in all
>  			 threads.  */
>  		      if (non_stop
> -			  && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
> -			  && linux_nat_status_is_event (lp->status)
> +			  && linux_nat_lp_status_is_event (lp)
>  			  && cancel_breakpoint (lp))
>  			{
>  			  /* Throw away the SIGTRAP.  */
> 


-- 
Pedro Alves

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint

[Jan Kratochvil]

On Tue, 27 Jul 2010 13:43:16 +0200, Pedro Alves wrote:
> On Sunday 25 July 2010 19:52:17, Jan Kratochvil wrote:
> > On Sun, 25 Jul 2010 00:26:04 +0200, Pedro Alves wrote:

> If your check on s390 is good, than that might have broken (rare ...) corner
> cases on s390, like a SIGILL reported for an instruction set at the address
> consecutive to an inserted software breakpoint.

gdb/gdbserver/linux-low.c maybe_internal_trap may get set wrong on s390, OK.


> > Which arch uses gdbarch_decr_pc_after_break && generates a non-SIGTRAP signal
> > on a GDB-placed breakpoint?  It is probably none of the arches I target (i686,
> > x86_64, ppc{,64}, s390{,x}, ia64, possibly arm).
> 
> I don't have a list.  Some archs may generate that only on some
> situations (or used to and no longer do on more up to date versions).
                                                          kernel


> > > (Alternatively, recode to avoid the assumption)
> > 
> > Sorry I do not try to code anything without an iron to test it.
> 
> What I meant was to have your new code not depend on decr_pc_after
> break being 0, you don't need any other system to try that on.
> It meant, to not have any code path in the SIGILL
> case that calls gdbarch_decr_pc_after_break at all. Conceptually,
> something like:
> 
> if (sig == SIGTRAP && software_breakpoint_inserted_at (PC - decr_pc_after_breakpoint))
>  {
>     rewind pc, discard signal
>     return 1
>  }
> else if (sig == SIGILL && software_breakpoint_inserted_at (PC))
>  {
>     discard signal
>     return 1
>  }
> return 0

I believe we could need some ` - decr_pc_after_sigill' in the second
conditional as shown on the s390 case.  But only if GDB would place any such
SIGILL instruction so this paragraph is just a hand waving.

Not implementing this alternative way, thanks for the analysis.


> Okay.  Please fix the spelling of "alternative".

I am sorry.  Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-07/msg00170.html

--- src/gdb/ChangeLog	2010/07/27 20:44:30	1.12025
+++ src/gdb/ChangeLog	2010/07/27 20:51:37	1.12026
@@ -1,3 +1,14 @@
+2010-07-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* ia64-linux-nat.c (ia64_linux_status_is_event): New function.
+	(_initialize_ia64_linux_nat): Install it.
+	* linux-nat.c (sigtrap_is_event, linux_nat_status_is_event)
+	(linux_nat_set_status_is_event): New.
+	(stop_wait_callback, count_events_callback, select_event_lwp_callback)
+	cancel_breakpoints_callback, linux_nat_filter_event)
+	(linux_nat_wait_1): Use linux_nat_status_is_event.
+	* linux-nat.h (linux_nat_set_status_is_event): New prototype.
+
 2010-07-27  Tom Tromey  <tromey@redhat.com>
 
 	* NEWS: Mention labels, .gdb_index.
--- src/gdb/ia64-linux-nat.c	2010/07/07 16:15:16	1.49
+++ src/gdb/ia64-linux-nat.c	2010/07/27 20:51:37	1.50
@@ -809,6 +809,18 @@
 			     offset, len);
 }
 
+/* For break.b instruction ia64 CPU forgets the immediate value and generates
+   SIGILL with ILL_ILLOPC instead of more common SIGTRAP with TRAP_BRKPT.
+   ia64 does not use gdbarch_decr_pc_after_break so we do not have to make any
+   difference for the signals here.  */
+
+static int
+ia64_linux_status_is_event (int status)
+{
+  return WIFSTOPPED (status) && (WSTOPSIG (status) == SIGTRAP
+				 || WSTOPSIG (status) == SIGILL);
+}
+
 void _initialize_ia64_linux_nat (void);
 
 void
@@ -848,4 +860,5 @@
   /* Register the target.  */
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, ia64_linux_new_thread);
+  linux_nat_set_status_is_event (t, ia64_linux_status_is_event);
 }
--- src/gdb/linux-nat.c	2010/07/25 09:31:12	1.178
+++ src/gdb/linux-nat.c	2010/07/27 20:51:37	1.179
@@ -2605,6 +2605,29 @@
   return lp->stopped_data_address_p;
 }
 
+/* Commonly any breakpoint / watchpoint generate only SIGTRAP.  */
+
+static int
+sigtrap_is_event (int status)
+{
+  return WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP;
+}
+
+/* SIGTRAP-like events recognizer.  */
+
+static int (*linux_nat_status_is_event) (int status) = sigtrap_is_event;
+
+/* Set alternative SIGTRAP-like events recognizer.  If
+   breakpoint_inserted_here_p there then gdbarch_decr_pc_after_break will be
+   applied.  */
+
+void
+linux_nat_set_status_is_event (struct target_ops *t,
+			       int (*status_is_event) (int status))
+{
+  linux_nat_status_is_event = status_is_event;
+}
+
 /* Wait until LP is stopped.  */
 
 static int
@@ -2645,7 +2668,7 @@
 
       if (WSTOPSIG (status) != SIGSTOP)
 	{
-	  if (WSTOPSIG (status) == SIGTRAP)
+	  if (linux_nat_status_is_event (status))
 	    {
 	      /* If a LWP other than the LWP that we're reporting an
 	         event for has hit a GDB breakpoint (as opposed to
@@ -2801,7 +2824,7 @@
 
   /* Count only resumed LWPs that have a SIGTRAP event pending.  */
   if (lp->status != 0 && lp->resumed
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP)
+      && linux_nat_status_is_event (lp->status))
     (*count)++;
 
   return 0;
@@ -2829,7 +2852,7 @@
 
   /* Select only resumed LWPs that have a SIGTRAP event pending. */
   if (lp->status != 0 && lp->resumed
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP)
+      && linux_nat_status_is_event (lp->status))
     if ((*selector)-- == 0)
       return 1;
 
@@ -2891,7 +2914,7 @@
 
   if (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
       && lp->status != 0
-      && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP
+      && linux_nat_status_is_event (lp->status)
       && cancel_breakpoint (lp))
     /* Throw away the SIGTRAP.  */
     lp->status = 0;
@@ -3064,7 +3087,7 @@
 	return NULL;
     }
 
-  if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
+  if (linux_nat_status_is_event (status))
     {
       /* Save the trap's siginfo in case we need it later.  */
       save_siginfo (lp);
@@ -3411,7 +3434,7 @@
 			 threads.  */
 		      if (non_stop
 			  && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-			  && WSTOPSIG (lp->status) == SIGTRAP
+			  && linux_nat_status_is_event (lp->status)
 			  && cancel_breakpoint (lp))
 			{
 			  /* Throw away the SIGTRAP.  */
@@ -3627,7 +3650,7 @@
   else
     lp->resumed = 0;
 
-  if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
+  if (linux_nat_status_is_event (status))
     {
       if (debug_linux_nat)
 	fprintf_unfiltered (gdb_stdlog,
--- src/gdb/linux-nat.h	2010/06/11 12:10:12	1.35
+++ src/gdb/linux-nat.h	2010/07/27 20:51:37	1.36
@@ -172,3 +172,7 @@
 
 /* Compute and return the processor core of a given thread.  */
 int linux_nat_core_of_thread_1 (ptid_t ptid);
+
+/* Set alternative SIGTRAP-like events recognizer.  */
+void linux_nat_set_status_is_event (struct target_ops *t,
+				    int (*status_is_event) (int status));
--- src/gdb/testsuite/ChangeLog	2010/07/27 18:08:47	1.2391
+++ src/gdb/testsuite/ChangeLog	2010/07/27 20:51:37	1.2392
@@ -1,3 +1,8 @@
+2010-07-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.threads/ia64-sigill.exp: New file.
+	* gdb.threads/ia64-sigill.c: New file.
+
 2010-07-27  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.opt/inline-cmds.c (ATTR): New define.
--- src/gdb/testsuite/gdb.threads/ia64-sigill.c
+++ src/gdb/testsuite/gdb.threads/ia64-sigill.c	2010-07-27 20:51:55.245809000 +0000
@@ -0,0 +1,360 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <stdio.h>
+#include <limits.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <unistd.h>
+#include <asm/unistd.h>
+
+#define gettid() syscall (__NR_gettid)
+
+/* Terminate always in the main task, it can lock up with SIGSTOPped GDB
+   otherwise.  */
+#define TIMEOUT (gettid () == getpid() ? 10 : 15)
+
+static pid_t thread1_tid;
+static pthread_cond_t thread1_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread1_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pid_t thread2_tid;
+static pthread_cond_t thread2_tid_cond = PTHREAD_COND_INITIALIZER;
+static pthread_mutex_t thread2_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+static pthread_mutex_t terminate_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+/* Do not use alarm as it would create a ptrace event which would hang up us if
+   we are being traced by GDB which we stopped ourselves.  */
+
+static void timed_mutex_lock (pthread_mutex_t *mutex)
+{
+  int i;
+  struct timespec start, now;
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      i = pthread_mutex_trylock (mutex);
+      if (i == 0)
+	return;
+      assert (i == EBUSY);
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for internal lock!\n");
+  exit (EXIT_FAILURE);
+}
+
+static void *
+thread_func (void *threadno_voidp)
+{
+  int threadno = (intptr_t) threadno_voidp;
+  int i;
+
+  switch (threadno)
+  {
+    case 1:
+      timed_mutex_lock (&thread1_tid_mutex);
+
+      /* THREAD1_TID_MUTEX must be already locked to avoid race.  */
+      thread1_tid = gettid ();
+
+      i = pthread_cond_signal (&thread1_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread1_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    case 2:
+      timed_mutex_lock (&thread2_tid_mutex);
+
+      /* THREAD2_TID_MUTEX must be already locked to avoid race.  */
+      thread2_tid = gettid ();
+
+      i = pthread_cond_signal (&thread2_tid_cond);
+      assert (i == 0);
+      i = pthread_mutex_unlock (&thread2_tid_mutex);
+      assert (i == 0);
+
+      break;
+
+    default:
+      assert (0);
+  }
+
+#ifdef __ia64__
+  asm volatile ("label:\n"
+		"nop.m 0\n"
+		"nop.i 0\n"
+		"nop.b 0\n");
+#endif
+  /* break-here */
+
+  /* Be sure the "t (tracing stop)" test can proceed for both threads.  */
+  timed_mutex_lock (&terminate_mutex);
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  return NULL;
+}
+
+static const char *
+proc_string (const char *filename, const char *line)
+{
+  FILE *f;
+  static char buf[LINE_MAX];
+  size_t line_len = strlen (line);
+
+  f = fopen (filename, "r");
+  if (f == NULL)
+    {
+      fprintf (stderr, "fopen (\"%s\") for \"%s\": %s\n", filename, line,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  while (errno = 0, fgets (buf, sizeof (buf), f))
+    {
+      char *s;
+
+      s = strchr (buf, '\n');
+      assert (s != NULL);
+      *s = 0;
+
+      if (strncmp (buf, line, line_len) != 0)
+	continue;
+
+      if (fclose (f))
+	{
+	  fprintf (stderr, "fclose (\"%s\") for \"%s\": %s\n", filename, line,
+		   strerror (errno));
+	  exit (EXIT_FAILURE);
+	}
+
+      return &buf[line_len];
+    }
+  if (errno != 0)
+    {
+      fprintf (stderr, "fgets (\"%s\": %s\n", filename, strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  fprintf (stderr, "\"%s\": No line \"%s\" found.\n", filename, line);
+  exit (EXIT_FAILURE);
+}
+
+static unsigned long
+proc_ulong (const char *filename, const char *line)
+{
+  const char *s = proc_string (filename, line);
+  long retval;
+  char *end;
+
+  errno = 0;
+  retval = strtol (s, &end, 10);
+  if (retval < 0 || retval >= LONG_MAX || (end && *end))
+    {
+      fprintf (stderr, "\"%s\":\"%s\": %ld, %s\n", filename, line, retval,
+	       strerror (errno));
+      exit (EXIT_FAILURE);
+    }
+  return retval;
+}
+
+static void
+state_wait (pid_t process, const char *wanted)
+{
+  char *filename;
+  int i;
+  struct timespec start, now;
+  const char *state;
+
+  i = asprintf (&filename, "/proc/%lu/status", (unsigned long) process);
+  assert (i > 0);
+
+  i = clock_gettime (CLOCK_MONOTONIC, &start);
+  assert (i == 0);
+
+  do
+    {
+      state = proc_string (filename, "State:\t");
+
+      /* torvalds/linux-2.6.git 464763cf1c6df632dccc8f2f4c7e50163154a2c0
+	 has changed "T (tracing stop)" to "t (tracing stop)".  Make the GDB
+	 testcase backward compatible with older Linux kernels.  */
+      if (strcmp (state, "T (tracing stop)") == 0)
+	state = "t (tracing stop)";
+
+      if (strcmp (state, wanted) == 0)
+	{
+	  free (filename);
+	  return;
+	}
+
+      if (sched_yield ())
+	{
+	  perror ("sched_yield()");
+	  exit (EXIT_FAILURE);
+	}
+
+      i = clock_gettime (CLOCK_MONOTONIC, &now);
+      assert (i == 0);
+      assert (now.tv_sec >= start.tv_sec);
+    }
+  while (now.tv_sec - start.tv_sec < TIMEOUT);
+
+  fprintf (stderr, "Timed out waiting for PID %lu \"%s\" (now it is \"%s\")!\n",
+	   (unsigned long) process, wanted, state);
+  exit (EXIT_FAILURE);
+}
+
+static volatile pid_t tracer = 0;
+static pthread_t thread1, thread2;
+
+static void
+cleanup (void)
+{
+  printf ("Resuming GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      int i;
+      int tracer_save = tracer;
+
+      tracer = 0;
+
+      i = kill (tracer_save, SIGCONT);
+      assert (i == 0);
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  int i;
+  int standalone = 0;
+
+  if (argc == 2 && strcmp (argv[1], "-s") == 0)
+    standalone = 1;
+  else
+    assert (argc == 1);
+
+  setbuf (stdout, NULL);
+
+  timed_mutex_lock (&thread1_tid_mutex);
+  timed_mutex_lock (&thread2_tid_mutex);
+
+  timed_mutex_lock (&terminate_mutex);
+
+  i = pthread_create (&thread1, NULL, thread_func, (void *) (intptr_t) 1);
+  assert (i == 0);
+
+  i = pthread_create (&thread2, NULL, thread_func, (void *) (intptr_t) 2);
+  assert (i == 0);
+
+  if (!standalone)
+    {
+      tracer = proc_ulong ("/proc/self/status", "TracerPid:\t");
+      if (tracer == 0)
+	{
+	  fprintf (stderr, "The testcase must be run by GDB!\n");
+	  exit (EXIT_FAILURE);
+	}
+      if (tracer != getppid ())
+	{
+	  fprintf (stderr, "The testcase parent must be our GDB tracer!\n");
+	  exit (EXIT_FAILURE);
+	}
+    }
+
+  /* SIGCONT our debugger in the case of our crash as we would deadlock
+     otherwise.  */
+
+  atexit (cleanup);
+
+  printf ("Stopping GDB PID %lu.\n", (unsigned long) tracer);
+
+  if (tracer)
+    {
+      i = kill (tracer, SIGSTOP);
+      assert (i == 0);
+      state_wait (tracer, "T (stopped)");
+    }
+
+  /* Threads are now waiting at timed_mutex_lock (thread1_tid_mutex) and so
+     they could not trigger the breakpoint before GDB gets unstopped later.
+     Threads get resumed at pthread_cond_wait below.  Use `while' loops for
+     protection against spurious pthread_cond_wait wakeups.  */
+
+  printf ("Waiting till the threads initialize their TIDs.\n");
+
+  while (thread1_tid == 0)
+    {
+      i = pthread_cond_wait (&thread1_tid_cond, &thread1_tid_mutex);
+      assert (i == 0);
+    }
+
+  while (thread2_tid == 0)
+    {
+      i = pthread_cond_wait (&thread2_tid_cond, &thread2_tid_mutex);
+      assert (i == 0);
+    }
+
+  printf ("Thread 1 TID = %lu, thread 2 TID = %lu, PID = %lu.\n",
+	  (unsigned long) thread1_tid, (unsigned long) thread2_tid,
+	  (unsigned long) getpid ());
+
+  printf ("Waiting till the threads get trapped by the breakpoint.\n");
+
+  if (tracer)
+    {
+      /* s390x-unknown-linux-gnu will fail with "R (running)".  */
+
+      state_wait (thread1_tid, "t (tracing stop)");
+
+      state_wait (thread2_tid, "t (tracing stop)");
+    }
+
+  cleanup ();
+
+  printf ("Joining the threads.\n");
+
+  i = pthread_mutex_unlock (&terminate_mutex);
+  assert (i == 0);
+
+  i = pthread_join (thread1, NULL);
+  assert (i == 0);
+
+  i = pthread_join (thread2, NULL);
+  assert (i == 0);
+
+  printf ("Exiting.\n");	/* break-at-exit */
+
+  return EXIT_SUCCESS;
+}
--- src/gdb/testsuite/gdb.threads/ia64-sigill.exp
+++ src/gdb/testsuite/gdb.threads/ia64-sigill.exp	2010-07-27 20:51:55.778901000 +0000
@@ -0,0 +1,76 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test SIGILL generated by some special cases of breakpoints on ia64.  Problem
+# was SIGILL being stored in non-current thread for later retrieval when its
+# breakpoint has been already deleted.  moribund locations are not active in
+# the default all-stop mode.
+
+set testfile "ia64-sigill"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" ${binfile} executable [list debug additional_flags=-lrt]] != "" } {
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    return -1
+}
+
+set test "info addr label"
+gdb_test_multiple $test $test {
+    -re "Symbol \"label\" is at 0x\[0-9a-f\]+0 in .*\r\n$gdb_prompt $" {
+	# Verify the label really starts at the start of ia64 bundle.
+	pass $test
+
+	# ia64 generates SIGILL for breakpoint at B slot of an MIB bundle.
+	gdb_test "break *label+2" {Breakpoint [0-9]+ at 0x[0-9a-f]+2:.*}
+    }
+    -re "No symbol \"label\" in current context\\.\r\n$gdb_prompt $" {
+	pass $test
+
+	# Either this target never generates non-SIGTRAP signals or they do
+	# not depend on the breakpoint address.  Try any address.
+	gdb_breakpoint [gdb_get_line_number "break-here"]
+    }
+}
+
+gdb_test_no_output {set $sigill_bpnum=$bpnum}
+
+gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+
+gdb_test_no_output "set debug infrun 1"
+
+# The ia64 SIGILL signal is visible only in the lin-lwp debug.
+gdb_test_no_output "set debug lin-lwp 1"
+
+gdb_test "continue" "Breakpoint \[0-9\]+,( .* in)? thread_func .*"
+
+gdb_test_no_output {delete $sigill_bpnum}
+
+set test "continue for the pending signal"
+gdb_test_multiple "continue" $test {
+    -re "Breakpoint \[0-9\]+, .*break-at-exit.*\r\n$gdb_prompt $" {
+	# Breakpoint has been skipped in the other thread.
+	pass $test
+    }
+    -re "Program received signal .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+}

Re: [patch] Fix linux-ia64 on SIGILL for deleted breakpoint [cleanup]

[Jan Kratochvil]

On Tue, 27 Jul 2010 13:59:18 +0200, Pedro Alves wrote:
> > +static int
> > +linux_nat_lp_status_is_event (struct lwp_info *lp)
> 
> Static helper functions that aren't a part of the target_ops
> interface (as opposed to linux_nat_resume, say), or external,
> don't usually take the "linux_nat_" prefix, so you could shorten
> the function name.

There is a problem that linux_nat_set_status_is_event is not static and
therefore it is correctly ^linux_nat_ prefixed.  I thought all the other
related functions should have matching name, which assigns them the incorrect
^linux_nat_ prefix.


> I'm not particularly fond of the function names.  The reason is
> that:
> 
> - linux_nat_status_is_event
> 
>     Returns true for breakpoint events, and other SIGTRAP events.
>     Returns false for other kinds of events (random signals), though
>     we could call these events too.
> 
> - linux_nat_lp_status_is_event
> 
>     Returns true for breakpoint events, and other SIGTRAP events
>     Returns false for other kinds of events (random signals), and
>     fork/exec/clone/exit events, though we could call these events too.
> 
> Note the subtle differences.

Yes... the problem is the variables already are not systematic.  There should
be that lp->status_p flag discussed in the comments, TARGET_SIGNAL_TRAP is
incorrectly used as an abstraction of inferior event (even for
SIGILL/SIGSEGV/etc.), lp->signalled should be called lp->sigstop_sent,
lp->resumed should be called lp->want_to_resume, target_signal should be
called gdb_signal, host signal should be called target signal,
lp->waitstatus.kind TARGET_WAITKIND_* I cannot describe right offhand now.

I have to admit I mostly gave up on building systematic functions and naming
on top of it.


> This appears to be changing behaviour (the new waitstatus check within
> the new function), but I doubt it makes a difference.  I'm pointing
> it out, because you're presenting the patch as cleanup.

Sorry for the naming, I did not indend to consider this patch as a no
functionality change patch.  (Although it should not be a user visible
change.)


Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-07/msg00171.html

--- src/gdb/ChangeLog	2010/07/27 20:51:37	1.12026
+++ src/gdb/ChangeLog	2010/07/27 21:22:07	1.12027
@@ -1,5 +1,11 @@
 2010-07-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	* linux-nat.c (linux_nat_lp_status_is_event): New function.
+	(count_events_callback, select_event_lwp_callback)
+	(cancel_breakpoints_callback, linux_nat_wait_1): Use it.
+
+2010-07-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* ia64-linux-nat.c (ia64_linux_status_is_event): New function.
 	(_initialize_ia64_linux_nat): Install it.
 	* linux-nat.c (sigtrap_is_event, linux_nat_status_is_event)
--- src/gdb/linux-nat.c	2010/07/27 20:51:37	1.179
+++ src/gdb/linux-nat.c	2010/07/27 21:22:09	1.180
@@ -2617,6 +2617,20 @@
 
 static int (*linux_nat_status_is_event) (int status) = sigtrap_is_event;
 
+/* Check for SIGTRAP-like events in LP.  */
+
+static int
+linux_nat_lp_status_is_event (struct lwp_info *lp)
+{
+  /* We check for lp->waitstatus in addition to lp->status, because we can
+     have pending process exits recorded in lp->status
+     and W_EXITCODE(0,0) == 0.  We should probably have an additional
+     lp->status_p flag.  */
+
+  return (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
+	  && linux_nat_status_is_event (lp->status));
+}
+
 /* Set alternative SIGTRAP-like events recognizer.  If
    breakpoint_inserted_here_p there then gdbarch_decr_pc_after_break will be
    applied.  */
@@ -2823,8 +2837,7 @@
   gdb_assert (count != NULL);
 
   /* Count only resumed LWPs that have a SIGTRAP event pending.  */
-  if (lp->status != 0 && lp->resumed
-      && linux_nat_status_is_event (lp->status))
+  if (lp->resumed && linux_nat_lp_status_is_event (lp))
     (*count)++;
 
   return 0;
@@ -2851,8 +2864,7 @@
   gdb_assert (selector != NULL);
 
   /* Select only resumed LWPs that have a SIGTRAP event pending. */
-  if (lp->status != 0 && lp->resumed
-      && linux_nat_status_is_event (lp->status))
+  if (lp->resumed && linux_nat_lp_status_is_event (lp))
     if ((*selector)-- == 0)
       return 1;
 
@@ -2912,9 +2924,7 @@
      delete or disable the breakpoint, but the LWP will have already
      tripped on it.  */
 
-  if (lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-      && lp->status != 0
-      && linux_nat_status_is_event (lp->status)
+  if (linux_nat_lp_status_is_event (lp)
       && cancel_breakpoint (lp))
     /* Throw away the SIGTRAP.  */
     lp->status = 0;
@@ -3433,8 +3443,7 @@
 			 always cancels breakpoint hits in all
 			 threads.  */
 		      if (non_stop
-			  && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE
-			  && linux_nat_status_is_event (lp->status)
+			  && linux_nat_lp_status_is_event (lp)
 			  && cancel_breakpoint (lp))
 			{
 			  /* Throw away the SIGTRAP.  */