Attachment 14005 Details for Bug 28942 – A WIP patch

[patch] A WIP patch

a.patch (text/plain), 12.07 KB, created by Andrew Burgess on 2022-03-04 14:01:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Burgess

Created: 2022-03-04 14:01:08 UTC

Size: 12.07 KB

patch

obsolete

>commit 568b4aaaff661dcb4c46b512ce8738d80ef4cc38
>Author: Andrew Burgess <aburgess@redhat.com>
>Date:   Wed Mar 2 09:38:35 2022 +0000
>
>    [wip]gdb: attempt to fix inferior call from breakpoint condition
>    
>    Make an inferior call from a breakpoint condition in all-stop mode.
>    This currently leads to an error about registers not being available,
>    which is because GDB tries to resume an already running thread.
>    
>    Interestingly, part of the problem here will be fixed by my big
>    executing/resumed fix patch, as we end up trying to resume a thread
>    that is executing, but not resumed - an illegal state.
>    
>    However, with the executing/resumed fix, GDB will now hang.  The
>    inferior call is completed correctly, but after the inferior call GDB
>    will stop all threads (GDB is in all-stop mode), now, if the
>    breakpoint condition is false, GDB will resume the main thread, but
>    all the other threads will be stuck in the stopped state!
>    
>    The fix in this commit doesn't actually require the big
>    executing/resumed patch, as the fix I propose here (currently) avoids
>    trying to resume any threads other than the thread in which the
>    breakpoint condition is hit.
>    
>    This is done by evaluating the breakpoint condition in non-stop mode.
>    
>    I still need to figure out what should happen if/when we are dealing
>    with a target that doesn't support non-stop mode, though I'm not
>    entirely sure if that matters or not.  Supporting non-stop mode is
>    really about supporting async-mode, which is about being able to
>    handle stop events arriving while the user is sat at a GDB prompt, and
>    still being able to notify GDB that the event has arrived.
>    
>    Problem is, in this case, I don't think we care about the event
>    interrupting us, we want to evaluate the condition, make a decision,
>    and then either stop the original thread, or resume the original
>    thread, at which point, events that arrived for "other" threads would
>    be handled.
>    
>    There's still a lot of testing that needs to be done for this change.
>
>diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>index a3cfeea6989..5d571433776 100644
>--- a/gdb/breakpoint.c
>+++ b/gdb/breakpoint.c
>@@ -5234,6 +5236,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>       return;
>     }
> 
>+  scoped_restore restore_non_stop = make_scoped_restore (&non_stop, 1);
>+
>   /* Evaluate extension language breakpoints that have a "stop" method
>      implemented.  */
>   bs->stop = breakpoint_ext_lang_cond_says_stop (b);
>diff --git a/gdb/testsuite/gdb.threads/all-stop-infcall.c b/gdb/testsuite/gdb.threads/all-stop-infcall.c
>new file mode 100644
>index 00000000000..2bae92b1ee6
>--- /dev/null
>+++ b/gdb/testsuite/gdb.threads/all-stop-infcall.c
>@@ -0,0 +1,230 @@
>+/* This testcase is part of GDB, the GNU debugger.
>+
>+   Copyright 2022 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/>.  */
>+
>+#include <stdio.h>
>+#include <pthread.h>
>+#include <unistd.h>
>+#include <stdlib.h>
>+#include <errno.h>
>+
>+/* Holds a thread id, and the pipe file descriptors used to talk between
>+   the child and parent thread.  */
>+
>+struct thread_info
>+{
>+  /* The thread handle.  */
>+  pthread_t id;
>+
>+  /* Pipe where the write end is for the parent, and the read end is for
>+     the thread.   */
>+  int to_thread[2];
>+
>+
>+  /* Pipe where the write end is for the thread, and the read end is for
>+     the parent.  */
>+  int to_parent[2];
>+};
>+
>+/* A global value. We use this for the breakpoint condition, but access it
>+   through get_global_value.  */
>+
>+int global_value = 0;
>+
>+/* An function to read global_value, calling this from the breakpoint
>+   condition will require an inferior function call.  */
>+
>+int
>+get_global_value ()
>+{
>+  char buf[1024];
>+
>+  snprintf (buf, sizeof (buf), "get_global_value (%d)", global_value);
>+  puts (buf);
>+  return global_value;
>+}
>+
>+/* Another function to call from a breakpoint condition, this one will
>+   trigger a segfault.  */
>+
>+void
>+function_that_segfaults (void)
>+{
>+  int *p = 0;
>+  *p = 1;	/* Segfault happens here.   */
>+}
>+
>+/* Wait for a single byte on the read end of PIPE (that is PIPE[0]).  */
>+
>+void
>+wait_for_byte (int pipe[2])
>+{
>+  ssize_t res;
>+  char buf;
>+
>+  do
>+    {
>+      res = read (pipe[0], &buf, sizeof (buf));
>+    }
>+  while (res == -1 && errno == EINTR);
>+  if (res != sizeof (buf))
>+    abort ();
>+}
>+
>+/* Send a single byte to the write end of PIPE (that is PIPE[1]).  */
>+
>+void
>+send_byte (int pipe[2])
>+{
>+  ssize_t res;
>+  char buf = 'x';
>+
>+  do
>+    {
>+      res = write (pipe[1], &buf, sizeof (buf));
>+    }
>+  while (res == -1 && errno == EINTR);
>+  if (res != sizeof (buf))
>+    {
>+      perror ("write");
>+      abort ();
>+    }
>+}
>+
>+/* Create a new thread, and the pipes to talk between the new child thread
>+   and this, the parent thread.  Return 0 on success, -1 on error.  */
>+
>+int
>+create_thread (void *(*func) (void *),
>+	       struct thread_info *info)
>+{
>+  if (pipe (info->to_thread) < 0)
>+    return -1;
>+  if (pipe (info->to_parent) < 0)
>+    return -1;
>+
>+  if (pthread_create (&info->id, NULL, func, info) != 0)
>+    return -1;
>+
>+  return 0;
>+}
>+
>+/* Join with the child thread identified by INFO.  Return 0 on success, -1
>+   on error.  */
>+
>+int
>+join_thread (struct thread_info *info)
>+{
>+  if (pthread_join (info->id, NULL) != 0)
>+    return -1;
>+
>+  return 0;
>+}
>+
>+/* An empty function, place breakpoints on this.  */
>+
>+void
>+breakpt ()
>+{
>+  /* Nothing.  */
>+}
>+
>+/* The thread worker function.  */
>+
>+void *
>+thread_func (void *arg)
>+{
>+  struct thread_info *info = (struct thread_info *) arg;
>+
>+  /* (1) Send to the parent thread.  */
>+  puts ("(1) child thread: send to parent");
>+  send_byte (info->to_parent);
>+
>+  /* (2) Wait for the parent thread.  */
>+  puts ("(2) child thread: wait for byte.");
>+  wait_for_byte (info->to_thread);
>+
>+  /* (3) Send to the parent thread.  */
>+  puts ("(3) child thread: send to parent");
>+  send_byte (info->to_parent);
>+
>+  /* (4) Wait for the parent thread.  */
>+  puts ("(4) child thread: wait for byte.");
>+  wait_for_byte (info->to_thread);
>+
>+  /* (5) Send to the parent thread.  */
>+  puts ("(5) child thread: send to parent");
>+  send_byte (info->to_parent);
>+
>+  return NULL;
>+}
>+
>+/* The main program entry point.  */
>+
>+int
>+main ()
>+{
>+  struct thread_info info;
>+
>+  puts ("(0) parent thread: creating child thread.");
>+  if (create_thread (thread_func, &info) < 0)
>+    abort ();
>+
>+  /* (1) Wait for child thread.  */
>+  puts ("(1) parent thread: wait for child.");
>+  wait_for_byte (info.to_parent);
>+
>+ /* If we stop at a breakpoint now then the child thread will be running
>+     while the breakpoint condition is evaluated.  */
>+  puts("    parent thread: calling breakpt.");
>+  breakpt ();
>+
>+  /* Now we want to make sure that the child thread is still running
>+     correctly, and GDB hasn't accidentally left the thread stopped.  */
>+
>+  /* (2) Send to child thread.  */
>+  puts("(2) parent thread: sending byte to child thread.");
>+  send_byte (info.to_thread);
>+
>+  /* (3) Wait for child thread.  */
>+  puts ("(3) parent thread: wait for child.");
>+  wait_for_byte (info.to_parent);
>+
>+  /* We now know that the child is alive and running.  Call breakpoint
>+     function again, this time the global value is such that the breakpoint
>+     should trigger.  */
>+
>+  global_value = 1;
>+  puts("    parent thread: calling breakpt.");
>+  breakpt ();
>+
>+  /* Once again, we want to ensure that the child is alive and kicking .  */
>+
>+  /* (4) Send to child thread.  */
>+  puts("(4) parent thread: sending byte to child thread.");
>+  send_byte (info.to_thread);
>+
>+  /* (5) Wait for child thread.  */
>+  puts ("(5) parent thread: wait for child.");
>+  wait_for_byte (info.to_parent);
>+
>+  puts ("Joining with child thread.");
>+
>+  if (join_thread (&info) < 0)
>+    abort ();
>+
>+  return 0;
>+}
>diff --git a/gdb/testsuite/gdb.threads/all-stop-infcall.exp b/gdb/testsuite/gdb.threads/all-stop-infcall.exp
>new file mode 100644
>index 00000000000..2668f8ce202
>--- /dev/null
>+++ b/gdb/testsuite/gdb.threads/all-stop-infcall.exp
>@@ -0,0 +1,109 @@
>+# Copyright 2022 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/>.
>+
>+standard_testfile
>+
>+if {[prepare_for_testing "failed to prepare" \
>+	 $testfile $srcfile {debug pthreads}] == -1} {
>+    return -1
>+}
>+
>+if ![runto_main] then {
>+    return 0
>+}
>+
>+# Check that we have two threads, and that neither thread is running.
>+proc check_threads { } {
>+
>+    array set threads {}
>+
>+    gdb_test_multiple "info threads" "" {
>+	-re "^info threads\r\n" {
>+	    verbose -log "APB: Saw command"
>+	    exp_continue
>+	}
>+
>+	-re "^\\s+Id\\s+Target Id\\s+Frame\\s+\r\n" {
>+	    verbose -log "APB: Saw header line"
>+	    exp_continue
>+	}
>+
>+	-re "^((\\*| )\\s+($::decimal)\\s+\[^\r\n\]+)\r\n" {
>+	    set status $expect_out(1,string)
>+	    set num $expect_out(3,string)
>+	    set threads($num) $status
>+	    verbose -log "APB: Saw thread $num line: $status"
>+	    exp_continue
>+	}
>+
>+	-re "$::gdb_prompt $" {
>+	    pass $gdb_test_name
>+	}
>+    }
>+
>+    # Check that we saw both threads, and that neither thread is in
>+    # the running state.
>+    foreach num { 1 2 } {
>+	gdb_assert [info exists threads($num)] "check thread $num was found"
>+
>+	gdb_assert {[string first "(running)" $threads($num)] == -1} \
>+	    "thread $num is not running"
>+    }
>+}
>+
>+# In this first test we call an inferior function from a breakpoint
>+# condition.  The first time the condition will evaluate to false, and
>+# the second time the condition evaluates to true.  The second
>+# breakpoint location will only be hit if the other thread keeps
>+# running after the breakpoint condition is evaluated.
>+
>+with_test_prefix "basic case" {
>+    gdb_breakpoint "breakpt if get_global_value () > 0"
>+    gdb_continue_to_breakpoint "breakpt"
>+    gdb_test "p global_value" " = 1"
>+    check_threads
>+}
>+
>+# In the next test we call an inferior function that segfaults from a
>+# breakpoint condition.
>+
>+with_test_prefix "with_segfault" {
>+
>+    clean_restart ${binfile}
>+
>+    if ![runto_main] then {
>+	return 0
>+    }
>+
>+    gdb_breakpoint "breakpt if function_that_segfaults ()"
>+
>+    set lineno [gdb_get_line_number "Segfault happens here"]
>+    gdb_test "continue" \
>+	[multi_line \
>+	     "Error in testing breakpoint condition:" \
>+	     "The program being debugged was signaled while in a function called from GDB." \
>+	     "GDB remains in the frame where the signal was received." \
>+	     "To change this behavior use \"set unwindonsignal on\"\\." \
>+	     "Evaluation of the expression containing the function" \
>+	     "\$function_that_segfaults\$ will be abandoned." \
>+	     "When the function is done executing, GDB will silently stop." \
>+	     "" \
>+	     "Thread 1 \"all-stop-infcal\" received signal SIGSEGV, Segmentation fault\\." \
>+	     "" \
>+	     "Thread 1 \"all-stop-infcal\" hit Breakpoint $decimal, $hex in function_that_segfaults \$\$ at \[^\r\n\]+/all-stop-infcall.c:${lineno}" \
>+	     "${lineno}\\s+\\*p = 1;\[^\r\n\]+"]
>+
>+    check_threads
>+}

Actions: View | Diff

Attachments on bug 28942: 14005