public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v3 2/2] gdb: raise and handle NOT_AVAILABLE_ERROR when accessing frame PC
Date: Wed, 23 Mar 2022 14:05:58 +0100	[thread overview]
Message-ID: <da98ab3367dcaf78f93ffe22fc8575d6f3b8aa58.1648040449.git.tankut.baris.aktemur@intel.com> (raw)
In-Reply-To: <cover.1648040449.git.tankut.baris.aktemur@intel.com>

This patch can be considered a continuation of

  commit 4778a5f87d253399083565b4919816f541ebe414
  Author: Tom de Vries <tdevries@suse.de>
  Date:   Tue Apr 21 15:45:57 2020 +0200

    [gdb] Fix hang after ext sigkill

and

  commit 47f1aceffa02be4726b854082d7587eb259136e0
  Author: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
  Date:   Thu May 14 13:59:54 2020 +0200

    gdb/infrun: handle already-exited threads when attempting to stop

If a process dies before GDB reports the exit error to the user, we
may see the "Couldn't get registers: No such process." error message
in various places.  For instance:

  (gdb) start
  ...
  (gdb) info inferior
    Num  Description       Connection           Executable
  * 1    process 31943     1 (native)           /tmp/a.out
  (gdb) shell kill -9 31943
  (gdb) maintenance flush register-cache
  Register cache flushed.
  Couldn't get registers: No such process.
  (gdb) info threads
    Id   Target Id              Frame
  * 1    process 31943 "a.out" Couldn't get registers: No such process.
  (gdb) backtrace
  Python Exception <class 'gdb.error'>: Couldn't get registers: No such process.
  Couldn't get registers: No such process.
  (gdb) inferior 1
  Couldn't get registers: No such process.
  (gdb) thread
  [Current thread is 1 (process 31943)]
  Couldn't get registers: No such process.
  (gdb)

The gdb.threads/killed-outside.exp, gdb.multi/multi-kill.exp, and
gdb.multi/multi-exit.exp tests also check related scenarios.

To improve the situation,

1. when printing the frame info, catch and process a NOT_AVAILABLE_ERROR.

2. when accessing the target to fetch registers, if the operation
   fails, raise a NOT_AVAILABLE_ERROR instead of a generic error, so
   that clients can attempt to recover accordingly.  This patch updates
   the amd64_linux_nat_target and remote_target in this direction.

With this patch, we obtain the following behavior:

  (gdb) start
  ...
  (gdb) info inferior
    Num  Description       Connection           Executable
  * 1    process 748       1 (native)           /tmp/a.out
  (gdb) shell kill -9 748
  (gdb) maintenance flush register-cache
  Register cache flushed.
  (gdb) info threads
    Id   Target Id           Frame
  * 1    process 748 "a.out" <PC register is not available>
  (gdb) backtrace
  #0  <PC register is not available>
  Backtrace stopped: not enough registers or memory available to unwind further
  (gdb) inferior 1
  [Switching to inferior 1 [process 748] (/tmp/a.out)]
  [Switching to thread 1 (process 748)]
  #0  <PC register is not available>
  (gdb) thread
  [Current thread is 1 (process 748)]
  (gdb)

Here is another "before/after" case.  Suppose we have two inferiors,
each having its own remote target underneath.  Before this patch, we
get the following output:

  # Create two inferiors on two remote targets, resume both until
  # termination.  Exit event from one of them is shown first, but the
  # other also exited -- just not yet shown.
  (gdb) maint set target-non-stop on
  (gdb) target remote | gdbserver - ./a.out
  (gdb) add-inferior -no-connection
  (gdb) inferior 2
  (gdb) target remote | gdbserver - ./a.out
  (gdb) set schedule-multiple on
  (gdb) continue
  ...
  [Inferior 2 (process 22127) exited normally]
  (gdb) inferior 1
  [Switching to inferior 1 [process 22111] (target:/tmp/a.out)]
  [Switching to thread 1.1 (Thread 22111.22111)]
  Could not read registers; remote failure reply 'E01'
  (gdb) info threads
    Id   Target Id                  Frame
  * 1.1  Thread 22111.22111 "a.out" Could not read registers; remote failure reply 'E01'
  (gdb) backtrace
  Python Exception <class 'gdb.error'>: Could not read registers; remote failure reply 'E01'
  Could not read registers; remote failure reply 'E01'
  (gdb) thread
  [Current thread is 1.1 (Thread 22111.22111)]
  Could not read registers; remote failure reply 'E01'
  (gdb)

With this patch, it becomes:

  ...
  [Inferior 1 (process 11759) exited normally]
  (gdb) inferior 2
  [Switching to inferior 2 [process 13440] (target:/path/to/a.out)]
  [Switching to thread 2.1 (Thread 13440.13440)]
  #0  <unavailable> in ?? ()
  (gdb) info threads
    Id   Target Id                   Frame
  * 2.1  Thread 13440.13440 "a.out" <unavailable> in ?? ()
  (gdb) backtrace
  #0  <unavailable> in ?? ()
  Backtrace stopped: not enough registers or memory available to unwind further
  (gdb) thread
  [Current thread is 2.1 (Thread 13440.13440)]
  (gdb)

Finally, together with its predecessor, this patch also fixes PR gdb/26877.

Regression-tested on X86_64-Linux.
---
 gdb/amd64-linux-nat.c                         |  3 +-
 gdb/remote.c                                  | 20 +++--
 gdb/stack.c                                   | 33 +++++++-
 gdb/testsuite/gdb.threads/killed-outside.exp  |  7 +-
 .../gdb.tui/multi-exit-remove-inferior.c      | 21 +++++
 .../gdb.tui/multi-exit-remove-inferior.exp    | 80 +++++++++++++++++++
 6 files changed, 152 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/multi-exit-remove-inferior.c
 create mode 100644 gdb/testsuite/gdb.tui/multi-exit-remove-inferior.exp

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 3d28d7e1d57..bdcc93eb498 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -222,7 +222,8 @@ amd64_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
       elf_gregset_t regs;
 
       if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
-	perror_with_name (_("Couldn't get registers"));
+	throw_perror_with_name (NOT_AVAILABLE_ERROR,
+				_("Couldn't get registers"));
 
       amd64_supply_native_gregset (regcache, &regs, -1);
       if (regnum != -1)
diff --git a/gdb/remote.c b/gdb/remote.c
index aa6a67a96e0..609f6a65c2f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8384,10 +8384,14 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
     case PACKET_UNKNOWN:
       return 0;
     case PACKET_ERROR:
-      error (_("Could not fetch register \"%s\"; remote failure reply '%s'"),
-	     gdbarch_register_name (regcache->arch (), 
-				    reg->regnum), 
-	     buf);
+      {
+	const char *regname = gdbarch_register_name (regcache->arch (),
+						     reg->regnum);
+	std::string msg
+	  = string_printf (_("Could not fetch register \"%s\"; remote "
+			     "failure reply '%s'"), regname, buf);
+	throw_perror_with_name (NOT_AVAILABLE_ERROR, msg.c_str ());
+      }
     }
 
   /* If this register is unfetchable, tell the regcache.  */
@@ -8424,8 +8428,12 @@ remote_target::send_g_packet ()
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
   if (packet_check_result (rs->buf) == PACKET_ERROR)
-    error (_("Could not read registers; remote failure reply '%s'"),
-	   rs->buf.data ());
+    {
+      std::string msg
+	= string_printf (_("Could not read registers; remote failure reply "
+			   "'%s'"), rs->buf.data ());
+      throw_perror_with_name (NOT_AVAILABLE_ERROR, msg.c_str ());
+    }
 
   /* We can get out of synch in various cases.  If the first character
      in the buffer is not a hex character, assume that has happened
diff --git a/gdb/stack.c b/gdb/stack.c
index 10da88b88e5..b1a61d5a612 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1107,7 +1107,38 @@ print_frame_info (const frame_print_options &fp_opts,
      the next frame is a SIGTRAMP_FRAME or a DUMMY_FRAME, then the
      next frame was not entered as the result of a call, and we want
      to get the line containing FRAME->pc.  */
-  symtab_and_line sal = find_frame_sal (frame);
+  symtab_and_line sal;
+  try
+    {
+      sal = find_frame_sal (frame);
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      if (ex.error == NOT_AVAILABLE_ERROR)
+	{
+	  ui_out_emit_tuple tuple_emitter (uiout, "frame");
+
+	  annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
+				gdbarch, 0);
+
+	  if (print_level)
+	    {
+	      uiout->text ("#");
+	      uiout->field_fmt_signed (2, ui_left, "level",
+				       frame_relative_level (frame));
+	    }
+
+	  std::string frame_info = "<";
+	  frame_info += ex.what ();
+	  frame_info += ">";
+	  uiout->field_string ("func", frame_info.c_str (),
+			       metadata_style.style ());
+	  uiout->text ("\n");
+	  annotate_frame_end ();
+	  return;
+	}
+      throw;
+    }
 
   location_print = (print_what == LOCATION
 		    || print_what == SRC_AND_LOC
diff --git a/gdb/testsuite/gdb.threads/killed-outside.exp b/gdb/testsuite/gdb.threads/killed-outside.exp
index 3d4543e088c..8e156c863dc 100644
--- a/gdb/testsuite/gdb.threads/killed-outside.exp
+++ b/gdb/testsuite/gdb.threads/killed-outside.exp
@@ -37,16 +37,15 @@ remote_exec target "kill -9 ${testpid}"
 # Give it some time to die.
 sleep 2
 
-set no_such_process_msg "Couldn't get registers: No such process\."
+set no_pc_available_msg "PC register is not available"
 set killed_msg "Program terminated with signal SIGKILL, Killed\."
 set no_longer_exists_msg "The program no longer exists\."
 set not_being_run_msg "The program is not being run\."
 
 gdb_test_multiple "continue" "prompt after first continue" {
-    -re "Continuing\.\r\n$no_such_process_msg\r\n$gdb_prompt " {
+    -re "Continuing\.\r\n$no_pc_available_msg\r\n$gdb_prompt " {
 	pass $gdb_test_name
-	# Saw $no_such_process_msg.  The bug condition was triggered, go
-	# check for it.
+	# The bug condition was triggered, go check for it.
 	gdb_test_multiple "" "messages" {
 	    -re ".*$killed_msg.*$no_longer_exists_msg\r\n" {
 		pass $gdb_test_name
diff --git a/gdb/testsuite/gdb.tui/multi-exit-remove-inferior.c b/gdb/testsuite/gdb.tui/multi-exit-remove-inferior.c
new file mode 100644
index 00000000000..d934d74fbb7
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/multi-exit-remove-inferior.c
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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/>.  */
+
+int main() {
+  int a = 42;
+  return 0; /* break-here */
+}
diff --git a/gdb/testsuite/gdb.tui/multi-exit-remove-inferior.exp b/gdb/testsuite/gdb.tui/multi-exit-remove-inferior.exp
new file mode 100644
index 00000000000..10eb69500cc
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/multi-exit-remove-inferior.exp
@@ -0,0 +1,80 @@
+# Copyright 2020 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/>.
+#
+# This is a regression test for PR gdb/26877.
+
+tuiterm_env
+
+standard_testfile
+
+if {[use_gdb_stub]} {
+    return 0
+}
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+# Make sure TUI is supported before continuing.
+with_test_prefix "initial check" {
+    Term::clean_restart 24 80 $testfile
+    if {![Term::enter_tui]} {
+	unsupported "TUI not supported"
+	return
+    }
+}
+
+Term::clean_restart 24 80 $testfile
+
+# Create a setting with two inferiors, where both are stopped
+# at a breakpoint at the end of main.  Then resume both.
+set bp [gdb_get_line_number "break-here"]
+gdb_breakpoint "$bp"
+
+with_test_prefix "inferior 1" {
+    gdb_run_cmd
+    gdb_test "" ".*reakpoint \[^\r\n\]+${srcfile}.*" "run until bp"
+}
+
+with_test_prefix "inferior 2" {
+    gdb_test "add-inferior -exec [standard_output_file $testfile]" \
+	"Added inferior 2.*" "add inferior"
+    gdb_test "inferior 2" "Switching to inferior 2.*" "switch"
+    gdb_run_cmd
+    gdb_test "" ".*reakpoint \[^\r\n\]+${srcfile}.*" "run until bp"
+}
+
+gdb_test_no_output "set schedule-multiple on"
+gdb_continue_to_end
+
+# Find out which inferior is current.  It is the inferior that exited.
+set exited_inf [get_integer_valueof {$_inferior} 1 "current inf"]
+
+# Switch to the other inferior and remove the exited one.
+# Bad GDB used to crash when this is done under TUI.
+if {![Term::enter_tui]} {
+    unsupported "TUI not supported"
+    return
+}
+
+if {$exited_inf == 1} {
+    Term::command "inferior 2"
+} else {
+    Term::command "inferior 1"
+}
+
+Term::command "remove-inferiors $exited_inf"
+Term::command "info inferior $exited_inf"
+Term::check_contents "inferior is removed" "No inferiors."
-- 
2.33.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


  parent reply	other threads:[~2022-03-23 13:07 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-08  9:15 [PATCH v2 0/2] Querying registers of already-exited processes Tankut Baris Aktemur
2022-02-08  9:15 ` [PATCH v2 1/2] gdb/regcache: return REG_UNAVAILABLE if raw_update raises NOT_AVAILABLE_ERROR Tankut Baris Aktemur
2022-03-16 15:18   ` Bruno Larsen
2022-03-23 12:55     ` Aktemur, Tankut Baris
2022-02-08  9:15 ` [PATCH v2 2/2] gdb: raise and handle NOT_AVAILABLE_ERROR when accessing frame PC Tankut Baris Aktemur
2022-03-16 17:26   ` Bruno Larsen
2022-03-23 12:55     ` Aktemur, Tankut Baris
2022-03-23 13:34       ` Bruno Larsen
2022-03-24  8:46         ` Aktemur, Tankut Baris
2022-02-22  7:31 ` [PATCH v2 0/2] Querying registers of already-exited processes Aktemur, Tankut Baris
2022-03-07  8:00 ` Aktemur, Tankut Baris
2022-03-23 13:05 ` [PATCH v3 " Tankut Baris Aktemur
2022-03-23 13:05   ` [PATCH v3 1/2] gdb/regcache: return REG_UNAVAILABLE in raw_read if NOT_AVAILABLE_ERROR is seen Tankut Baris Aktemur
2022-03-23 13:05   ` Tankut Baris Aktemur [this message]
2022-05-04  7:19   ` [PATCH v3 0/2] Querying registers of already-exited processes Aktemur, Tankut Baris
2022-12-23 17:10     ` Aktemur, Tankut Baris
2023-01-17 20:40       ` Aktemur, Tankut Baris
2023-01-24 10:35       ` Aktemur, Tankut Baris
2023-01-31 20:14       ` Aktemur, Tankut Baris
2023-02-20 13:07       ` Aktemur, Tankut Baris
2023-03-03  7:46       ` Aktemur, Tankut Baris
2023-03-28 13:40       ` Aktemur, Tankut Baris
2023-12-18 14:40   ` [PATCH v4 " Tankut Baris Aktemur
2023-12-18 14:40     ` [PATCH v4 1/2] gdb/regcache: return REG_UNAVAILABLE in raw_read if NOT_AVAILABLE_ERROR is seen Tankut Baris Aktemur
2023-12-18 14:40     ` [PATCH v4 2/2] gdb: raise and handle NOT_AVAILABLE_ERROR when accessing frame PC Tankut Baris Aktemur
2023-12-20 22:00       ` John Baldwin
2023-12-21  6:41         ` Eli Zaretskii
2023-12-27 18:41           ` Aktemur, Tankut Baris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=da98ab3367dcaf78f93ffe22fc8575d6f3b8aa58.1648040449.git.tankut.baris.aktemur@intel.com \
    --to=tankut.baris.aktemur@intel.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).