public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Stop notifications when unwindonsignal is on
@ 2023-08-08 14:45 Andrew Burgess
  2023-08-08 14:45 ` [PATCH 1/2] gdb/testsuite: add mi_info_frame helper proc (and use it) Andrew Burgess
  2023-08-08 14:45 ` [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on Andrew Burgess
  0 siblings, 2 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-08-08 14:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, Andrew Burgess

First patch just adds some new testsuite infrastructure.  Second patch
is where the interesting work is done.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb/testsuite: add mi_info_frame helper proc (and use it)
  gdb: MI stopped events when unwindonsignal is on

 gdb/infcall.c                                |  52 ++++++--
 gdb/testsuite/gdb.base/unwindonsignal.exp    |   7 +-
 gdb/testsuite/gdb.compile/compile-cplus.exp  |  14 ++-
 gdb/testsuite/gdb.compile/compile.exp        |  14 ++-
 gdb/testsuite/gdb.cp/gdb2495.exp             |   7 +-
 gdb/testsuite/gdb.mi/mi-cmd-user-context.exp |  12 +-
 gdb/testsuite/gdb.mi/mi-condbreak-fail.exp   | 118 +++++++++++++------
 gdb/testsuite/gdb.mi/mi-condbreak-throw.cc   |  38 ++++++
 gdb/testsuite/gdb.mi/mi-condbreak-throw.exp  | 116 ++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-stack.exp            |   7 +-
 gdb/testsuite/lib/mi-support.exp             |  52 ++++++++
 11 files changed, 375 insertions(+), 62 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-condbreak-throw.exp


base-commit: 066c738ec5d803a080e3da3b0936484a10c8f31b
-- 
2.25.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] gdb/testsuite: add mi_info_frame helper proc (and use it)
  2023-08-08 14:45 [PATCH 0/2] Stop notifications when unwindonsignal is on Andrew Burgess
@ 2023-08-08 14:45 ` Andrew Burgess
  2023-08-08 14:45 ` [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on Andrew Burgess
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-08-08 14:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, Andrew Burgess

New helper proc mi_info_frame which takes care of running the MI
-stack-info-frame command and matching its output.

Like the breakpoint helper procs, this new proc takes a name/value
argument list and uses this to build the expected result regexp.  This
means that we can now write something like:

  mi_info_frame "test name here" \
    -level 0 -func name -line 123

Instead of the current equivalent:

  mi_gdb_test "235-stack-info-frame" \
    "235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"name\",file=\".*\",fullname=\".*\",line=\"123\",arch=\".*\"\}" \
    "test name here"

There's also a helper proc mi_make_info_frame_regexp which is
responsible for building the 'frame={...}' part of the pattern.

I've update the two existing tests that use -stack-info-frame and
expect the command to succeed.  There is another test that runs
-stack-info-frame and expects the command to fail -- the helper proc
doesn't help with this case, so that test is not changed.
---
 gdb/testsuite/gdb.mi/mi-cmd-user-context.exp | 12 ++---
 gdb/testsuite/gdb.mi/mi-stack.exp            |  7 +--
 gdb/testsuite/lib/mi-support.exp             | 52 ++++++++++++++++++++
 3 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
index 6abdb94addd..b6877b6e0cd 100644
--- a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
+++ b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
@@ -84,9 +84,9 @@ mi_gdb_test "frame" \
 
 # Ask about a different frame in the current thread, the current frame
 # should not change.
-mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \
-    "\\^done,frame=\{level=\"1\".*" \
-    "-stack-info-frame 1"
+mi_info_frame "-stack-info-frame 1" \
+    -thread 2 -frame 1 \
+    -level 1
 
 mi_gdb_test "thread" \
     ".*Current thread is 2.*" \
@@ -99,9 +99,9 @@ mi_gdb_test "frame" \
 
 # Ask about a different frame in a different thread.  After this the
 # current thread and frame should not have changed.
-mi_gdb_test "-stack-info-frame --thread 3 --frame 1" \
-    "\\^done,frame=\{level=\"1\".*" \
-    "-stack-info-frame 2"
+mi_info_frame "-stack-info-frame 2" \
+    -thread 3 -frame 1 \
+    -level 1
 
 mi_gdb_test "thread" \
     ".*Current thread is 2.*" \
diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp
index 777a425894f..592a3cfaba4 100644
--- a/gdb/testsuite/gdb.mi/mi-stack.exp
+++ b/gdb/testsuite/gdb.mi/mi-stack.exp
@@ -65,9 +65,10 @@ proc test_stack_frame_listing {} {
 	"234\\^error,msg=\"-stack-list-frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
 	"stack frame listing wrong"
 
-    mi_gdb_test "235-stack-info-frame" \
-	"235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*${srcfile}\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$callee4_begin\",arch=\"$any\"\}" \
-	"selected frame listing"
+    mi_info_frame "selected frame listing" \
+	-level 0 -addr $hex -func callee4 -file ".*${srcfile}" \
+	-fullname "${fullname_syntax}${srcfile}" -line $callee4_begin \
+	-arch $any
 
     mi_gdb_test "236-stack-list-frames 1 300" \
 	"236\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\}\\\]" \
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 49d5e2ef272..a1dbefe2e8d 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2629,6 +2629,58 @@ proc mi_make_breakpoint_1 {attr_list thread cond evaluated-by times \
 }
 
 
+# Construct a regexp to match against a frame description as returned by
+# -stack-info-frame.  Take a list of name value pairs.  Valid names are
+# -level, -addr, -func, -file, -fullname, line, and -arch, each takes a
+# regexp which is matched against the corresponding field in the
+# -stack-info-frame output.
+
+proc mi_make_info_frame_regexp {args} {
+    parse_args [list [list level "$::decimal"] [list addr "$::hex"] \
+		    {func .*} {file .*} {fullname .*} \
+		    [list line "$::decimal"] {arch .*}]
+
+    set attr_list {}
+    foreach attr {level addr func file fullname line arch} {
+	lappend attr_list $attr [set $attr]
+    }
+
+    set result "frame=\\\{"
+    append result [mi_build_kv_pairs $attr_list]
+    append result "\\\}"
+    return $result
+}
+
+# Run the -stack-info-frame command and match the result, return true if the
+# test passes, otherwise, return false.
+#
+# TEST is the name used for this test.
+#
+# ARGS is an optional list of name value pairs, the names -frame and -thread
+# if present, expect a decimal argument and control the frame and thread for
+# which -stack-info-frame is run.  If -frame is not given then the
+# -stack-info-frame will operate on the current frame.  If -thread is not
+# given then -stack-info-frame will operate on the current thread.
+#
+# The remaining arguments are passed to mi_make_frame_regexp and are used to
+# build the regexp for matching against the -stack-info-frame output.
+
+proc mi_info_frame { test args } {
+    parse_args {{frame ""} {thread ""}}
+
+    set re [eval mi_make_info_frame_regexp $args]
+
+    set cmd "235-stack-info-frame"
+    if {$frame ne ""} {
+	append cmd " --frame ${frame}"
+    }
+    if {$thread ne ""} {
+	append cmd " --thread ${thread}"
+    }
+
+    return [mi_gdb_test $cmd "235\\^done,$re" $test]
+}
+
 # Construct a breakpoint regexp, for a breakpoint with multiple
 # locations.  This may be used to test the output of -break-insert,
 # -dprintf-insert, or -break-info with breakpoints with multiple
-- 
2.25.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-08 14:45 [PATCH 0/2] Stop notifications when unwindonsignal is on Andrew Burgess
  2023-08-08 14:45 ` [PATCH 1/2] gdb/testsuite: add mi_info_frame helper proc (and use it) Andrew Burgess
@ 2023-08-08 14:45 ` Andrew Burgess
  2023-08-10 15:06   ` Thiago Jung Bauermann
  2023-08-29 16:37   ` Simon Marchi
  1 sibling, 2 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-08-08 14:45 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, Andrew Burgess

This recent commit:

  commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
  Date:   Wed Jul 12 21:56:50 2023 +0100

      gdb: avoid double stop after failed breakpoint condition check

Addressed a problem where two MI stopped events would be reported if a
breakpoint condition failed due to a signal, this commit was a
replacement for this commit:

  commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
  Date:   Fri Oct 14 14:53:15 2022 +0100

      gdb: don't always print breakpoint location after failed condition check

which solved the two stop problem, but only for the CLI.  Before both
of these commits, if a b/p condition failed due to a signal then the
user would see two stops reported, the first at the location where the
signal occurred, and the second at the location of the breakpoint.

By default GDB remains at the location where the signal occurred, so
the second reported stop can be confusing, this is the problem that
commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
extended also address the issue for MI too.

However, while working on another patch I realised that there was a
problem with GDB after the above commits.  Neither of the above
commits considered 'set unwindonsignal on'.  With this setting on,
when an inferior function call fails with a signal GDB will unwind the
stack back to the location where the inferior function call started.
In the b/p case we're looking at, the stop should be reported at the
location of the breakpoint, not at the location where the signal
occurred, and this isn't what happens.

This commit fixes this by ensuring that when unwindonsignal is 'on',
GDB reports a single stop event at the location of the breakpoint,
this fixes things for both CLI and MI.

The function call_thread_fsm::should_notify_stop is called when the
inferior function call completes and GDB is figuring out if the user
should be notified about this stop event by calling normal_stop from
fetch_inferior_event in infrun.c.  If normal_stop is called, then this
notification will be for the location where the inferior call stopped,
which will be the location at which the signal occurred.

Prior to this commit, the only time that normal_stop was not called,
was if the inferior function call completed successfully, this was
controlled by ::should_notify_stop, which only turns false when the
inferior function call has completed successfully.

In this commit I have extended the logic in ::should_notify_stop.  Now
there are three cases in which ::should_notify_stop will return false,
and we will not announce the first stop (by calling normal_stop).
These three reasons are:

  1. If the inferior function call completes successfully, this is
  unchanged behaviour,

  2. If the inferior function call stopped due to a signal and 'set
  unwindonsignal on' is in effect, and

  3. If the inferior function call stopped due to an uncaught C++
  exception, and 'set unwind-on-terminating-exception on' is in
  effect.

However, if we don't call normal_stop then we need to call
async_enable_stdin in call_thread_fsm::should_stop.  Prior to this
commit this was only done for the case where the inferior function
call completed successfully.

In this commit I now call ::should_notify_stop and use this to
determine if we need to call async_enable_stdin.  With this done we
now call async_enable_stdin for each of the three cases listed above,
which means that GDB will exit wait_sync_command_done correctly (see
run_inferior_call in infcall.c).

With these two changes the problem is mostly resolved.  However, the
solution isn't ideal, we've still lost some information.

Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
2e411b8c68eb:

  $ gdb -q /tmp/mi-condbreak-fail \
        -ex 'set unwindonsignal on' \
        -ex 'break 30 if (cond_fail())' \
        -ex 'run'
  Reading symbols from /tmp/mi-condbreak-fail...
  Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
  Starting program: /tmp/mi-condbreak-fail

  Program received signal SIGSEGV, Segmentation fault.
  0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
  24	  return *p;			/* Crash here.  */
  Error in testing breakpoint condition:
  The program being debugged was signaled while in a function called from GDB.
  GDB has restored the context to what it was before the call.
  To change this behavior use "set unwindonsignal off".
  Evaluation of the expression containing the function
  (cond_fail) will be abandoned.

  Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
  30	  global_counter += 1;		/* Set breakpoint here.  */
  (gdb)

In this state we see two stop notifications, the first is where the
signal occurred, while the second is where the breakpoint is located.
As GDB has unwound the stack (thanks to unwindonsignal) the second
stop notification reflects where the inferior is actually located.

Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
to this:

  $ gdb -q /tmp/mi-condbreak-fail \
        -ex 'set unwindonsignal on' \
	-ex 'break 30 if (cond_fail())' \
	-ex 'run'
  Reading symbols from /tmp/mi-condbreak-fail...
  Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
  Starting program: /tmp/mi-condbreak-fail

  Program received signal SIGSEGV, Segmentation fault.
  0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
  24	  return *p;			/* Crash here.  */
  Error in testing condition for breakpoint 1:
  The program being debugged was signaled while in a function called from GDB.
  GDB has restored the context to what it was before the call.
  To change this behavior use "set unwindonsignal off".
  Evaluation of the expression containing the function
  (cond_fail) will be abandoned.
  (gdb) bt 1
  #0  foo () at /tmp/mi-condbreak-fail.c:30
  (More stack frames follow...)
  (gdb)

This is the broken state.  GDB is reports the SIGSEGV location, but
not the unwound breakpoint location.  The final 'bt 1' shows that the
inferior is not located in cond_fail, which is the only location GDB
reported, so this is clearly wrong.

After implementing the fixes described above we now get this
behaviour:

  $ gdb -q /tmp/mi-condbreak-fail \
        -ex 'set unwindonsignal on' \
        -ex 'break 30 if (cond_fail())' \
        -ex 'run'
  Reading symbols from /tmp/mi-condbreak-fail...
  Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
  Starting program: /tmp/mi-condbreak-fail
  Error in testing breakpoint condition for breakpoint 1:
  The program being debugged was signaled while in a function called from GDB.
  GDB has restored the context to what it was before the call.
  To change this behavior use "set unwindonsignal off".
  Evaluation of the expression containing the function
  (cond_fail) will be abandoned.

  Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
  30	  global_counter += 1;		/* Set breakpoint here.  */
  (gdb)

This is better.  GDB now reports a single stop at the location of the
breakpoint, which is where the inferior is actually located.  However,
by removing the first stop notification we have lost some potentially
useful information about which signal caused the inferior to stop.

To address this I've reworked the message that is printed to include
the signal information.  GDB now reports this:

  $ gdb -q /tmp/mi-condbreak-fail \
        -ex 'set unwindonsignal on' \
        -ex 'break 30 if (cond_fail())' \
        -ex 'run'
  Reading symbols from /tmp/mi-condbreak-fail...
  Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
  Starting program: /tmp/mi-condbreak-fail
  Error in testing condition for breakpoint 1:
  The program being debugged received signal SIGSEGV, Segmentation fault
  while in a function called from GDB.  GDB has restored the context
  to what it was before the call.  To change this behavior use
  "set unwindonsignal off".  Evaluation of the expression containing
  the function (cond_fail) will be abandoned.

  Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
  30	  global_counter += 1;		/* Set breakpoint here.  */
  (gdb)

This is better, the user now sees a single stop notification at the
correct location, and the error message describes which signal caused
the inferior function call to stop.

However, we have lost the information about where the signal
occurred.  I did consider trying to include this information in the
error message, but, in the end, I opted not too.  I wasn't sure it was
worth the effort.  If the user has selected to unwind on signal, then
surely this implies they maybe aren't interested in debugging failed
inferior calls, so, hopefully, just knowing the signal name will be
enough.  I figure we can always add this information in later if
there's a demand for it.
---
 gdb/infcall.c                               |  52 +++++++--
 gdb/testsuite/gdb.base/unwindonsignal.exp   |   7 +-
 gdb/testsuite/gdb.compile/compile-cplus.exp |  14 ++-
 gdb/testsuite/gdb.compile/compile.exp       |  14 ++-
 gdb/testsuite/gdb.cp/gdb2495.exp            |   7 +-
 gdb/testsuite/gdb.mi/mi-condbreak-fail.exp  | 118 +++++++++++++-------
 gdb/testsuite/gdb.mi/mi-condbreak-throw.cc  |  38 +++++++
 gdb/testsuite/gdb.mi/mi-condbreak-throw.exp | 116 +++++++++++++++++++
 8 files changed, 313 insertions(+), 53 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-condbreak-throw.exp

diff --git a/gdb/infcall.c b/gdb/infcall.c
index bea5b185ddc..697f8a6291b 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -563,11 +563,20 @@ call_thread_fsm::should_stop (struct thread_info *thread)
 	 registers are restored to what they were before the
 	 call..  */
       return_value = get_call_return_value (&return_meta_info);
+    }
+
+  /* We are always going to stop this thread, but we might not be planning
+     to call call normal_stop, which is only done if should_notify_stop
+     returns true.
+
+     As normal_stop is responsible for calling async_enable_stdin, which
+     would break us out of wait_sync_command_done, then, if we don't plan
+     to call normal_stop, we should call async_enable_stdin here instead.
 
-      /* Break out of wait_sync_command_done.  This is similar to the
-	 async_enable_stdin call in normal_stop (which we don't call),
-	 however, in this case we only change the WAITING_UI.  This is
-	 enough for wait_sync_command_done.  */
+     Unlike normal_stop, we only call async_enable_stdin on WAITING_UI, but
+     that is sufficient for wait_sync_command_done.  */
+  if (!this->should_notify_stop ())
+    {
       scoped_restore save_ui = make_scoped_restore (&current_ui, waiting_ui);
       gdb_assert (current_ui->prompt_state == PROMPT_BLOCKED);
       async_enable_stdin ();
@@ -581,10 +590,28 @@ call_thread_fsm::should_stop (struct thread_info *thread)
 bool
 call_thread_fsm::should_notify_stop ()
 {
+  INFCALL_SCOPED_DEBUG_ENTER_EXIT;
+
   if (finished_p ())
     {
       /* Infcall succeeded.  Be silent and proceed with evaluating the
 	 expression.  */
+      infcall_debug_printf ("inferior call has finished, don't notify");
+      return false;
+    }
+
+  infcall_debug_printf ("inferior call didn't complete fully");
+
+  if (stopped_by_random_signal && unwind_on_signal_p)
+    {
+      infcall_debug_printf ("unwind-on-signal is on, don't notify");
+      return false;
+    }
+
+  if (stop_stack_dummy == STOP_STD_TERMINATE
+      && unwind_on_terminating_exception_p)
+    {
+      infcall_debug_printf ("unwind-on-terminating-exception is on, don't notify");
       return false;
     }
 
@@ -1512,6 +1539,11 @@ When the function is done executing, GDB will silently stop."),
 	    {
 	      /* The user wants the context restored.  */
 
+	      /* Capture details of the signal so we can include them in
+		 the error message.  Calling dummy_frame_pop will restore
+		 the previous stop signal details.  */
+	      gdb_signal stop_signal = call_thread->stop_signal ();
+
 	      /* We must get back to the frame we were before the
 		 dummy call.  */
 	      dummy_frame_pop (dummy_id, call_thread.get ());
@@ -1523,11 +1555,13 @@ When the function is done executing, GDB will silently stop."),
 	      /* FIXME: Insert a bunch of wrap_here; name can be very
 		 long if it's a C++ name with arguments and stuff.  */
 	      error (_("\
-The program being debugged was signaled while in a function called from GDB.\n\
-GDB has restored the context to what it was before the call.\n\
-To change this behavior use \"set unwindonsignal off\".\n\
-Evaluation of the expression containing the function\n\
-(%s) will be abandoned."),
+The program being debugged received signal %s, %s\n\
+while in a function called from GDB.  GDB has restored the context\n\
+to what it was before the call.  To change this behavior use\n\
+\"set unwindonsignal off\".  Evaluation of the expression containing\n\
+the function (%s) will be abandoned."),
+		     gdb_signal_to_name (stop_signal),
+		     gdb_signal_to_string (stop_signal),
 		     name.c_str ());
 	    }
 	  else
diff --git a/gdb/testsuite/gdb.base/unwindonsignal.exp b/gdb/testsuite/gdb.base/unwindonsignal.exp
index 625b0c4db12..5c2243236ba 100644
--- a/gdb/testsuite/gdb.base/unwindonsignal.exp
+++ b/gdb/testsuite/gdb.base/unwindonsignal.exp
@@ -45,7 +45,12 @@ gdb_test "show unwindonsignal" \
 # Call function (causing the program to get a signal), and see if gdb handles
 # it properly.
 if {[gdb_test "call gen_signal ()"  \
-	 "\[\r\n\]*The program being debugged was signaled.*" \
+	 [multi_line \
+	      "The program being debugged received signal SIGABRT, Aborted" \
+	      "while in a function called from GDB\\.  GDB has restored the context" \
+	      "to what it was before the call\\.  To change this behavior use" \
+	      "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	      "the function \\(gen_signal\\) will be abandoned\\."] \
 	 "unwindonsignal, inferior function call signaled"] != 0} {
     return 0
 }
diff --git a/gdb/testsuite/gdb.compile/compile-cplus.exp b/gdb/testsuite/gdb.compile/compile-cplus.exp
index c8a40ada7f3..48fb75c3d78 100644
--- a/gdb/testsuite/gdb.compile/compile-cplus.exp
+++ b/gdb/testsuite/gdb.compile/compile-cplus.exp
@@ -133,7 +133,12 @@ gdb_test "info sym $infcall_pc" "\r\nNo symbol matches .*" "info sym not found"
 
 gdb_test_no_output "set unwindonsignal on"
 gdb_test "compile code *(volatile int *) 0 = 0;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*" \
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\(__gdb_regs\\*\\)\\) will be abandoned\\."] \
     "compile code segfault second"
 
 # C++ Specific tests.
@@ -304,7 +309,12 @@ gdb_test "p globalvar" " = -76" "expect -76"
 
 gdb_test_no_output "set debug compile on"
 gdb_test "compile code static const int readonly = 1; *(int *) &readonly = 2;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*"
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\(__gdb_regs\\*\\)\\) will be abandoned\\."]
 gdb_test_no_output "set debug compile off"
 
 
diff --git a/gdb/testsuite/gdb.compile/compile.exp b/gdb/testsuite/gdb.compile/compile.exp
index 38818d2d255..f2ab4fafa93 100644
--- a/gdb/testsuite/gdb.compile/compile.exp
+++ b/gdb/testsuite/gdb.compile/compile.exp
@@ -159,7 +159,12 @@ gdb_test "info sym $infcall_pc" "\r\nNo symbol matches .*" "info sym not found"
 
 gdb_test_no_output "set unwindonsignal on"
 gdb_test "compile code *(volatile int *) 0 = 0;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*" \
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\) will be abandoned\\."] \
     "compile code segfault second"
 
 gdb_breakpoint [gdb_get_line_number "break-here"]
@@ -312,7 +317,12 @@ gdb_test "p globalvar" " = -76" "expect -76"
 
 gdb_test_no_output "set debug compile on"
 gdb_test "compile code static const int readonly = 1; *(int *) &readonly = 2;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*"
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\) will be abandoned\\."]
 gdb_test_no_output "set debug compile off"
 
 
diff --git a/gdb/testsuite/gdb.cp/gdb2495.exp b/gdb/testsuite/gdb.cp/gdb2495.exp
index e3c0cca3175..93b046ad3a6 100644
--- a/gdb/testsuite/gdb.cp/gdb2495.exp
+++ b/gdb/testsuite/gdb.cp/gdb2495.exp
@@ -108,7 +108,12 @@ gdb_test "show unwindonsignal" \
 # Check to see if new behaviour interferes with
 # normal signal handling in inferior function calls.
 gdb_test "p exceptions.raise_signal(1)" \
-    "To change this behavior use \"set unwindonsignal off\".*" \
+    [multi_line \
+	 "The program being debugged received signal SIGABRT, Aborted" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(SimpleException::raise_signal\\(int\\)\\) will be abandoned\\."]\
     "check for unwindonsignal off message"
 
 # And reverse - turn off again.
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp b/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
index 34be4b907d3..3ccca4c2e9b 100644
--- a/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Check that when GDB fails to evaluate the condition of a conditional
-# breakpoint we only get one *stopped notification.
+# breakpoint we only get one *stopped notification.  In this test case
+# the breakpoint condition fails due to receiving a signal (SIGSEGV).
 
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
@@ -25,43 +26,84 @@ if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
     return -1
 }
 
-if {[mi_clean_restart $binfile]} {
-    return
-}
+# Create a breakpoint with a condition that invokes an inferior
+# function call, that will segfault.  Run until GDB hits the
+# breakpoint and check how GDB reports the failed condition check.
+#
+# UNWIND_ON_SIGNAL is either 'on' or 'off'.  This is used to configure
+# GDB's 'set unwindonsignal' setting.
+
+proc run_test { unwind_on_signal } {
+
+    if {[mi_clean_restart $::binfile]} {
+	return
+    }
+
+    if {[mi_runto_main] == -1} {
+	return
+    }
+
+    mi_gdb_test "-gdb-set unwindonsignal ${unwind_on_signal}" {\^done} \
+	"set unwind-on-signal"
+
+    # Create the conditional breakpoint.
+    set bp_location [gdb_get_line_number "Set breakpoint here"]
+    mi_create_breakpoint "-c \"cond_fail ()\" $::srcfile:$bp_location" \
+	"insert conditional breakpoint" \
+	-func foo -file ".*$::srcfile" -line "$bp_location" \
+	-cond "cond_fail \\(\\)"
 
-if {[mi_runto_main] == -1} {
-    return
+    # Number of the previous breakpoint.
+    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
+		   "get number for breakpoint"]
+
+    # The line where we expect the inferior to crash.
+    set crash_linenum [gdb_get_line_number "Crash here"]
+
+    # Run the inferior and wait for it to stop.
+    mi_send_resuming_command "exec-continue" "continue the inferior"
+
+    if {$unwind_on_signal} {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged received signal SIGSEGV, Segmentation fault\\\\n\"" \
+		 "&\"while in a function called from GDB\\.  GDB has restored the context\\\\n\"" \
+		 "&\"to what it was before the call\\.  To change this behavior use\\\\n\"" \
+		 "&\"\\\\\"set unwindonsignal off\\\\\"\\.  Evaluation of the expression containing\\\\n\"" \
+		 "&\"the function \\(cond_fail\\) will be abandoned\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
+		 "~\"\\\\n\"" \
+		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
+		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func foo -line $bp_location
+    } else {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "~\"\\\\nProgram\"" \
+		 "~\" received signal SIGSEGV, Segmentation fault\\.\\\\n\"" \
+		 "~\"$::hex in cond_fail \\(\\) at \[^\r\n\]+\"" \
+		 "~\"${crash_linenum}\\\\t\\s+return \\*p;\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\"\[^\r\n\]+frame=\\{addr=\"$::hex\",func=\"cond_fail\",args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$crash_linenum\",\[^\r\n\]+\\}\[^\r\n\]+" \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
+		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
+		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function\\\\n\"" \
+		 "&\"\\(cond_fail\\) will be abandoned\\.\\\\n\"" \
+		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func cond_fail -line $crash_linenum
+    }
 }
 
-# Create the conditional breakpoint.
-set bp_location [gdb_get_line_number "Set breakpoint here"]
-mi_create_breakpoint "-c \"cond_fail ()\" $srcfile:$bp_location" \
-    "insert conditional breakpoint" \
-    -func foo -file ".*$srcfile" -line "$bp_location" \
-    -cond "cond_fail \\(\\)"
-
-# Number of the previous breakpoint.
-set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
-	       "get number for breakpoint"]
-
-# The line where we expect the inferior to crash.
-set crash_linenum [gdb_get_line_number "Crash here"]
-
-# Run the inferior and wait for it to stop.
-mi_send_resuming_command "exec-continue" "continue the inferior"
-mi_gdb_test "" \
-    [multi_line \
-	 "~\"\\\\nProgram\"" \
-	 "~\" received signal SIGSEGV, Segmentation fault\\.\\\\n\"" \
-	 "~\"$hex in cond_fail \\(\\) at \[^\r\n\]+\"" \
-	 "~\"${crash_linenum}\\\\t\\s+return \\*p;\[^\r\n\]+\\\\n\"" \
-	 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\"\[^\r\n\]+" \
-	 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
-	 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
-	 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
-	 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
-	 "&\"Evaluation of the expression containing the function\\\\n\"" \
-	 "&\"\\(cond_fail\\) will be abandoned\\.\\\\n\"" \
-	 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
-	 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
-    "wait for stop"
+foreach_with_prefix unwind_on_signal { off on } {
+    run_test $unwind_on_signal
+}
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc b/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
new file mode 100644
index 00000000000..4ce8e72a3c9
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
@@ -0,0 +1,38 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+volatile int global_counter = 0;
+
+int
+cond_throw ()
+{
+  throw 3;
+}
+
+int
+foo ()
+{
+  global_counter += 1;		/* Set breakpoint here.  */
+  return 0;
+}
+
+int
+main ()
+{
+  int res = foo ();
+  return res;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
new file mode 100644
index 00000000000..25fd15e7602
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
@@ -0,0 +1,116 @@
+# Copyright (C) 2023 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/>.
+
+# Check that when GDB fails to evaluate the condition of a conditional
+# breakpoint we only get one *stopped notification.  In this test case
+# the breakpoint condition fails due to throwing an uncaught C++
+# excpetion.
+
+require allow_cplus_tests
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+
+if [build_executable ${testfile}.exp ${binfile} ${srcfile} {debug c++}] {
+    return -1
+}
+
+# Create a breakpoint with a condition that invokes an inferior
+# function call, that will segfault.  Run until GDB hits the
+# breakpoint and check how GDB reports the failed condition check.
+#
+# UNWIND_ON_EXCEPTION is either 'on' or 'off'.  This is used to configure
+# GDB's 'set unwind-on-terminating-exception' setting.
+
+proc run_test { unwind_on_exception } {
+
+    if {[mi_clean_restart $::binfile]} {
+	return
+    }
+
+    if {[mi_runto_main] == -1} {
+	return
+    }
+
+    mi_gdb_test "-gdb-set unwind-on-terminating-exception ${unwind_on_exception}" {\^done} \
+	"set unwind-on-terminating-exception"
+
+    # Create the conditional breakpoint.
+    set bp_location [gdb_get_line_number "Set breakpoint here"]
+    mi_create_breakpoint "-c \"cond_throw ()\" $::srcfile:$bp_location" \
+	"insert conditional breakpoint" \
+	-func "foo\\(\\)" -file ".*$::srcfile" -line "$bp_location" \
+	-cond "cond_throw \\(\\)"
+
+    # Number of the previous breakpoint.
+    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
+		   "get number for breakpoint"]
+
+    # The line where we expect the inferior to crash.
+    set crash_linenum 0
+    #[gdb_get_line_number "Crash here"]
+
+    # Run the inferior and wait for it to stop.
+    mi_send_resuming_command "exec-continue" "continue the inferior"
+
+    if {$unwind_on_exception} {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged entered a std::terminate call, most likely\\\\n\"" \
+		 "&\"caused by an unhandled C\\+\\+ exception.  GDB blocked this call in order\\\\n\"" \
+		 "&\"to prevent the program from being terminated, and has restored the\\\\n\"" \
+		 "&\"context to its original state before the call.\\\\n\"" \
+		 "&\"To change this behaviour use \\\\\"set unwind-on-terminating-exception off\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function \\(cond_throw\\(\\)\\)\\\\n\"" \
+		 "&\"will be abandoned.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
+		 "~\"\\\\n\"" \
+		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
+		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func foo -line $bp_location
+    } else {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "terminate called after throwing an instance of 'int'" \
+		 "~\"\\\\nProgram\"" \
+		 "~\" received signal SIGABRT, Aborted\\.\\\\n\"" \
+		 "~\"$::hex in \[^\r\n\]+\"" \
+		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGABRT\"\[^\r\n\]+frame=\\{addr=\"$::hex\",\[^\r\n\]+\\}\[^\r\n\]+" \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
+		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
+		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function\\\\n\"" \
+		 "&\"\\(cond_throw\\(\\)\\) will be abandoned\\.\\\\n\"" \
+		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
+	    "wait for stop"
+
+	# Don't try to check the current frame here, the inferior will
+	# be stopped somewhere in the C++ runtime at the point where
+	# it is determined that the exception has not been handled.
+    }
+}
+
+foreach_with_prefix unwind_on_exception { off } {
+    run_test $unwind_on_exception
+}
-- 
2.25.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-08 14:45 ` [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on Andrew Burgess
@ 2023-08-10 15:06   ` Thiago Jung Bauermann
  2023-08-14 14:50     ` Andrew Burgess
  2023-08-29 16:37   ` Simon Marchi
  1 sibling, 1 reply; 12+ messages in thread
From: Thiago Jung Bauermann @ 2023-08-10 15:06 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: pedro, gdb-patches


Hello,

This is an area I don't know much about, so in my reading of the code I
didn't find anything to comment. But we are starting to do precommit
testing of GDB patches, and our CI caught a failure in
gdb.mi/mi-condbreak-throw.exp on armv8l-unknown-linux-gnueabihf and
aarch64-linux-gnu:

Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
> new file mode 100644
> index 00000000000..25fd15e7602
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
> @@ -0,0 +1,116 @@
> +# Copyright (C) 2023 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/>.
> +
> +# Check that when GDB fails to evaluate the condition of a conditional
> +# breakpoint we only get one *stopped notification.  In this test case
> +# the breakpoint condition fails due to throwing an uncaught C++
> +# excpetion.
> +
> +require allow_cplus_tests
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile .cc
> +
> +if [build_executable ${testfile}.exp ${binfile} ${srcfile} {debug c++}] {
> +    return -1
> +}
> +
> +# Create a breakpoint with a condition that invokes an inferior
> +# function call, that will segfault.  Run until GDB hits the
> +# breakpoint and check how GDB reports the failed condition check.
> +#
> +# UNWIND_ON_EXCEPTION is either 'on' or 'off'.  This is used to configure
> +# GDB's 'set unwind-on-terminating-exception' setting.
> +
> +proc run_test { unwind_on_exception } {
> +
> +    if {[mi_clean_restart $::binfile]} {
> +	return
> +    }
> +
> +    if {[mi_runto_main] == -1} {
> +	return
> +    }
> +
> +    mi_gdb_test "-gdb-set unwind-on-terminating-exception ${unwind_on_exception}" {\^done} \
> +	"set unwind-on-terminating-exception"
> +
> +    # Create the conditional breakpoint.
> +    set bp_location [gdb_get_line_number "Set breakpoint here"]
> +    mi_create_breakpoint "-c \"cond_throw ()\" $::srcfile:$bp_location" \
> +	"insert conditional breakpoint" \
> +	-func "foo\\(\\)" -file ".*$::srcfile" -line "$bp_location" \
> +	-cond "cond_throw \\(\\)"
> +
> +    # Number of the previous breakpoint.
> +    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
> +		   "get number for breakpoint"]
> +
> +    # The line where we expect the inferior to crash.
> +    set crash_linenum 0
> +    #[gdb_get_line_number "Crash here"]
> +
> +    # Run the inferior and wait for it to stop.
> +    mi_send_resuming_command "exec-continue" "continue the inferior"
> +
> +    if {$unwind_on_exception} {
> +	mi_gdb_test "" \
> +	    [multi_line \
> +		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
> +		 "&\"The program being debugged entered a std::terminate call, most likely\\\\n\"" \
> +		 "&\"caused by an unhandled C\\+\\+ exception.  GDB blocked this call in order\\\\n\"" \
> +		 "&\"to prevent the program from being terminated, and has restored the\\\\n\"" \
> +		 "&\"context to its original state before the call.\\\\n\"" \
> +		 "&\"To change this behaviour use \\\\\"set unwind-on-terminating-exception off\\\\\"\\.\\\\n\"" \
> +		 "&\"Evaluation of the expression containing the function \\(cond_throw\\(\\)\\)\\\\n\"" \
> +		 "&\"will be abandoned.\\\\n\"" \
> +		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
> +		 "~\"\\\\n\"" \
> +		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
> +		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
> +		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
> +	    "wait for stop"
> +
> +	mi_info_frame "check the current frame" \
> +	    -level 0 -func foo -line $bp_location
> +    } else {
> +	mi_gdb_test "" \
> +	    [multi_line \
> +		 "terminate called after throwing an instance of 'int'" \
> +		 "~\"\\\\nProgram\"" \
> +		 "~\" received signal SIGABRT, Aborted\\.\\\\n\"" \
> +		 "~\"$::hex in \[^\r\n\]+\"" \
> +		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGABRT\"\[^\r\n\]+frame=\\{addr=\"$::hex\",\[^\r\n\]+\\}\[^\r\n\]+" \
> +		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
> +		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
> +		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
> +		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
> +		 "&\"Evaluation of the expression containing the function\\\\n\"" \
> +		 "&\"\\(cond_throw\\(\\)\\) will be abandoned\\.\\\\n\"" \
> +		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
> +		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
> +	    "wait for stop"

This test fails:

Expecting: ^([
]+)?(terminate called after throwing an instance of 'int'
~"\\nProgram"
~" received signal SIGABRT, Aborted\.\\n"
~"0x[0-9A-Fa-f]+ in [^
]+"
\*stopped,reason="signal-received",signal-name="SIGABRT"[^
]+frame=\{addr="0x[0-9A-Fa-f]+",[^
]+\}[^
]+
&"Error in testing condition for breakpoint 2:\\n"
&"The program being debugged was signaled while in a function called from GDB\.\\n"
&"GDB remains in the frame where the signal was received\.\\n"
&"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
&"Evaluation of the expression containing the function\\n"
&"\(cond_throw\(\)\) will be abandoned\.\\n"
&"When the function is done executing, GDB will silently stop\.\\n"
=breakpoint-modified,bkpt={number="2",type="breakpoint",[^
]+times="1",[^
]+}[
]+[(]gdb[)] 
[ ]*)
terminate called after throwing an instance of 'int'
~"\nProgram"
~" received signal SIGABRT, Aborted.\n"
~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
&"44\t./nptl/pthread_kill.c: No such file or directory.\n"
*stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842417696"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="78"
&"Error in testing condition for breakpoint 2:\n"
&"The program being debugged was signaled while in a function called from GDB.\n"
&"GDB remains in the frame where the signal was received.\n"
&"To change this behavior use \"set unwindonsignal on\".\n"
&"Evaluation of the expression containing the function\n"
&"(cond_throw()) will be abandoned.\n"
&"When the function is done executing, GDB will silently stop.\n"
=breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaa087c",func="foo()",file="/home/tcwg-build/workspace/tcwg_gnu_5/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/tcwg-build/workspace/tcwg_gnu_5/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
(gdb) 
FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)

It looks like the testcase isn't expecting these lines:

~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
&"44\t./nptl/pthread_kill.c: No such file or directory.\n"

> +
> +	# Don't try to check the current frame here, the inferior will
> +	# be stopped somewhere in the C++ runtime at the point where
> +	# it is determined that the exception has not been handled.
> +    }
> +}
> +
> +foreach_with_prefix unwind_on_exception { off } {
> +    run_test $unwind_on_exception
> +}

-- 
Thiago

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-10 15:06   ` Thiago Jung Bauermann
@ 2023-08-14 14:50     ` Andrew Burgess
  2023-08-15  0:32       ` Thiago Jung Bauermann
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Burgess @ 2023-08-14 14:50 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: pedro, gdb-patches

Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:

> Hello,
>
> This is an area I don't know much about, so in my reading of the code I
> didn't find anything to comment. But we are starting to do precommit
> testing of GDB patches, and our CI caught a failure in
> gdb.mi/mi-condbreak-throw.exp on armv8l-unknown-linux-gnueabihf and
> aarch64-linux-gnu:
>
> Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
>> new file mode 100644
>> index 00000000000..25fd15e7602
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
>> @@ -0,0 +1,116 @@
>> +# Copyright (C) 2023 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/>.
>> +
>> +# Check that when GDB fails to evaluate the condition of a conditional
>> +# breakpoint we only get one *stopped notification.  In this test case
>> +# the breakpoint condition fails due to throwing an uncaught C++
>> +# excpetion.
>> +
>> +require allow_cplus_tests
>> +
>> +load_lib mi-support.exp
>> +set MIFLAGS "-i=mi"
>> +
>> +standard_testfile .cc
>> +
>> +if [build_executable ${testfile}.exp ${binfile} ${srcfile} {debug c++}] {
>> +    return -1
>> +}
>> +
>> +# Create a breakpoint with a condition that invokes an inferior
>> +# function call, that will segfault.  Run until GDB hits the
>> +# breakpoint and check how GDB reports the failed condition check.
>> +#
>> +# UNWIND_ON_EXCEPTION is either 'on' or 'off'.  This is used to configure
>> +# GDB's 'set unwind-on-terminating-exception' setting.
>> +
>> +proc run_test { unwind_on_exception } {
>> +
>> +    if {[mi_clean_restart $::binfile]} {
>> +	return
>> +    }
>> +
>> +    if {[mi_runto_main] == -1} {
>> +	return
>> +    }
>> +
>> +    mi_gdb_test "-gdb-set unwind-on-terminating-exception ${unwind_on_exception}" {\^done} \
>> +	"set unwind-on-terminating-exception"
>> +
>> +    # Create the conditional breakpoint.
>> +    set bp_location [gdb_get_line_number "Set breakpoint here"]
>> +    mi_create_breakpoint "-c \"cond_throw ()\" $::srcfile:$bp_location" \
>> +	"insert conditional breakpoint" \
>> +	-func "foo\\(\\)" -file ".*$::srcfile" -line "$bp_location" \
>> +	-cond "cond_throw \\(\\)"
>> +
>> +    # Number of the previous breakpoint.
>> +    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
>> +		   "get number for breakpoint"]
>> +
>> +    # The line where we expect the inferior to crash.
>> +    set crash_linenum 0
>> +    #[gdb_get_line_number "Crash here"]
>> +
>> +    # Run the inferior and wait for it to stop.
>> +    mi_send_resuming_command "exec-continue" "continue the inferior"
>> +
>> +    if {$unwind_on_exception} {
>> +	mi_gdb_test "" \
>> +	    [multi_line \
>> +		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
>> +		 "&\"The program being debugged entered a std::terminate call, most likely\\\\n\"" \
>> +		 "&\"caused by an unhandled C\\+\\+ exception.  GDB blocked this call in order\\\\n\"" \
>> +		 "&\"to prevent the program from being terminated, and has restored the\\\\n\"" \
>> +		 "&\"context to its original state before the call.\\\\n\"" \
>> +		 "&\"To change this behaviour use \\\\\"set unwind-on-terminating-exception off\\\\\"\\.\\\\n\"" \
>> +		 "&\"Evaluation of the expression containing the function \\(cond_throw\\(\\)\\)\\\\n\"" \
>> +		 "&\"will be abandoned.\\\\n\"" \
>> +		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
>> +		 "~\"\\\\n\"" \
>> +		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
>> +		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
>> +		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
>> +	    "wait for stop"
>> +
>> +	mi_info_frame "check the current frame" \
>> +	    -level 0 -func foo -line $bp_location
>> +    } else {
>> +	mi_gdb_test "" \
>> +	    [multi_line \
>> +		 "terminate called after throwing an instance of 'int'" \
>> +		 "~\"\\\\nProgram\"" \
>> +		 "~\" received signal SIGABRT, Aborted\\.\\\\n\"" \
>> +		 "~\"$::hex in \[^\r\n\]+\"" \
>> +		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGABRT\"\[^\r\n\]+frame=\\{addr=\"$::hex\",\[^\r\n\]+\\}\[^\r\n\]+" \
>> +		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
>> +		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
>> +		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
>> +		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
>> +		 "&\"Evaluation of the expression containing the function\\\\n\"" \
>> +		 "&\"\\(cond_throw\\(\\)\\) will be abandoned\\.\\\\n\"" \
>> +		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
>> +		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
>> +	    "wait for stop"
>
> This test fails:
>
> Expecting: ^([
> ]+)?(terminate called after throwing an instance of 'int'
> ~"\\nProgram"
> ~" received signal SIGABRT, Aborted\.\\n"
> ~"0x[0-9A-Fa-f]+ in [^
> ]+"
> \*stopped,reason="signal-received",signal-name="SIGABRT"[^
> ]+frame=\{addr="0x[0-9A-Fa-f]+",[^
> ]+\}[^
> ]+
> &"Error in testing condition for breakpoint 2:\\n"
> &"The program being debugged was signaled while in a function called from GDB\.\\n"
> &"GDB remains in the frame where the signal was received\.\\n"
> &"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
> &"Evaluation of the expression containing the function\\n"
> &"\(cond_throw\(\)\) will be abandoned\.\\n"
> &"When the function is done executing, GDB will silently stop\.\\n"
> =breakpoint-modified,bkpt={number="2",type="breakpoint",[^
> ]+times="1",[^
> ]+}[
> ]+[(]gdb[)] 
> [ ]*)
> terminate called after throwing an instance of 'int'
> ~"\nProgram"
> ~" received signal SIGABRT, Aborted.\n"
> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
> *stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842417696"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="78"
> &"Error in testing condition for breakpoint 2:\n"
> &"The program being debugged was signaled while in a function called from GDB.\n"
> &"GDB remains in the frame where the signal was received.\n"
> &"To change this behavior use \"set unwindonsignal on\".\n"
> &"Evaluation of the expression containing the function\n"
> &"(cond_throw()) will be abandoned.\n"
> &"When the function is done executing, GDB will silently stop.\n"
> =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaa087c",func="foo()",file="/home/tcwg-build/workspace/tcwg_gnu_5/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/tcwg-build/workspace/tcwg_gnu_5/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
> (gdb) 
> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>
> It looks like the testcase isn't expecting these lines:
>
> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"

Thanks for testing this, and highlighting this failure.

I've included an updated patch below.  I've made the test pattern a
little more general so it should (I hope) handle the lines you are
seeing.

I'd be grateful if you could retest this new version and let me know if
you are still seeing the failure.

Thanks,
Andrew

---

commit e10cc38a93d75401ed659175ae1310b15088d27a
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Aug 8 10:45:20 2023 +0100

    gdb: MI stopped events when unwindonsignal is on
    
    This recent commit:
    
      commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
      Date:   Wed Jul 12 21:56:50 2023 +0100
    
          gdb: avoid double stop after failed breakpoint condition check
    
    Addressed a problem where two MI stopped events would be reported if a
    breakpoint condition failed due to a signal, this commit was a
    replacement for this commit:
    
      commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
      Date:   Fri Oct 14 14:53:15 2022 +0100
    
          gdb: don't always print breakpoint location after failed condition check
    
    which solved the two stop problem, but only for the CLI.  Before both
    of these commits, if a b/p condition failed due to a signal then the
    user would see two stops reported, the first at the location where the
    signal occurred, and the second at the location of the breakpoint.
    
    By default GDB remains at the location where the signal occurred, so
    the second reported stop can be confusing, this is the problem that
    commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
    extended also address the issue for MI too.
    
    However, while working on another patch I realised that there was a
    problem with GDB after the above commits.  Neither of the above
    commits considered 'set unwindonsignal on'.  With this setting on,
    when an inferior function call fails with a signal GDB will unwind the
    stack back to the location where the inferior function call started.
    In the b/p case we're looking at, the stop should be reported at the
    location of the breakpoint, not at the location where the signal
    occurred, and this isn't what happens.
    
    This commit fixes this by ensuring that when unwindonsignal is 'on',
    GDB reports a single stop event at the location of the breakpoint,
    this fixes things for both CLI and MI.
    
    The function call_thread_fsm::should_notify_stop is called when the
    inferior function call completes and GDB is figuring out if the user
    should be notified about this stop event by calling normal_stop from
    fetch_inferior_event in infrun.c.  If normal_stop is called, then this
    notification will be for the location where the inferior call stopped,
    which will be the location at which the signal occurred.
    
    Prior to this commit, the only time that normal_stop was not called,
    was if the inferior function call completed successfully, this was
    controlled by ::should_notify_stop, which only turns false when the
    inferior function call has completed successfully.
    
    In this commit I have extended the logic in ::should_notify_stop.  Now
    there are three cases in which ::should_notify_stop will return false,
    and we will not announce the first stop (by calling normal_stop).
    These three reasons are:
    
      1. If the inferior function call completes successfully, this is
      unchanged behaviour,
    
      2. If the inferior function call stopped due to a signal and 'set
      unwindonsignal on' is in effect, and
    
      3. If the inferior function call stopped due to an uncaught C++
      exception, and 'set unwind-on-terminating-exception on' is in
      effect.
    
    However, if we don't call normal_stop then we need to call
    async_enable_stdin in call_thread_fsm::should_stop.  Prior to this
    commit this was only done for the case where the inferior function
    call completed successfully.
    
    In this commit I now call ::should_notify_stop and use this to
    determine if we need to call async_enable_stdin.  With this done we
    now call async_enable_stdin for each of the three cases listed above,
    which means that GDB will exit wait_sync_command_done correctly (see
    run_inferior_call in infcall.c).
    
    With these two changes the problem is mostly resolved.  However, the
    solution isn't ideal, we've still lost some information.
    
    Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
    2e411b8c68eb:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
    
      Program received signal SIGSEGV, Segmentation fault.
      0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
      24      return *p;                    /* Crash here.  */
      Error in testing breakpoint condition:
      The program being debugged was signaled while in a function called from GDB.
      GDB has restored the context to what it was before the call.
      To change this behavior use "set unwindonsignal off".
      Evaluation of the expression containing the function
      (cond_fail) will be abandoned.
    
      Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
      30      global_counter += 1;          /* Set breakpoint here.  */
      (gdb)
    
    In this state we see two stop notifications, the first is where the
    signal occurred, while the second is where the breakpoint is located.
    As GDB has unwound the stack (thanks to unwindonsignal) the second
    stop notification reflects where the inferior is actually located.
    
    Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
    to this:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
    
      Program received signal SIGSEGV, Segmentation fault.
      0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
      24      return *p;                    /* Crash here.  */
      Error in testing condition for breakpoint 1:
      The program being debugged was signaled while in a function called from GDB.
      GDB has restored the context to what it was before the call.
      To change this behavior use "set unwindonsignal off".
      Evaluation of the expression containing the function
      (cond_fail) will be abandoned.
      (gdb) bt 1
      #0  foo () at /tmp/mi-condbreak-fail.c:30
      (More stack frames follow...)
      (gdb)
    
    This is the broken state.  GDB is reports the SIGSEGV location, but
    not the unwound breakpoint location.  The final 'bt 1' shows that the
    inferior is not located in cond_fail, which is the only location GDB
    reported, so this is clearly wrong.
    
    After implementing the fixes described above we now get this
    behaviour:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
      Error in testing breakpoint condition for breakpoint 1:
      The program being debugged was signaled while in a function called from GDB.
      GDB has restored the context to what it was before the call.
      To change this behavior use "set unwindonsignal off".
      Evaluation of the expression containing the function
      (cond_fail) will be abandoned.
    
      Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
      30      global_counter += 1;          /* Set breakpoint here.  */
      (gdb)
    
    This is better.  GDB now reports a single stop at the location of the
    breakpoint, which is where the inferior is actually located.  However,
    by removing the first stop notification we have lost some potentially
    useful information about which signal caused the inferior to stop.
    
    To address this I've reworked the message that is printed to include
    the signal information.  GDB now reports this:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
      Error in testing condition for breakpoint 1:
      The program being debugged received signal SIGSEGV, Segmentation fault
      while in a function called from GDB.  GDB has restored the context
      to what it was before the call.  To change this behavior use
      "set unwindonsignal off".  Evaluation of the expression containing
      the function (cond_fail) will be abandoned.
    
      Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
      30      global_counter += 1;          /* Set breakpoint here.  */
      (gdb)
    
    This is better, the user now sees a single stop notification at the
    correct location, and the error message describes which signal caused
    the inferior function call to stop.
    
    However, we have lost the information about where the signal
    occurred.  I did consider trying to include this information in the
    error message, but, in the end, I opted not too.  I wasn't sure it was
    worth the effort.  If the user has selected to unwind on signal, then
    surely this implies they maybe aren't interested in debugging failed
    inferior calls, so, hopefully, just knowing the signal name will be
    enough.  I figure we can always add this information in later if
    there's a demand for it.

diff --git a/gdb/infcall.c b/gdb/infcall.c
index bea5b185ddc..697f8a6291b 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -563,11 +563,20 @@ call_thread_fsm::should_stop (struct thread_info *thread)
 	 registers are restored to what they were before the
 	 call..  */
       return_value = get_call_return_value (&return_meta_info);
+    }
+
+  /* We are always going to stop this thread, but we might not be planning
+     to call call normal_stop, which is only done if should_notify_stop
+     returns true.
+
+     As normal_stop is responsible for calling async_enable_stdin, which
+     would break us out of wait_sync_command_done, then, if we don't plan
+     to call normal_stop, we should call async_enable_stdin here instead.
 
-      /* Break out of wait_sync_command_done.  This is similar to the
-	 async_enable_stdin call in normal_stop (which we don't call),
-	 however, in this case we only change the WAITING_UI.  This is
-	 enough for wait_sync_command_done.  */
+     Unlike normal_stop, we only call async_enable_stdin on WAITING_UI, but
+     that is sufficient for wait_sync_command_done.  */
+  if (!this->should_notify_stop ())
+    {
       scoped_restore save_ui = make_scoped_restore (&current_ui, waiting_ui);
       gdb_assert (current_ui->prompt_state == PROMPT_BLOCKED);
       async_enable_stdin ();
@@ -581,10 +590,28 @@ call_thread_fsm::should_stop (struct thread_info *thread)
 bool
 call_thread_fsm::should_notify_stop ()
 {
+  INFCALL_SCOPED_DEBUG_ENTER_EXIT;
+
   if (finished_p ())
     {
       /* Infcall succeeded.  Be silent and proceed with evaluating the
 	 expression.  */
+      infcall_debug_printf ("inferior call has finished, don't notify");
+      return false;
+    }
+
+  infcall_debug_printf ("inferior call didn't complete fully");
+
+  if (stopped_by_random_signal && unwind_on_signal_p)
+    {
+      infcall_debug_printf ("unwind-on-signal is on, don't notify");
+      return false;
+    }
+
+  if (stop_stack_dummy == STOP_STD_TERMINATE
+      && unwind_on_terminating_exception_p)
+    {
+      infcall_debug_printf ("unwind-on-terminating-exception is on, don't notify");
       return false;
     }
 
@@ -1512,6 +1539,11 @@ When the function is done executing, GDB will silently stop."),
 	    {
 	      /* The user wants the context restored.  */
 
+	      /* Capture details of the signal so we can include them in
+		 the error message.  Calling dummy_frame_pop will restore
+		 the previous stop signal details.  */
+	      gdb_signal stop_signal = call_thread->stop_signal ();
+
 	      /* We must get back to the frame we were before the
 		 dummy call.  */
 	      dummy_frame_pop (dummy_id, call_thread.get ());
@@ -1523,11 +1555,13 @@ When the function is done executing, GDB will silently stop."),
 	      /* FIXME: Insert a bunch of wrap_here; name can be very
 		 long if it's a C++ name with arguments and stuff.  */
 	      error (_("\
-The program being debugged was signaled while in a function called from GDB.\n\
-GDB has restored the context to what it was before the call.\n\
-To change this behavior use \"set unwindonsignal off\".\n\
-Evaluation of the expression containing the function\n\
-(%s) will be abandoned."),
+The program being debugged received signal %s, %s\n\
+while in a function called from GDB.  GDB has restored the context\n\
+to what it was before the call.  To change this behavior use\n\
+\"set unwindonsignal off\".  Evaluation of the expression containing\n\
+the function (%s) will be abandoned."),
+		     gdb_signal_to_name (stop_signal),
+		     gdb_signal_to_string (stop_signal),
 		     name.c_str ());
 	    }
 	  else
diff --git a/gdb/testsuite/gdb.base/unwindonsignal.exp b/gdb/testsuite/gdb.base/unwindonsignal.exp
index 625b0c4db12..5c2243236ba 100644
--- a/gdb/testsuite/gdb.base/unwindonsignal.exp
+++ b/gdb/testsuite/gdb.base/unwindonsignal.exp
@@ -45,7 +45,12 @@ gdb_test "show unwindonsignal" \
 # Call function (causing the program to get a signal), and see if gdb handles
 # it properly.
 if {[gdb_test "call gen_signal ()"  \
-	 "\[\r\n\]*The program being debugged was signaled.*" \
+	 [multi_line \
+	      "The program being debugged received signal SIGABRT, Aborted" \
+	      "while in a function called from GDB\\.  GDB has restored the context" \
+	      "to what it was before the call\\.  To change this behavior use" \
+	      "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	      "the function \\(gen_signal\\) will be abandoned\\."] \
 	 "unwindonsignal, inferior function call signaled"] != 0} {
     return 0
 }
diff --git a/gdb/testsuite/gdb.compile/compile-cplus.exp b/gdb/testsuite/gdb.compile/compile-cplus.exp
index c8a40ada7f3..48fb75c3d78 100644
--- a/gdb/testsuite/gdb.compile/compile-cplus.exp
+++ b/gdb/testsuite/gdb.compile/compile-cplus.exp
@@ -133,7 +133,12 @@ gdb_test "info sym $infcall_pc" "\r\nNo symbol matches .*" "info sym not found"
 
 gdb_test_no_output "set unwindonsignal on"
 gdb_test "compile code *(volatile int *) 0 = 0;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*" \
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\(__gdb_regs\\*\\)\\) will be abandoned\\."] \
     "compile code segfault second"
 
 # C++ Specific tests.
@@ -304,7 +309,12 @@ gdb_test "p globalvar" " = -76" "expect -76"
 
 gdb_test_no_output "set debug compile on"
 gdb_test "compile code static const int readonly = 1; *(int *) &readonly = 2;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*"
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\(__gdb_regs\\*\\)\\) will be abandoned\\."]
 gdb_test_no_output "set debug compile off"
 
 
diff --git a/gdb/testsuite/gdb.compile/compile.exp b/gdb/testsuite/gdb.compile/compile.exp
index 38818d2d255..f2ab4fafa93 100644
--- a/gdb/testsuite/gdb.compile/compile.exp
+++ b/gdb/testsuite/gdb.compile/compile.exp
@@ -159,7 +159,12 @@ gdb_test "info sym $infcall_pc" "\r\nNo symbol matches .*" "info sym not found"
 
 gdb_test_no_output "set unwindonsignal on"
 gdb_test "compile code *(volatile int *) 0 = 0;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*" \
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\) will be abandoned\\."] \
     "compile code segfault second"
 
 gdb_breakpoint [gdb_get_line_number "break-here"]
@@ -312,7 +317,12 @@ gdb_test "p globalvar" " = -76" "expect -76"
 
 gdb_test_no_output "set debug compile on"
 gdb_test "compile code static const int readonly = 1; *(int *) &readonly = 2;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*"
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\) will be abandoned\\."]
 gdb_test_no_output "set debug compile off"
 
 
diff --git a/gdb/testsuite/gdb.cp/gdb2495.exp b/gdb/testsuite/gdb.cp/gdb2495.exp
index e3c0cca3175..93b046ad3a6 100644
--- a/gdb/testsuite/gdb.cp/gdb2495.exp
+++ b/gdb/testsuite/gdb.cp/gdb2495.exp
@@ -108,7 +108,12 @@ gdb_test "show unwindonsignal" \
 # Check to see if new behaviour interferes with
 # normal signal handling in inferior function calls.
 gdb_test "p exceptions.raise_signal(1)" \
-    "To change this behavior use \"set unwindonsignal off\".*" \
+    [multi_line \
+	 "The program being debugged received signal SIGABRT, Aborted" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(SimpleException::raise_signal\\(int\\)\\) will be abandoned\\."]\
     "check for unwindonsignal off message"
 
 # And reverse - turn off again.
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp b/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
index 34be4b907d3..3ccca4c2e9b 100644
--- a/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Check that when GDB fails to evaluate the condition of a conditional
-# breakpoint we only get one *stopped notification.
+# breakpoint we only get one *stopped notification.  In this test case
+# the breakpoint condition fails due to receiving a signal (SIGSEGV).
 
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
@@ -25,43 +26,84 @@ if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
     return -1
 }
 
-if {[mi_clean_restart $binfile]} {
-    return
-}
+# Create a breakpoint with a condition that invokes an inferior
+# function call, that will segfault.  Run until GDB hits the
+# breakpoint and check how GDB reports the failed condition check.
+#
+# UNWIND_ON_SIGNAL is either 'on' or 'off'.  This is used to configure
+# GDB's 'set unwindonsignal' setting.
+
+proc run_test { unwind_on_signal } {
+
+    if {[mi_clean_restart $::binfile]} {
+	return
+    }
+
+    if {[mi_runto_main] == -1} {
+	return
+    }
+
+    mi_gdb_test "-gdb-set unwindonsignal ${unwind_on_signal}" {\^done} \
+	"set unwind-on-signal"
+
+    # Create the conditional breakpoint.
+    set bp_location [gdb_get_line_number "Set breakpoint here"]
+    mi_create_breakpoint "-c \"cond_fail ()\" $::srcfile:$bp_location" \
+	"insert conditional breakpoint" \
+	-func foo -file ".*$::srcfile" -line "$bp_location" \
+	-cond "cond_fail \\(\\)"
 
-if {[mi_runto_main] == -1} {
-    return
+    # Number of the previous breakpoint.
+    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
+		   "get number for breakpoint"]
+
+    # The line where we expect the inferior to crash.
+    set crash_linenum [gdb_get_line_number "Crash here"]
+
+    # Run the inferior and wait for it to stop.
+    mi_send_resuming_command "exec-continue" "continue the inferior"
+
+    if {$unwind_on_signal} {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged received signal SIGSEGV, Segmentation fault\\\\n\"" \
+		 "&\"while in a function called from GDB\\.  GDB has restored the context\\\\n\"" \
+		 "&\"to what it was before the call\\.  To change this behavior use\\\\n\"" \
+		 "&\"\\\\\"set unwindonsignal off\\\\\"\\.  Evaluation of the expression containing\\\\n\"" \
+		 "&\"the function \\(cond_fail\\) will be abandoned\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
+		 "~\"\\\\n\"" \
+		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
+		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func foo -line $bp_location
+    } else {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "~\"\\\\nProgram\"" \
+		 "~\" received signal SIGSEGV, Segmentation fault\\.\\\\n\"" \
+		 "~\"$::hex in cond_fail \\(\\) at \[^\r\n\]+\"" \
+		 "~\"${crash_linenum}\\\\t\\s+return \\*p;\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\"\[^\r\n\]+frame=\\{addr=\"$::hex\",func=\"cond_fail\",args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$crash_linenum\",\[^\r\n\]+\\}\[^\r\n\]+" \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
+		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
+		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function\\\\n\"" \
+		 "&\"\\(cond_fail\\) will be abandoned\\.\\\\n\"" \
+		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func cond_fail -line $crash_linenum
+    }
 }
 
-# Create the conditional breakpoint.
-set bp_location [gdb_get_line_number "Set breakpoint here"]
-mi_create_breakpoint "-c \"cond_fail ()\" $srcfile:$bp_location" \
-    "insert conditional breakpoint" \
-    -func foo -file ".*$srcfile" -line "$bp_location" \
-    -cond "cond_fail \\(\\)"
-
-# Number of the previous breakpoint.
-set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
-	       "get number for breakpoint"]
-
-# The line where we expect the inferior to crash.
-set crash_linenum [gdb_get_line_number "Crash here"]
-
-# Run the inferior and wait for it to stop.
-mi_send_resuming_command "exec-continue" "continue the inferior"
-mi_gdb_test "" \
-    [multi_line \
-	 "~\"\\\\nProgram\"" \
-	 "~\" received signal SIGSEGV, Segmentation fault\\.\\\\n\"" \
-	 "~\"$hex in cond_fail \\(\\) at \[^\r\n\]+\"" \
-	 "~\"${crash_linenum}\\\\t\\s+return \\*p;\[^\r\n\]+\\\\n\"" \
-	 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\"\[^\r\n\]+" \
-	 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
-	 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
-	 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
-	 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
-	 "&\"Evaluation of the expression containing the function\\\\n\"" \
-	 "&\"\\(cond_fail\\) will be abandoned\\.\\\\n\"" \
-	 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
-	 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
-    "wait for stop"
+foreach_with_prefix unwind_on_signal { off on } {
+    run_test $unwind_on_signal
+}
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc b/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
new file mode 100644
index 00000000000..4ce8e72a3c9
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
@@ -0,0 +1,38 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+volatile int global_counter = 0;
+
+int
+cond_throw ()
+{
+  throw 3;
+}
+
+int
+foo ()
+{
+  global_counter += 1;		/* Set breakpoint here.  */
+  return 0;
+}
+
+int
+main ()
+{
+  int res = foo ();
+  return res;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
new file mode 100644
index 00000000000..463eefd7567
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
@@ -0,0 +1,122 @@
+# Copyright (C) 2023 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/>.
+
+# Check that when GDB fails to evaluate the condition of a conditional
+# breakpoint we only get one *stopped notification.  In this test case
+# the breakpoint condition fails due to throwing an uncaught C++
+# excpetion.
+
+require allow_cplus_tests
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+
+if [build_executable ${testfile}.exp ${binfile} ${srcfile} {debug c++}] {
+    return -1
+}
+
+# Create a breakpoint with a condition that invokes an inferior
+# function call, that will segfault.  Run until GDB hits the
+# breakpoint and check how GDB reports the failed condition check.
+#
+# UNWIND_ON_EXCEPTION is either 'on' or 'off'.  This is used to configure
+# GDB's 'set unwind-on-terminating-exception' setting.
+
+proc run_test { unwind_on_exception } {
+
+    if {[mi_clean_restart $::binfile]} {
+	return
+    }
+
+    if {[mi_runto_main] == -1} {
+	return
+    }
+
+    mi_gdb_test "-gdb-set unwind-on-terminating-exception ${unwind_on_exception}" {\^done} \
+	"set unwind-on-terminating-exception"
+
+    # Create the conditional breakpoint.
+    set bp_location [gdb_get_line_number "Set breakpoint here"]
+    mi_create_breakpoint "-c \"cond_throw ()\" $::srcfile:$bp_location" \
+	"insert conditional breakpoint" \
+	-func "foo\\(\\)" -file ".*$::srcfile" -line "$bp_location" \
+	-cond "cond_throw \\(\\)"
+
+    # Number of the previous breakpoint.
+    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
+		   "get number for breakpoint"]
+
+    # The line where we expect the inferior to crash.
+    set crash_linenum 0
+    #[gdb_get_line_number "Crash here"]
+
+    # Run the inferior and wait for it to stop.
+    mi_send_resuming_command "exec-continue" "continue the inferior"
+
+    if {$unwind_on_exception} {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged entered a std::terminate call, most likely\\\\n\"" \
+		 "&\"caused by an unhandled C\\+\\+ exception.  GDB blocked this call in order\\\\n\"" \
+		 "&\"to prevent the program from being terminated, and has restored the\\\\n\"" \
+		 "&\"context to its original state before the call.\\\\n\"" \
+		 "&\"To change this behaviour use \\\\\"set unwind-on-terminating-exception off\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function \\(cond_throw\\(\\)\\)\\\\n\"" \
+		 "&\"will be abandoned.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
+		 "~\"\\\\n\"" \
+		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
+		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func foo -line $bp_location
+    } else {
+	# This pattern matches multiple lines being sent to MI's
+	# stdout stream (that is wrapped in ~"...").  Depending on
+	# where exactly the thread stops, and which debug info is
+	# available, the following stop will produce different numbers
+	# of lines.
+	set out_ln_re "(?:\r\n~\"\[^\r\n\]+\")*"
+
+	mi_gdb_test "" \
+	    [multi_line \
+		 "terminate called after throwing an instance of 'int'" \
+		 "~\"\\\\nProgram\"" \
+		 "~\" received signal SIGABRT, Aborted\\.\\\\n\"${out_ln_re}" \
+		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGABRT\"\[^\r\n\]+frame=\\{addr=\"$::hex\",\[^\r\n\]+\\}\[^\r\n\]+" \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
+		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
+		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function\\\\n\"" \
+		 "&\"\\(cond_throw\\(\\)\\) will be abandoned\\.\\\\n\"" \
+		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
+	    "wait for stop"
+
+	# Don't try to check the current frame here, the inferior will
+	# be stopped somewhere in the C++ runtime at the point where
+	# it is determined that the exception has not been handled.
+    }
+}
+
+foreach_with_prefix unwind_on_exception { off } {
+    run_test $unwind_on_exception
+}


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-14 14:50     ` Andrew Burgess
@ 2023-08-15  0:32       ` Thiago Jung Bauermann
  2023-08-16 14:28         ` Andrew Burgess
  0 siblings, 1 reply; 12+ messages in thread
From: Thiago Jung Bauermann @ 2023-08-15  0:32 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: pedro, gdb-patches

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


Hello Andrew,

Andrew Burgess <aburgess@redhat.com> writes:

> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>
>> This test fails:
>>
>> Expecting: ^([
>> ]+)?(terminate called after throwing an instance of 'int'
>> ~"\\nProgram"
>> ~" received signal SIGABRT, Aborted\.\\n"
>> ~"0x[0-9A-Fa-f]+ in [^
>> ]+"
>> \*stopped,reason="signal-received",signal-name="SIGABRT"[^
>> ]+frame=\{addr="0x[0-9A-Fa-f]+",[^
>> ]+\}[^
>> ]+
>> &"Error in testing condition for breakpoint 2:\\n"
>> &"The program being debugged was signaled while in a function called from GDB\.\\n"
>> &"GDB remains in the frame where the signal was received\.\\n"
>> &"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
>> &"Evaluation of the expression containing the function\\n"
>> &"\(cond_throw\(\)\) will be abandoned\.\\n"
>> &"When the function is done executing, GDB will silently stop\.\\n"
>> =breakpoint-modified,bkpt={number="2",type="breakpoint",[^
>> ]+times="1",[^
>> ]+}[
>> ]+[(]gdb[)] 
>> [ ]*)
>> terminate called after throwing an instance of 'int'
>> ~"\nProgram"
>> ~" received signal SIGABRT, Aborted.\n"
>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>> *stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842417696"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="78"
>> &"Error in testing condition for breakpoint 2:\n"
>> &"The program being debugged was signaled while in a function called from GDB.\n"
>> &"GDB remains in the frame where the signal was received.\n"
>> &"To change this behavior use \"set unwindonsignal on\".\n"
>> &"Evaluation of the expression containing the function\n"
>> &"(cond_throw()) will be abandoned.\n"
>> &"When the function is done executing, GDB will silently stop.\n"
>> =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaa087c",func="foo()",file="/home/tcwg-build/workspace/tcwg_gnu_5/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/tcwg-build/workspace/tcwg_gnu_5/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
>> (gdb) 
>> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>>
>> It looks like the testcase isn't expecting these lines:
>>
>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>
> Thanks for testing this, and highlighting this failure.
>
> I've included an updated patch below.  I've made the test pattern a
> little more general so it should (I hope) handle the lines you are
> seeing.
>
> I'd be grateful if you could retest this new version and let me know if
> you are still seeing the failure.

Thank you for the new version. Unfortunately I still see the failure.
I'm attaching the gdb.log file.

-- 
Thiago



[-- Attachment #2: gdb.log --]
[-- Type: text/plain, Size: 15232 bytes --]

Test run by thiago.bauermann on Mon Aug 14 21:04:10 2023
Native configuration is aarch64-unknown-linux-gnu

		=== gdb tests ===

Schedule of variations:
    tcwg-local

Running target tcwg-local
Using /home/thiago.bauermann/tmp/workspace-2/abe/config/boards/tcwg-local.exp as board description file for target.
Using /home/thiago.bauermann/tmp/workspace-2/abe/builds/hosttools/aarch64-unknown-linux-gnu/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /home/thiago.bauermann/tmp/workspace-2/abe/builds/hosttools/aarch64-unknown-linux-gnu/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp ...
Executing on build: rm -rf /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw    (timeout = 300)
builtin_spawn -ignore SIGHUP rm -rf /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw
Executing on host: gcc    -fdiagnostics-color=never -c -o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/temp/3245298/ccopts.o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/temp/3245298/ccopts.c    (timeout = 300)
builtin_spawn -ignore SIGHUP gcc -fdiagnostics-color=never -c -o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/temp/3245298/ccopts.o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/temp/3245298/ccopts.c
get_compiler_info: gcc-11-3-0
get_compiler_info: gcc-11-3-0
Executing on host: g++  -fno-stack-protector  -fdiagnostics-color=never  -c -g  -o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw0.o /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc    (timeout = 300)
builtin_spawn -ignore SIGHUP g++ -fno-stack-protector -fdiagnostics-color=never -c -g -o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw0.o /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
Executing on host: g++  -fno-stack-protector /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw0.o  -fdiagnostics-color=never  -L/home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/libiberty  -g  -lm   -o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw    (timeout = 300)
builtin_spawn -ignore SIGHUP g++ -fno-stack-protector /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw0.o -fdiagnostics-color=never -L/home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/libiberty -g -lm -o /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw
builtin_spawn /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/../../gdb/gdb -nw -nx -q -iex set height 0 -iex set width 0 -data-directory /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/data-directory -i=mi
=thread-group-added,id="i1"
=cmd-param-changed,param="height",value="4294967295"
=cmd-param-changed,param="width",value="4294967295"
(gdb) 
100-gdb-set height 0
100^done
(gdb) 
101-gdb-set width 0
101^done
(gdb) 
show mi-async
&"show mi-async\n"
~"Whether MI is in asynchronous mode is off.\n"
^done
(gdb) 
104-environment-directory -r
104^done,source-path="$cdir:$cwd"
(gdb) 
105-environment-directory /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi
105^done,source-path="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi:$cdir:$cwd"
(gdb) 
105-file-exec-and-symbols /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/outputs/gdb.mi/mi-condbreak-throw/mi-condbreak-throw
105^done
(gdb) 
Expecting: ^(200-break-insert --qualified -t main[
]+)?(200\^done,bkpt=\{number=".*",type="breakpoint",disp="del",enabled=".*",addr=".*",func="main(\(.*\))?",file=".*",fullname=".*",line=".*",thread-groups=\[.*\],times=".*".*original-location=".*"\}[
]+[(]gdb[)] 
[ ]*)
200-break-insert --qualified -t main
200^done,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x00000000000008a8",func="main()",file="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/thiago.bauermann/tmp/workspace-2/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="36",thread-groups=["i1"],times="0",original-location="-qualified main"}
(gdb) 
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: breakpoint at main
220-exec-run 
=thread-group-started,id="i1",pid="3245541"
=thread-created,id="1",group-id="i1"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000aaaaaaaaa8a8",func="main()",file="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/thiago.bauermann/tmp/workspace-2/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="36",thread-groups=["i1"],times="0",original-location="-qualified main"}
=library-loaded,id="/lib/ld-linux-aarch64.so.1",target-name="/lib/ld-linux-aarch64.so.1",host-name="/lib/ld-linux-aarch64.so.1",symbols-loaded="0",thread-group="i1",ranges=[{from="0x0000fffff7fc3c40",to="0x0000fffff7fe2064"}]
220^running
*running,thread-id="all"
(gdb) 
mi_expect_stop: expecting: \*stopped,reason="breakpoint-hit",disp="del",bkptno="[0-9]+",frame={addr="0x[0-9A-Fa-f]+",func="main",args=\[.*\],(?:file="[^
]*.*",fullname="(/[^\n]*/|\\\\[^\\]+\\[^\n]+\\|\\[^\\][^\n]*\\|[a-zA-Z]:[^\n]*\\).*",line="[0-9]+",arch="[^
]*"|from=".*")},thread-id="[0-9]+",stopped-threads=[^
]*
(=thread-selected,id="[0-9]+"
|=(?:breakpoint-created|breakpoint-deleted)[^
]+"
)*[(]gdb[)] 
$
=library-loaded,id="/lib/aarch64-linux-gnu/libstdc++.so.6",target-name="/lib/aarch64-linux-gnu/libstdc++.so.6",host-name="/lib/aarch64-linux-gnu/libstdc++.so.6",symbols-loaded="0",thread-group="i1",ranges=[{from="0x0000fffff7e2de30",to="0x0000fffff7f2ee9c"}]
=library-loaded,id="/lib/aarch64-linux-gnu/libc.so.6",target-name="/lib/aarch64-linux-gnu/libc.so.6",host-name="/lib/aarch64-linux-gnu/libc.so.6",symbols-loaded="0",thread-group="i1",ranges=[{from="0x0000fffff7c07040",to="0x0000fffff7d13f20"}]
=library-loaded,id="/lib/aarch64-linux-gnu/libm.so.6",target-name="/lib/aarch64-linux-gnu/libm.so.6",host-name="/lib/aarch64-linux-gnu/libm.so.6",symbols-loaded="0",thread-group="i1",ranges=[{from="0x0000fffff7b4ca50",to="0x0000fffff7b93ea0"}]
=library-loaded,id="/lib/aarch64-linux-gnu/libgcc_s.so.1",target-name="/lib/aarch64-linux-gnu/libgcc_s.so.1",host-name="/lib/aarch64-linux-gnu/libgcc_s.so.1",symbols-loaded="0",thread-group="i1",ranges=[{from="0x0000fffff7b12bc0",to="0x0000fffff7b20e60"}]
~"[Thread debugging using libthread_db enabled]\n"
~"Using host libthread_db library \"/lib/aarch64-linux-gnu/libthread_db.so.1\".\n"
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000aaaaaaaaa8a8",func="main()",file="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/thiago.bauermann/tmp/workspace-2/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="36",thread-groups=["i1"],times="1",original-location="-qualified main"}
~"\n"
~"Temporary breakpoint 1, main () at /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc:36\n"
~"36\t  int res = foo ();\n"
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x0000aaaaaaaaa8a8",func="main",args=[],file="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/thiago.bauermann/tmp/workspace-2/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="36",arch="aarch64"},thread-id="1",stopped-threads="all",core="97"
=breakpoint-deleted,id="1"
(gdb) 
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: mi runto main
Expecting: ^(-gdb-set unwind-on-terminating-exception off[
]+)?(\^done[
]+[(]gdb[)] 
[ ]*)
-gdb-set unwind-on-terminating-exception off
^done
(gdb) 
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: set unwind-on-terminating-exception
Expecting: ^(222-break-insert -c "cond_throw \(\)" mi-condbreak-throw\.cc:29[
]+)?(222\^done,bkpt=\{number=".*",type=".*",disp=".*",enabled=".*",addr=".*",func="foo\(\)",file=".*mi-condbreak-throw.cc",fullname=".*",line="29",thread-groups=\[.*\],cond="cond_throw \(\)"(,evaluated-by=".*")?,times=".*".*original-location=".*"\}[
]+[(]gdb[)] 
[ ]*)
222-break-insert -c "cond_throw ()" mi-condbreak-throw.cc:29
222^done,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaaa87c",func="foo()",file="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/thiago.bauermann/tmp/workspace-2/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="0",original-location="mi-condbreak-throw.cc:29"}
(gdb) 
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: insert conditional breakpoint
print/d $bpnum
&"print/d $bpnum\n"
~"$1 = 2\n"
^done
(gdb) 
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: get number for breakpoint
-exec-continue
^running
*running,thread-id="all"
(gdb) 
Expecting: ^([
]+)?(terminate called after throwing an instance of 'int'
~"\\nProgram"
~" received signal SIGABRT, Aborted\.\\n"(?:
~"[^
]+")*
\*stopped,reason="signal-received",signal-name="SIGABRT"[^
]+frame=\{addr="0x[0-9A-Fa-f]+",[^
]+\}[^
]+
&"Error in testing condition for breakpoint 2:\\n"
&"The program being debugged was signaled while in a function called from GDB\.\\n"
&"GDB remains in the frame where the signal was received\.\\n"
&"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
&"Evaluation of the expression containing the function\\n"
&"\(cond_throw\(\)\) will be abandoned\.\\n"
&"When the function is done executing, GDB will silently stop\.\\n"
=breakpoint-modified,bkpt={number="2",type="breakpoint",[^
]+times="1",[^
]+}[
]+[(]gdb[)] 
[ ]*)
terminate called after throwing an instance of 'int'
~"\nProgram"
~" received signal SIGABRT, Aborted.\n"
~"__pthread_kill_implementation (threadid=281474842233376, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
&"44\t./nptl/pthread_kill.c: No such file or directory.\n"
*stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842233376"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="97"
&"Error in testing condition for breakpoint 2:\n"
&"The program being debugged was signaled while in a function called from GDB.\n"
&"GDB remains in the frame where the signal was received.\n"
&"To change this behavior use \"set unwindonsignal on\".\n"
&"Evaluation of the expression containing the function\n"
&"(cond_throw()) will be abandoned.\n"
&"When the function is done executing, GDB will silently stop.\n"
=breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaaa87c",func="foo()",file="/home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/thiago.bauermann/tmp/workspace-2/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
(gdb) 
FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
testcase /home/thiago.bauermann/tmp/workspace-2/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp completed in 1 seconds

		=== gdb Summary ===

# of expected passes		5
# of unexpected failures	1
Executing on host: /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/../../gdb/gdb -nw -nx -q -iex "set height 0" -iex "set width 0" -data-directory /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/data-directory --version    (timeout = 300)
builtin_spawn -ignore SIGHUP /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/testsuite/../../gdb/gdb -nw -nx -q -iex set height 0 -iex set width 0 -data-directory /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/data-directory --version
GNU gdb (Linaro_GDB-2023.08.14) 14.0.50.20230814-git
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
/home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/gdb version  14.0.50.20230814-git -nw -nx -q -iex "set height 0" -iex "set width 0" -data-directory /home/thiago.bauermann/tmp/workspace-2/abe/builds/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/gdb-gdb.git~master/gdb/data-directory 

runtest completed at Mon Aug 14 21:04:12 2023

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-15  0:32       ` Thiago Jung Bauermann
@ 2023-08-16 14:28         ` Andrew Burgess
  2023-08-18 21:56           ` Thiago Jung Bauermann
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Burgess @ 2023-08-16 14:28 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: pedro, gdb-patches

Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:

> Hello Andrew,
>
> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>>
>>> This test fails:
>>>
>>> Expecting: ^([
>>> ]+)?(terminate called after throwing an instance of 'int'
>>> ~"\\nProgram"
>>> ~" received signal SIGABRT, Aborted\.\\n"
>>> ~"0x[0-9A-Fa-f]+ in [^
>>> ]+"
>>> \*stopped,reason="signal-received",signal-name="SIGABRT"[^
>>> ]+frame=\{addr="0x[0-9A-Fa-f]+",[^
>>> ]+\}[^
>>> ]+
>>> &"Error in testing condition for breakpoint 2:\\n"
>>> &"The program being debugged was signaled while in a function called from GDB\.\\n"
>>> &"GDB remains in the frame where the signal was received\.\\n"
>>> &"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
>>> &"Evaluation of the expression containing the function\\n"
>>> &"\(cond_throw\(\)\) will be abandoned\.\\n"
>>> &"When the function is done executing, GDB will silently stop\.\\n"
>>> =breakpoint-modified,bkpt={number="2",type="breakpoint",[^
>>> ]+times="1",[^
>>> ]+}[
>>> ]+[(]gdb[)] 
>>> [ ]*)
>>> terminate called after throwing an instance of 'int'
>>> ~"\nProgram"
>>> ~" received signal SIGABRT, Aborted.\n"
>>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>>> *stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842417696"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="78"
>>> &"Error in testing condition for breakpoint 2:\n"
>>> &"The program being debugged was signaled while in a function called from GDB.\n"
>>> &"GDB remains in the frame where the signal was received.\n"
>>> &"To change this behavior use \"set unwindonsignal on\".\n"
>>> &"Evaluation of the expression containing the function\n"
>>> &"(cond_throw()) will be abandoned.\n"
>>> &"When the function is done executing, GDB will silently stop.\n"
>>> =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaa087c",func="foo()",file="/home/tcwg-build/workspace/tcwg_gnu_5/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/tcwg-build/workspace/tcwg_gnu_5/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
>>> (gdb) 
>>> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>>>
>>> It looks like the testcase isn't expecting these lines:
>>>
>>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>>
>> Thanks for testing this, and highlighting this failure.
>>
>> I've included an updated patch below.  I've made the test pattern a
>> little more general so it should (I hope) handle the lines you are
>> seeing.
>>
>> I'd be grateful if you could retest this new version and let me know if
>> you are still seeing the failure.
>
> Thank you for the new version. Unfortunately I still see the failure.
> I'm attaching the gdb.log file.

Ok, I think I have it this time :) Maybe...

... would you be so kind as to give the version below a try please.

Thanks,
Andrew

---

commit c61090369f37c0daec2d6b6e7819061e3dee83c9
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Aug 8 10:45:20 2023 +0100

    gdb: MI stopped events when unwindonsignal is on
    
    This recent commit:
    
      commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
      Date:   Wed Jul 12 21:56:50 2023 +0100
    
          gdb: avoid double stop after failed breakpoint condition check
    
    Addressed a problem where two MI stopped events would be reported if a
    breakpoint condition failed due to a signal, this commit was a
    replacement for this commit:
    
      commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
      Date:   Fri Oct 14 14:53:15 2022 +0100
    
          gdb: don't always print breakpoint location after failed condition check
    
    which solved the two stop problem, but only for the CLI.  Before both
    of these commits, if a b/p condition failed due to a signal then the
    user would see two stops reported, the first at the location where the
    signal occurred, and the second at the location of the breakpoint.
    
    By default GDB remains at the location where the signal occurred, so
    the second reported stop can be confusing, this is the problem that
    commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
    extended also address the issue for MI too.
    
    However, while working on another patch I realised that there was a
    problem with GDB after the above commits.  Neither of the above
    commits considered 'set unwindonsignal on'.  With this setting on,
    when an inferior function call fails with a signal GDB will unwind the
    stack back to the location where the inferior function call started.
    In the b/p case we're looking at, the stop should be reported at the
    location of the breakpoint, not at the location where the signal
    occurred, and this isn't what happens.
    
    This commit fixes this by ensuring that when unwindonsignal is 'on',
    GDB reports a single stop event at the location of the breakpoint,
    this fixes things for both CLI and MI.
    
    The function call_thread_fsm::should_notify_stop is called when the
    inferior function call completes and GDB is figuring out if the user
    should be notified about this stop event by calling normal_stop from
    fetch_inferior_event in infrun.c.  If normal_stop is called, then this
    notification will be for the location where the inferior call stopped,
    which will be the location at which the signal occurred.
    
    Prior to this commit, the only time that normal_stop was not called,
    was if the inferior function call completed successfully, this was
    controlled by ::should_notify_stop, which only turns false when the
    inferior function call has completed successfully.
    
    In this commit I have extended the logic in ::should_notify_stop.  Now
    there are three cases in which ::should_notify_stop will return false,
    and we will not announce the first stop (by calling normal_stop).
    These three reasons are:
    
      1. If the inferior function call completes successfully, this is
      unchanged behaviour,
    
      2. If the inferior function call stopped due to a signal and 'set
      unwindonsignal on' is in effect, and
    
      3. If the inferior function call stopped due to an uncaught C++
      exception, and 'set unwind-on-terminating-exception on' is in
      effect.
    
    However, if we don't call normal_stop then we need to call
    async_enable_stdin in call_thread_fsm::should_stop.  Prior to this
    commit this was only done for the case where the inferior function
    call completed successfully.
    
    In this commit I now call ::should_notify_stop and use this to
    determine if we need to call async_enable_stdin.  With this done we
    now call async_enable_stdin for each of the three cases listed above,
    which means that GDB will exit wait_sync_command_done correctly (see
    run_inferior_call in infcall.c).
    
    With these two changes the problem is mostly resolved.  However, the
    solution isn't ideal, we've still lost some information.
    
    Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
    2e411b8c68eb:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
    
      Program received signal SIGSEGV, Segmentation fault.
      0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
      24      return *p;                    /* Crash here.  */
      Error in testing breakpoint condition:
      The program being debugged was signaled while in a function called from GDB.
      GDB has restored the context to what it was before the call.
      To change this behavior use "set unwindonsignal off".
      Evaluation of the expression containing the function
      (cond_fail) will be abandoned.
    
      Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
      30      global_counter += 1;          /* Set breakpoint here.  */
      (gdb)
    
    In this state we see two stop notifications, the first is where the
    signal occurred, while the second is where the breakpoint is located.
    As GDB has unwound the stack (thanks to unwindonsignal) the second
    stop notification reflects where the inferior is actually located.
    
    Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
    to this:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
    
      Program received signal SIGSEGV, Segmentation fault.
      0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
      24      return *p;                    /* Crash here.  */
      Error in testing condition for breakpoint 1:
      The program being debugged was signaled while in a function called from GDB.
      GDB has restored the context to what it was before the call.
      To change this behavior use "set unwindonsignal off".
      Evaluation of the expression containing the function
      (cond_fail) will be abandoned.
      (gdb) bt 1
      #0  foo () at /tmp/mi-condbreak-fail.c:30
      (More stack frames follow...)
      (gdb)
    
    This is the broken state.  GDB is reports the SIGSEGV location, but
    not the unwound breakpoint location.  The final 'bt 1' shows that the
    inferior is not located in cond_fail, which is the only location GDB
    reported, so this is clearly wrong.
    
    After implementing the fixes described above we now get this
    behaviour:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
      Error in testing breakpoint condition for breakpoint 1:
      The program being debugged was signaled while in a function called from GDB.
      GDB has restored the context to what it was before the call.
      To change this behavior use "set unwindonsignal off".
      Evaluation of the expression containing the function
      (cond_fail) will be abandoned.
    
      Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
      30      global_counter += 1;          /* Set breakpoint here.  */
      (gdb)
    
    This is better.  GDB now reports a single stop at the location of the
    breakpoint, which is where the inferior is actually located.  However,
    by removing the first stop notification we have lost some potentially
    useful information about which signal caused the inferior to stop.
    
    To address this I've reworked the message that is printed to include
    the signal information.  GDB now reports this:
    
      $ gdb -q /tmp/mi-condbreak-fail \
            -ex 'set unwindonsignal on' \
            -ex 'break 30 if (cond_fail())' \
            -ex 'run'
      Reading symbols from /tmp/mi-condbreak-fail...
      Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
      Starting program: /tmp/mi-condbreak-fail
      Error in testing condition for breakpoint 1:
      The program being debugged received signal SIGSEGV, Segmentation fault
      while in a function called from GDB.  GDB has restored the context
      to what it was before the call.  To change this behavior use
      "set unwindonsignal off".  Evaluation of the expression containing
      the function (cond_fail) will be abandoned.
    
      Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
      30      global_counter += 1;          /* Set breakpoint here.  */
      (gdb)
    
    This is better, the user now sees a single stop notification at the
    correct location, and the error message describes which signal caused
    the inferior function call to stop.
    
    However, we have lost the information about where the signal
    occurred.  I did consider trying to include this information in the
    error message, but, in the end, I opted not too.  I wasn't sure it was
    worth the effort.  If the user has selected to unwind on signal, then
    surely this implies they maybe aren't interested in debugging failed
    inferior calls, so, hopefully, just knowing the signal name will be
    enough.  I figure we can always add this information in later if
    there's a demand for it.

diff --git a/gdb/infcall.c b/gdb/infcall.c
index bea5b185ddc..697f8a6291b 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -563,11 +563,20 @@ call_thread_fsm::should_stop (struct thread_info *thread)
 	 registers are restored to what they were before the
 	 call..  */
       return_value = get_call_return_value (&return_meta_info);
+    }
+
+  /* We are always going to stop this thread, but we might not be planning
+     to call call normal_stop, which is only done if should_notify_stop
+     returns true.
+
+     As normal_stop is responsible for calling async_enable_stdin, which
+     would break us out of wait_sync_command_done, then, if we don't plan
+     to call normal_stop, we should call async_enable_stdin here instead.
 
-      /* Break out of wait_sync_command_done.  This is similar to the
-	 async_enable_stdin call in normal_stop (which we don't call),
-	 however, in this case we only change the WAITING_UI.  This is
-	 enough for wait_sync_command_done.  */
+     Unlike normal_stop, we only call async_enable_stdin on WAITING_UI, but
+     that is sufficient for wait_sync_command_done.  */
+  if (!this->should_notify_stop ())
+    {
       scoped_restore save_ui = make_scoped_restore (&current_ui, waiting_ui);
       gdb_assert (current_ui->prompt_state == PROMPT_BLOCKED);
       async_enable_stdin ();
@@ -581,10 +590,28 @@ call_thread_fsm::should_stop (struct thread_info *thread)
 bool
 call_thread_fsm::should_notify_stop ()
 {
+  INFCALL_SCOPED_DEBUG_ENTER_EXIT;
+
   if (finished_p ())
     {
       /* Infcall succeeded.  Be silent and proceed with evaluating the
 	 expression.  */
+      infcall_debug_printf ("inferior call has finished, don't notify");
+      return false;
+    }
+
+  infcall_debug_printf ("inferior call didn't complete fully");
+
+  if (stopped_by_random_signal && unwind_on_signal_p)
+    {
+      infcall_debug_printf ("unwind-on-signal is on, don't notify");
+      return false;
+    }
+
+  if (stop_stack_dummy == STOP_STD_TERMINATE
+      && unwind_on_terminating_exception_p)
+    {
+      infcall_debug_printf ("unwind-on-terminating-exception is on, don't notify");
       return false;
     }
 
@@ -1512,6 +1539,11 @@ When the function is done executing, GDB will silently stop."),
 	    {
 	      /* The user wants the context restored.  */
 
+	      /* Capture details of the signal so we can include them in
+		 the error message.  Calling dummy_frame_pop will restore
+		 the previous stop signal details.  */
+	      gdb_signal stop_signal = call_thread->stop_signal ();
+
 	      /* We must get back to the frame we were before the
 		 dummy call.  */
 	      dummy_frame_pop (dummy_id, call_thread.get ());
@@ -1523,11 +1555,13 @@ When the function is done executing, GDB will silently stop."),
 	      /* FIXME: Insert a bunch of wrap_here; name can be very
 		 long if it's a C++ name with arguments and stuff.  */
 	      error (_("\
-The program being debugged was signaled while in a function called from GDB.\n\
-GDB has restored the context to what it was before the call.\n\
-To change this behavior use \"set unwindonsignal off\".\n\
-Evaluation of the expression containing the function\n\
-(%s) will be abandoned."),
+The program being debugged received signal %s, %s\n\
+while in a function called from GDB.  GDB has restored the context\n\
+to what it was before the call.  To change this behavior use\n\
+\"set unwindonsignal off\".  Evaluation of the expression containing\n\
+the function (%s) will be abandoned."),
+		     gdb_signal_to_name (stop_signal),
+		     gdb_signal_to_string (stop_signal),
 		     name.c_str ());
 	    }
 	  else
diff --git a/gdb/testsuite/gdb.base/unwindonsignal.exp b/gdb/testsuite/gdb.base/unwindonsignal.exp
index 625b0c4db12..5c2243236ba 100644
--- a/gdb/testsuite/gdb.base/unwindonsignal.exp
+++ b/gdb/testsuite/gdb.base/unwindonsignal.exp
@@ -45,7 +45,12 @@ gdb_test "show unwindonsignal" \
 # Call function (causing the program to get a signal), and see if gdb handles
 # it properly.
 if {[gdb_test "call gen_signal ()"  \
-	 "\[\r\n\]*The program being debugged was signaled.*" \
+	 [multi_line \
+	      "The program being debugged received signal SIGABRT, Aborted" \
+	      "while in a function called from GDB\\.  GDB has restored the context" \
+	      "to what it was before the call\\.  To change this behavior use" \
+	      "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	      "the function \\(gen_signal\\) will be abandoned\\."] \
 	 "unwindonsignal, inferior function call signaled"] != 0} {
     return 0
 }
diff --git a/gdb/testsuite/gdb.compile/compile-cplus.exp b/gdb/testsuite/gdb.compile/compile-cplus.exp
index c8a40ada7f3..48fb75c3d78 100644
--- a/gdb/testsuite/gdb.compile/compile-cplus.exp
+++ b/gdb/testsuite/gdb.compile/compile-cplus.exp
@@ -133,7 +133,12 @@ gdb_test "info sym $infcall_pc" "\r\nNo symbol matches .*" "info sym not found"
 
 gdb_test_no_output "set unwindonsignal on"
 gdb_test "compile code *(volatile int *) 0 = 0;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*" \
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\(__gdb_regs\\*\\)\\) will be abandoned\\."] \
     "compile code segfault second"
 
 # C++ Specific tests.
@@ -304,7 +309,12 @@ gdb_test "p globalvar" " = -76" "expect -76"
 
 gdb_test_no_output "set debug compile on"
 gdb_test "compile code static const int readonly = 1; *(int *) &readonly = 2;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*"
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\(__gdb_regs\\*\\)\\) will be abandoned\\."]
 gdb_test_no_output "set debug compile off"
 
 
diff --git a/gdb/testsuite/gdb.compile/compile.exp b/gdb/testsuite/gdb.compile/compile.exp
index 38818d2d255..f2ab4fafa93 100644
--- a/gdb/testsuite/gdb.compile/compile.exp
+++ b/gdb/testsuite/gdb.compile/compile.exp
@@ -159,7 +159,12 @@ gdb_test "info sym $infcall_pc" "\r\nNo symbol matches .*" "info sym not found"
 
 gdb_test_no_output "set unwindonsignal on"
 gdb_test "compile code *(volatile int *) 0 = 0;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*" \
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\) will be abandoned\\."] \
     "compile code segfault second"
 
 gdb_breakpoint [gdb_get_line_number "break-here"]
@@ -312,7 +317,12 @@ gdb_test "p globalvar" " = -76" "expect -76"
 
 gdb_test_no_output "set debug compile on"
 gdb_test "compile code static const int readonly = 1; *(int *) &readonly = 2;" \
-    "The program being debugged was signaled while in a function called from GDB\\.\r\nGDB has restored the context to what it was before the call\\.\r\n.*"
+    [multi_line \
+	 "The program being debugged received signal SIGSEGV, Segmentation fault" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(_gdb_expr\\) will be abandoned\\."]
 gdb_test_no_output "set debug compile off"
 
 
diff --git a/gdb/testsuite/gdb.cp/gdb2495.exp b/gdb/testsuite/gdb.cp/gdb2495.exp
index e3c0cca3175..93b046ad3a6 100644
--- a/gdb/testsuite/gdb.cp/gdb2495.exp
+++ b/gdb/testsuite/gdb.cp/gdb2495.exp
@@ -108,7 +108,12 @@ gdb_test "show unwindonsignal" \
 # Check to see if new behaviour interferes with
 # normal signal handling in inferior function calls.
 gdb_test "p exceptions.raise_signal(1)" \
-    "To change this behavior use \"set unwindonsignal off\".*" \
+    [multi_line \
+	 "The program being debugged received signal SIGABRT, Aborted" \
+	 "while in a function called from GDB\\.  GDB has restored the context" \
+	 "to what it was before the call\\.  To change this behavior use" \
+	 "\"set unwindonsignal off\"\\.  Evaluation of the expression containing" \
+	 "the function \\(SimpleException::raise_signal\\(int\\)\\) will be abandoned\\."]\
     "check for unwindonsignal off message"
 
 # And reverse - turn off again.
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp b/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
index 34be4b907d3..3ccca4c2e9b 100644
--- a/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-fail.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # Check that when GDB fails to evaluate the condition of a conditional
-# breakpoint we only get one *stopped notification.
+# breakpoint we only get one *stopped notification.  In this test case
+# the breakpoint condition fails due to receiving a signal (SIGSEGV).
 
 load_lib mi-support.exp
 set MIFLAGS "-i=mi"
@@ -25,43 +26,84 @@ if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
     return -1
 }
 
-if {[mi_clean_restart $binfile]} {
-    return
-}
+# Create a breakpoint with a condition that invokes an inferior
+# function call, that will segfault.  Run until GDB hits the
+# breakpoint and check how GDB reports the failed condition check.
+#
+# UNWIND_ON_SIGNAL is either 'on' or 'off'.  This is used to configure
+# GDB's 'set unwindonsignal' setting.
+
+proc run_test { unwind_on_signal } {
+
+    if {[mi_clean_restart $::binfile]} {
+	return
+    }
+
+    if {[mi_runto_main] == -1} {
+	return
+    }
+
+    mi_gdb_test "-gdb-set unwindonsignal ${unwind_on_signal}" {\^done} \
+	"set unwind-on-signal"
+
+    # Create the conditional breakpoint.
+    set bp_location [gdb_get_line_number "Set breakpoint here"]
+    mi_create_breakpoint "-c \"cond_fail ()\" $::srcfile:$bp_location" \
+	"insert conditional breakpoint" \
+	-func foo -file ".*$::srcfile" -line "$bp_location" \
+	-cond "cond_fail \\(\\)"
 
-if {[mi_runto_main] == -1} {
-    return
+    # Number of the previous breakpoint.
+    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
+		   "get number for breakpoint"]
+
+    # The line where we expect the inferior to crash.
+    set crash_linenum [gdb_get_line_number "Crash here"]
+
+    # Run the inferior and wait for it to stop.
+    mi_send_resuming_command "exec-continue" "continue the inferior"
+
+    if {$unwind_on_signal} {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged received signal SIGSEGV, Segmentation fault\\\\n\"" \
+		 "&\"while in a function called from GDB\\.  GDB has restored the context\\\\n\"" \
+		 "&\"to what it was before the call\\.  To change this behavior use\\\\n\"" \
+		 "&\"\\\\\"set unwindonsignal off\\\\\"\\.  Evaluation of the expression containing\\\\n\"" \
+		 "&\"the function \\(cond_fail\\) will be abandoned\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
+		 "~\"\\\\n\"" \
+		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
+		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func foo -line $bp_location
+    } else {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "~\"\\\\nProgram\"" \
+		 "~\" received signal SIGSEGV, Segmentation fault\\.\\\\n\"" \
+		 "~\"$::hex in cond_fail \\(\\) at \[^\r\n\]+\"" \
+		 "~\"${crash_linenum}\\\\t\\s+return \\*p;\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\"\[^\r\n\]+frame=\\{addr=\"$::hex\",func=\"cond_fail\",args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$crash_linenum\",\[^\r\n\]+\\}\[^\r\n\]+" \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
+		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
+		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function\\\\n\"" \
+		 "&\"\\(cond_fail\\) will be abandoned\\.\\\\n\"" \
+		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func cond_fail -line $crash_linenum
+    }
 }
 
-# Create the conditional breakpoint.
-set bp_location [gdb_get_line_number "Set breakpoint here"]
-mi_create_breakpoint "-c \"cond_fail ()\" $srcfile:$bp_location" \
-    "insert conditional breakpoint" \
-    -func foo -file ".*$srcfile" -line "$bp_location" \
-    -cond "cond_fail \\(\\)"
-
-# Number of the previous breakpoint.
-set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
-	       "get number for breakpoint"]
-
-# The line where we expect the inferior to crash.
-set crash_linenum [gdb_get_line_number "Crash here"]
-
-# Run the inferior and wait for it to stop.
-mi_send_resuming_command "exec-continue" "continue the inferior"
-mi_gdb_test "" \
-    [multi_line \
-	 "~\"\\\\nProgram\"" \
-	 "~\" received signal SIGSEGV, Segmentation fault\\.\\\\n\"" \
-	 "~\"$hex in cond_fail \\(\\) at \[^\r\n\]+\"" \
-	 "~\"${crash_linenum}\\\\t\\s+return \\*p;\[^\r\n\]+\\\\n\"" \
-	 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGSEGV\"\[^\r\n\]+" \
-	 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
-	 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
-	 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
-	 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
-	 "&\"Evaluation of the expression containing the function\\\\n\"" \
-	 "&\"\\(cond_fail\\) will be abandoned\\.\\\\n\"" \
-	 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
-	 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
-    "wait for stop"
+foreach_with_prefix unwind_on_signal { off on } {
+    run_test $unwind_on_signal
+}
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc b/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
new file mode 100644
index 00000000000..4ce8e72a3c9
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc
@@ -0,0 +1,38 @@
+/* Copyright 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+volatile int global_counter = 0;
+
+int
+cond_throw ()
+{
+  throw 3;
+}
+
+int
+foo ()
+{
+  global_counter += 1;		/* Set breakpoint here.  */
+  return 0;
+}
+
+int
+main ()
+{
+  int res = foo ();
+  return res;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
new file mode 100644
index 00000000000..4151fa18395
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
@@ -0,0 +1,122 @@
+# Copyright (C) 2023 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/>.
+
+# Check that when GDB fails to evaluate the condition of a conditional
+# breakpoint we only get one *stopped notification.  In this test case
+# the breakpoint condition fails due to throwing an uncaught C++
+# excpetion.
+
+require allow_cplus_tests
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+
+if [build_executable ${testfile}.exp ${binfile} ${srcfile} {debug c++}] {
+    return -1
+}
+
+# Create a breakpoint with a condition that invokes an inferior
+# function call, that will segfault.  Run until GDB hits the
+# breakpoint and check how GDB reports the failed condition check.
+#
+# UNWIND_ON_EXCEPTION is either 'on' or 'off'.  This is used to configure
+# GDB's 'set unwind-on-terminating-exception' setting.
+
+proc run_test { unwind_on_exception } {
+
+    if {[mi_clean_restart $::binfile]} {
+	return
+    }
+
+    if {[mi_runto_main] == -1} {
+	return
+    }
+
+    mi_gdb_test "-gdb-set unwind-on-terminating-exception ${unwind_on_exception}" {\^done} \
+	"set unwind-on-terminating-exception"
+
+    # Create the conditional breakpoint.
+    set bp_location [gdb_get_line_number "Set breakpoint here"]
+    mi_create_breakpoint "-c \"cond_throw ()\" $::srcfile:$bp_location" \
+	"insert conditional breakpoint" \
+	-func "foo\\(\\)" -file ".*$::srcfile" -line "$bp_location" \
+	-cond "cond_throw \\(\\)"
+
+    # Number of the previous breakpoint.
+    set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID" \
+		   "get number for breakpoint"]
+
+    # The line where we expect the inferior to crash.
+    set crash_linenum 0
+    #[gdb_get_line_number "Crash here"]
+
+    # Run the inferior and wait for it to stop.
+    mi_send_resuming_command "exec-continue" "continue the inferior"
+
+    if {$unwind_on_exception} {
+	mi_gdb_test "" \
+	    [multi_line \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged entered a std::terminate call, most likely\\\\n\"" \
+		 "&\"caused by an unhandled C\\+\\+ exception.  GDB blocked this call in order\\\\n\"" \
+		 "&\"to prevent the program from being terminated, and has restored the\\\\n\"" \
+		 "&\"context to its original state before the call.\\\\n\"" \
+		 "&\"To change this behaviour use \\\\\"set unwind-on-terminating-exception off\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function \\(cond_throw\\(\\)\\)\\\\n\"" \
+		 "&\"will be abandoned.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}" \
+		 "~\"\\\\n\"" \
+		 "~\"Breakpoint $bpnum, foo \\(\\) at \[^\r\n\]+/${::srcfile}:${bp_location}\\\\n\"" \
+		 "~\"${bp_location}\\\\t\[^\r\n\]+Set breakpoint here\\.\[^\r\n\]+\\\\n\"" \
+		 "\\*stopped,reason=\"breakpoint-hit\",disp=\"keep\",bkptno=\"$bpnum\",frame=\\{addr=\"$::hex\",func=\"foo\"\\,args=\\\[\\\],file=\"\[^\r\n\]+\",fullname=\"\[^\r\n\]+\",line=\"$bp_location\",\[^\r\n\]+}\[^\r\n\]+"] \
+	    "wait for stop"
+
+	mi_info_frame "check the current frame" \
+	    -level 0 -func foo -line $bp_location
+    } else {
+	# This pattern matches multiple lines being sent to MI's
+	# stdout stream (that is wrapped in ~"...").  Depending on
+	# where exactly the thread stops, and which debug info is
+	# available, the following stop will produce different numbers
+	# of lines.
+	set out_ln_re "(?:\r\n\[~&\]\"\[^\r\n\]+\")*"
+
+	mi_gdb_test "" \
+	    [multi_line \
+		 "terminate called after throwing an instance of 'int'" \
+		 "~\"\\\\nProgram\"" \
+		 "~\" received signal SIGABRT, Aborted\\.\\\\n\"${out_ln_re}" \
+		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGABRT\"\[^\r\n\]+frame=\\{addr=\"$::hex\",\[^\r\n\]+\\}\[^\r\n\]+" \
+		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \
+		 "&\"The program being debugged was signaled while in a function called from GDB\\.\\\\n\"" \
+		 "&\"GDB remains in the frame where the signal was received\\.\\\\n\"" \
+		 "&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\.\\\\n\"" \
+		 "&\"Evaluation of the expression containing the function\\\\n\"" \
+		 "&\"\\(cond_throw\\(\\)\\) will be abandoned\\.\\\\n\"" \
+		 "&\"When the function is done executing, GDB will silently stop\\.\\\\n\"" \
+		 "=breakpoint-modified,bkpt={number=\"$bpnum\",type=\"breakpoint\",\[^\r\n\]+times=\"1\",\[^\r\n\]+}"] \
+	    "wait for stop"
+
+	# Don't try to check the current frame here, the inferior will
+	# be stopped somewhere in the C++ runtime at the point where
+	# it is determined that the exception has not been handled.
+    }
+}
+
+foreach_with_prefix unwind_on_exception { off } {
+    run_test $unwind_on_exception
+}


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-16 14:28         ` Andrew Burgess
@ 2023-08-18 21:56           ` Thiago Jung Bauermann
  2023-08-23  9:33             ` Andrew Burgess
  0 siblings, 1 reply; 12+ messages in thread
From: Thiago Jung Bauermann @ 2023-08-18 21:56 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: pedro, gdb-patches


Hello Andrew,

Andrew Burgess <aburgess@redhat.com> writes:

> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>
>> Hello Andrew,
>>
>> Andrew Burgess <aburgess@redhat.com> writes:
>>
>>> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>>>
>>>> This test fails:
>>>>
>>>> Expecting: ^([
>>>> ]+)?(terminate called after throwing an instance of 'int'
>>>> ~"\\nProgram"
>>>> ~" received signal SIGABRT, Aborted\.\\n"
>>>> ~"0x[0-9A-Fa-f]+ in [^
>>>> ]+"
>>>> \*stopped,reason="signal-received",signal-name="SIGABRT"[^
>>>> ]+frame=\{addr="0x[0-9A-Fa-f]+",[^
>>>> ]+\}[^
>>>> ]+
>>>> &"Error in testing condition for breakpoint 2:\\n"
>>>> &"The program being debugged was signaled while in a function called from GDB\.\\n"
>>>> &"GDB remains in the frame where the signal was received\.\\n"
>>>> &"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
>>>> &"Evaluation of the expression containing the function\\n"
>>>> &"\(cond_throw\(\)\) will be abandoned\.\\n"
>>>> &"When the function is done executing, GDB will silently stop\.\\n"
>>>> =breakpoint-modified,bkpt={number="2",type="breakpoint",[^
>>>> ]+times="1",[^
>>>> ]+}[
>>>> ]+[(]gdb[)] 
>>>> [ ]*)
>>>> terminate called after throwing an instance of 'int'
>>>> ~"\nProgram"
>>>> ~" received signal SIGABRT, Aborted.\n"
>>>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>>>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>>>> *stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842417696"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="78"
>>>> &"Error in testing condition for breakpoint 2:\n"
>>>> &"The program being debugged was signaled while in a function called from GDB.\n"
>>>> &"GDB remains in the frame where the signal was received.\n"
>>>> &"To change this behavior use \"set unwindonsignal on\".\n"
>>>> &"Evaluation of the expression containing the function\n"
>>>> &"(cond_throw()) will be abandoned.\n"
>>>> &"When the function is done executing, GDB will silently stop.\n"
>>>> =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaa087c",func="foo()",file="/home/tcwg-build/workspace/tcwg_gnu_5/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/tcwg-build/workspace/tcwg_gnu_5/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
>>>> (gdb) 
>>>> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>>>>
>>>> It looks like the testcase isn't expecting these lines:
>>>>
>>>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>>>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>>>
>>> Thanks for testing this, and highlighting this failure.
>>>
>>> I've included an updated patch below.  I've made the test pattern a
>>> little more general so it should (I hope) handle the lines you are
>>> seeing.
>>>
>>> I'd be grateful if you could retest this new version and let me know if
>>> you are still seeing the failure.
>>
>> Thank you for the new version. Unfortunately I still see the failure.
>> I'm attaching the gdb.log file.
>
> Ok, I think I have it this time :) Maybe...
>
> ... would you be so kind as to give the version below a try please.

Of course. Thank you for the new version. This one worked well.

Running gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp ...
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: breakpoint at main
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: mi runto main
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: set unwind-on-terminating-exception
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: insert conditional breakpoint
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: get number for breakpoint
PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop

                === gdb Summary ===

# of expected passes            6

Sorry for the delay in testing this one.

-- 
Thiago

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-18 21:56           ` Thiago Jung Bauermann
@ 2023-08-23  9:33             ` Andrew Burgess
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-08-23  9:33 UTC (permalink / raw)
  To: Thiago Jung Bauermann; +Cc: pedro, gdb-patches

Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:

> Hello Andrew,
>
> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>>
>>> Hello Andrew,
>>>
>>> Andrew Burgess <aburgess@redhat.com> writes:
>>>
>>>> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>>>>
>>>>> This test fails:
>>>>>
>>>>> Expecting: ^([
>>>>> ]+)?(terminate called after throwing an instance of 'int'
>>>>> ~"\\nProgram"
>>>>> ~" received signal SIGABRT, Aborted\.\\n"
>>>>> ~"0x[0-9A-Fa-f]+ in [^
>>>>> ]+"
>>>>> \*stopped,reason="signal-received",signal-name="SIGABRT"[^
>>>>> ]+frame=\{addr="0x[0-9A-Fa-f]+",[^
>>>>> ]+\}[^
>>>>> ]+
>>>>> &"Error in testing condition for breakpoint 2:\\n"
>>>>> &"The program being debugged was signaled while in a function called from GDB\.\\n"
>>>>> &"GDB remains in the frame where the signal was received\.\\n"
>>>>> &"To change this behavior use \\"set unwindonsignal on\\"\.\\n"
>>>>> &"Evaluation of the expression containing the function\\n"
>>>>> &"\(cond_throw\(\)\) will be abandoned\.\\n"
>>>>> &"When the function is done executing, GDB will silently stop\.\\n"
>>>>> =breakpoint-modified,bkpt={number="2",type="breakpoint",[^
>>>>> ]+times="1",[^
>>>>> ]+}[
>>>>> ]+[(]gdb[)] 
>>>>> [ ]*)
>>>>> terminate called after throwing an instance of 'int'
>>>>> ~"\nProgram"
>>>>> ~" received signal SIGABRT, Aborted.\n"
>>>>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>>>>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>>>>> *stopped,reason="signal-received",signal-name="SIGABRT",signal-meaning="Aborted",frame={addr="0x0000fffff7c5f200",func="__pthread_kill_implementation",args=[{name="threadid",value="281474842417696"},{name="signo",value="6"},{name="signo@entry",value="6"},{name="no_tid",value="0"},{name="no_tid@entry",value="0"}],file="./nptl/pthread_kill.c",fullname="./nptl/./nptl/pthread_kill.c",line="44",arch="aarch64"},thread-id="1",stopped-threads="all",core="78"
>>>>> &"Error in testing condition for breakpoint 2:\n"
>>>>> &"The program being debugged was signaled while in a function called from GDB.\n"
>>>>> &"GDB remains in the frame where the signal was received.\n"
>>>>> &"To change this behavior use \"set unwindonsignal on\".\n"
>>>>> &"Evaluation of the expression containing the function\n"
>>>>> &"(cond_throw()) will be abandoned.\n"
>>>>> &"When the function is done executing, GDB will silently stop.\n"
>>>>> =breakpoint-modified,bkpt={number="2",type="breakpoint",disp="keep",enabled="y",addr="0x0000aaaaaaaa087c",func="foo()",file="/home/tcwg-build/workspace/tcwg_gnu_5/abe/snapshots/gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",fullname="/home/tcwg-build/workspace/tcwg_gnu_5/gdb/gdb/testsuite/gdb.mi/mi-condbreak-throw.cc",line="29",thread-groups=["i1"],cond="cond_throw ()",times="1",original-location="mi-condbreak-throw.cc:29"}
>>>>> (gdb) 
>>>>> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>>>>>
>>>>> It looks like the testcase isn't expecting these lines:
>>>>>
>>>>> ~"__pthread_kill_implementation (threadid=281474842417696, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44\n"
>>>>> &"44\t./nptl/pthread_kill.c: No such file or directory.\n"
>>>>
>>>> Thanks for testing this, and highlighting this failure.
>>>>
>>>> I've included an updated patch below.  I've made the test pattern a
>>>> little more general so it should (I hope) handle the lines you are
>>>> seeing.
>>>>
>>>> I'd be grateful if you could retest this new version and let me know if
>>>> you are still seeing the failure.
>>>
>>> Thank you for the new version. Unfortunately I still see the failure.
>>> I'm attaching the gdb.log file.
>>
>> Ok, I think I have it this time :) Maybe...
>>
>> ... would you be so kind as to give the version below a try please.
>
> Of course. Thank you for the new version. This one worked well.
>
> Running gdb.git~master/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp ...
> PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: breakpoint at main
> PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: mi runto main
> PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: set unwind-on-terminating-exception
> PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: insert conditional breakpoint
> PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: get number for breakpoint
> PASS: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop
>
>                 === gdb Summary ===
>
> # of expected passes            6
>
> Sorry for the delay in testing this one.

Not a problem.  Thanks for all the testing.

I've gone ahead and pushed this series, as this was blocking me from
posting the latest version of my inferior function call series[1].

If there's any post-commit feedback, I am always happy to address it.

Thanks,
Andrew

[1] https://inbox.sourceware.org/gdb-patches/cover.1686131880.git.aburgess@redhat.com/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-08 14:45 ` [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on Andrew Burgess
  2023-08-10 15:06   ` Thiago Jung Bauermann
@ 2023-08-29 16:37   ` Simon Marchi
  2023-09-08  8:54     ` Andrew Burgess
  2023-09-08 10:03     ` Andrew Burgess
  1 sibling, 2 replies; 12+ messages in thread
From: Simon Marchi @ 2023-08-29 16:37 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches; +Cc: pedro

On 8/8/23 10:45, Andrew Burgess via Gdb-patches wrote:
> This recent commit:
> 
>   commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
>   Date:   Wed Jul 12 21:56:50 2023 +0100
> 
>       gdb: avoid double stop after failed breakpoint condition check
> 
> Addressed a problem where two MI stopped events would be reported if a
> breakpoint condition failed due to a signal, this commit was a
> replacement for this commit:
> 
>   commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
>   Date:   Fri Oct 14 14:53:15 2022 +0100
> 
>       gdb: don't always print breakpoint location after failed condition check
> 
> which solved the two stop problem, but only for the CLI.  Before both
> of these commits, if a b/p condition failed due to a signal then the
> user would see two stops reported, the first at the location where the
> signal occurred, and the second at the location of the breakpoint.
> 
> By default GDB remains at the location where the signal occurred, so
> the second reported stop can be confusing, this is the problem that
> commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
> extended also address the issue for MI too.
> 
> However, while working on another patch I realised that there was a
> problem with GDB after the above commits.  Neither of the above
> commits considered 'set unwindonsignal on'.  With this setting on,
> when an inferior function call fails with a signal GDB will unwind the
> stack back to the location where the inferior function call started.
> In the b/p case we're looking at, the stop should be reported at the
> location of the breakpoint, not at the location where the signal
> occurred, and this isn't what happens.
> 
> This commit fixes this by ensuring that when unwindonsignal is 'on',
> GDB reports a single stop event at the location of the breakpoint,
> this fixes things for both CLI and MI.
> 
> The function call_thread_fsm::should_notify_stop is called when the
> inferior function call completes and GDB is figuring out if the user
> should be notified about this stop event by calling normal_stop from
> fetch_inferior_event in infrun.c.  If normal_stop is called, then this
> notification will be for the location where the inferior call stopped,
> which will be the location at which the signal occurred.
> 
> Prior to this commit, the only time that normal_stop was not called,
> was if the inferior function call completed successfully, this was
> controlled by ::should_notify_stop, which only turns false when the
> inferior function call has completed successfully.
> 
> In this commit I have extended the logic in ::should_notify_stop.  Now
> there are three cases in which ::should_notify_stop will return false,
> and we will not announce the first stop (by calling normal_stop).
> These three reasons are:
> 
>   1. If the inferior function call completes successfully, this is
>   unchanged behaviour,
> 
>   2. If the inferior function call stopped due to a signal and 'set
>   unwindonsignal on' is in effect, and
> 
>   3. If the inferior function call stopped due to an uncaught C++
>   exception, and 'set unwind-on-terminating-exception on' is in
>   effect.
> 
> However, if we don't call normal_stop then we need to call
> async_enable_stdin in call_thread_fsm::should_stop.  Prior to this
> commit this was only done for the case where the inferior function
> call completed successfully.
> 
> In this commit I now call ::should_notify_stop and use this to
> determine if we need to call async_enable_stdin.  With this done we
> now call async_enable_stdin for each of the three cases listed above,
> which means that GDB will exit wait_sync_command_done correctly (see
> run_inferior_call in infcall.c).
> 
> With these two changes the problem is mostly resolved.  However, the
> solution isn't ideal, we've still lost some information.
> 
> Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
> 2e411b8c68eb:
> 
>   $ gdb -q /tmp/mi-condbreak-fail \
>         -ex 'set unwindonsignal on' \
>         -ex 'break 30 if (cond_fail())' \
>         -ex 'run'
>   Reading symbols from /tmp/mi-condbreak-fail...
>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>   Starting program: /tmp/mi-condbreak-fail
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
>   24	  return *p;			/* Crash here.  */
>   Error in testing breakpoint condition:
>   The program being debugged was signaled while in a function called from GDB.
>   GDB has restored the context to what it was before the call.
>   To change this behavior use "set unwindonsignal off".
>   Evaluation of the expression containing the function
>   (cond_fail) will be abandoned.
> 
>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>   30	  global_counter += 1;		/* Set breakpoint here.  */
>   (gdb)
> 
> In this state we see two stop notifications, the first is where the
> signal occurred, while the second is where the breakpoint is located.
> As GDB has unwound the stack (thanks to unwindonsignal) the second
> stop notification reflects where the inferior is actually located.
> 
> Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
> to this:
> 
>   $ gdb -q /tmp/mi-condbreak-fail \
>         -ex 'set unwindonsignal on' \
> 	-ex 'break 30 if (cond_fail())' \
> 	-ex 'run'
>   Reading symbols from /tmp/mi-condbreak-fail...
>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>   Starting program: /tmp/mi-condbreak-fail
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
>   24	  return *p;			/* Crash here.  */
>   Error in testing condition for breakpoint 1:
>   The program being debugged was signaled while in a function called from GDB.
>   GDB has restored the context to what it was before the call.
>   To change this behavior use "set unwindonsignal off".
>   Evaluation of the expression containing the function
>   (cond_fail) will be abandoned.
>   (gdb) bt 1
>   #0  foo () at /tmp/mi-condbreak-fail.c:30
>   (More stack frames follow...)
>   (gdb)
> 
> This is the broken state.  GDB is reports the SIGSEGV location, but
> not the unwound breakpoint location.  The final 'bt 1' shows that the
> inferior is not located in cond_fail, which is the only location GDB
> reported, so this is clearly wrong.
> 
> After implementing the fixes described above we now get this
> behaviour:
> 
>   $ gdb -q /tmp/mi-condbreak-fail \
>         -ex 'set unwindonsignal on' \
>         -ex 'break 30 if (cond_fail())' \
>         -ex 'run'
>   Reading symbols from /tmp/mi-condbreak-fail...
>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>   Starting program: /tmp/mi-condbreak-fail
>   Error in testing breakpoint condition for breakpoint 1:
>   The program being debugged was signaled while in a function called from GDB.
>   GDB has restored the context to what it was before the call.
>   To change this behavior use "set unwindonsignal off".
>   Evaluation of the expression containing the function
>   (cond_fail) will be abandoned.
> 
>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>   30	  global_counter += 1;		/* Set breakpoint here.  */
>   (gdb)
> 
> This is better.  GDB now reports a single stop at the location of the
> breakpoint, which is where the inferior is actually located.  However,
> by removing the first stop notification we have lost some potentially
> useful information about which signal caused the inferior to stop.
> 
> To address this I've reworked the message that is printed to include
> the signal information.  GDB now reports this:
> 
>   $ gdb -q /tmp/mi-condbreak-fail \
>         -ex 'set unwindonsignal on' \
>         -ex 'break 30 if (cond_fail())' \
>         -ex 'run'
>   Reading symbols from /tmp/mi-condbreak-fail...
>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>   Starting program: /tmp/mi-condbreak-fail
>   Error in testing condition for breakpoint 1:
>   The program being debugged received signal SIGSEGV, Segmentation fault
>   while in a function called from GDB.  GDB has restored the context
>   to what it was before the call.  To change this behavior use
>   "set unwindonsignal off".  Evaluation of the expression containing
>   the function (cond_fail) will be abandoned.
> 
>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>   30	  global_counter += 1;		/* Set breakpoint here.  */
>   (gdb)
> 
> This is better, the user now sees a single stop notification at the
> correct location, and the error message describes which signal caused
> the inferior function call to stop.
> 
> However, we have lost the information about where the signal
> occurred.  I did consider trying to include this information in the
> error message, but, in the end, I opted not too.  I wasn't sure it was
> worth the effort.  If the user has selected to unwind on signal, then
> surely this implies they maybe aren't interested in debugging failed
> inferior calls, so, hopefully, just knowing the signal name will be
> enough.  I figure we can always add this information in later if
> there's a demand for it.

Hi Andrew,

Starting with this commit, I see this failure with the native-gdbserver
board:

FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)

Do you see it?

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-29 16:37   ` Simon Marchi
@ 2023-09-08  8:54     ` Andrew Burgess
  2023-09-08 10:03     ` Andrew Burgess
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-09-08  8:54 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: pedro

Simon Marchi <simon.marchi@polymtl.ca> writes:

> On 8/8/23 10:45, Andrew Burgess via Gdb-patches wrote:
>> This recent commit:
>> 
>>   commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
>>   Date:   Wed Jul 12 21:56:50 2023 +0100
>> 
>>       gdb: avoid double stop after failed breakpoint condition check
>> 
>> Addressed a problem where two MI stopped events would be reported if a
>> breakpoint condition failed due to a signal, this commit was a
>> replacement for this commit:
>> 
>>   commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
>>   Date:   Fri Oct 14 14:53:15 2022 +0100
>> 
>>       gdb: don't always print breakpoint location after failed condition check
>> 
>> which solved the two stop problem, but only for the CLI.  Before both
>> of these commits, if a b/p condition failed due to a signal then the
>> user would see two stops reported, the first at the location where the
>> signal occurred, and the second at the location of the breakpoint.
>> 
>> By default GDB remains at the location where the signal occurred, so
>> the second reported stop can be confusing, this is the problem that
>> commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
>> extended also address the issue for MI too.
>> 
>> However, while working on another patch I realised that there was a
>> problem with GDB after the above commits.  Neither of the above
>> commits considered 'set unwindonsignal on'.  With this setting on,
>> when an inferior function call fails with a signal GDB will unwind the
>> stack back to the location where the inferior function call started.
>> In the b/p case we're looking at, the stop should be reported at the
>> location of the breakpoint, not at the location where the signal
>> occurred, and this isn't what happens.
>> 
>> This commit fixes this by ensuring that when unwindonsignal is 'on',
>> GDB reports a single stop event at the location of the breakpoint,
>> this fixes things for both CLI and MI.
>> 
>> The function call_thread_fsm::should_notify_stop is called when the
>> inferior function call completes and GDB is figuring out if the user
>> should be notified about this stop event by calling normal_stop from
>> fetch_inferior_event in infrun.c.  If normal_stop is called, then this
>> notification will be for the location where the inferior call stopped,
>> which will be the location at which the signal occurred.
>> 
>> Prior to this commit, the only time that normal_stop was not called,
>> was if the inferior function call completed successfully, this was
>> controlled by ::should_notify_stop, which only turns false when the
>> inferior function call has completed successfully.
>> 
>> In this commit I have extended the logic in ::should_notify_stop.  Now
>> there are three cases in which ::should_notify_stop will return false,
>> and we will not announce the first stop (by calling normal_stop).
>> These three reasons are:
>> 
>>   1. If the inferior function call completes successfully, this is
>>   unchanged behaviour,
>> 
>>   2. If the inferior function call stopped due to a signal and 'set
>>   unwindonsignal on' is in effect, and
>> 
>>   3. If the inferior function call stopped due to an uncaught C++
>>   exception, and 'set unwind-on-terminating-exception on' is in
>>   effect.
>> 
>> However, if we don't call normal_stop then we need to call
>> async_enable_stdin in call_thread_fsm::should_stop.  Prior to this
>> commit this was only done for the case where the inferior function
>> call completed successfully.
>> 
>> In this commit I now call ::should_notify_stop and use this to
>> determine if we need to call async_enable_stdin.  With this done we
>> now call async_enable_stdin for each of the three cases listed above,
>> which means that GDB will exit wait_sync_command_done correctly (see
>> run_inferior_call in infcall.c).
>> 
>> With these two changes the problem is mostly resolved.  However, the
>> solution isn't ideal, we've still lost some information.
>> 
>> Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
>> 2e411b8c68eb:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>>         -ex 'break 30 if (cond_fail())' \
>>         -ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>> 
>>   Program received signal SIGSEGV, Segmentation fault.
>>   0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
>>   24	  return *p;			/* Crash here.  */
>>   Error in testing breakpoint condition:
>>   The program being debugged was signaled while in a function called from GDB.
>>   GDB has restored the context to what it was before the call.
>>   To change this behavior use "set unwindonsignal off".
>>   Evaluation of the expression containing the function
>>   (cond_fail) will be abandoned.
>> 
>>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>>   30	  global_counter += 1;		/* Set breakpoint here.  */
>>   (gdb)
>> 
>> In this state we see two stop notifications, the first is where the
>> signal occurred, while the second is where the breakpoint is located.
>> As GDB has unwound the stack (thanks to unwindonsignal) the second
>> stop notification reflects where the inferior is actually located.
>> 
>> Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
>> to this:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>> 	-ex 'break 30 if (cond_fail())' \
>> 	-ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>> 
>>   Program received signal SIGSEGV, Segmentation fault.
>>   0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
>>   24	  return *p;			/* Crash here.  */
>>   Error in testing condition for breakpoint 1:
>>   The program being debugged was signaled while in a function called from GDB.
>>   GDB has restored the context to what it was before the call.
>>   To change this behavior use "set unwindonsignal off".
>>   Evaluation of the expression containing the function
>>   (cond_fail) will be abandoned.
>>   (gdb) bt 1
>>   #0  foo () at /tmp/mi-condbreak-fail.c:30
>>   (More stack frames follow...)
>>   (gdb)
>> 
>> This is the broken state.  GDB is reports the SIGSEGV location, but
>> not the unwound breakpoint location.  The final 'bt 1' shows that the
>> inferior is not located in cond_fail, which is the only location GDB
>> reported, so this is clearly wrong.
>> 
>> After implementing the fixes described above we now get this
>> behaviour:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>>         -ex 'break 30 if (cond_fail())' \
>>         -ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>>   Error in testing breakpoint condition for breakpoint 1:
>>   The program being debugged was signaled while in a function called from GDB.
>>   GDB has restored the context to what it was before the call.
>>   To change this behavior use "set unwindonsignal off".
>>   Evaluation of the expression containing the function
>>   (cond_fail) will be abandoned.
>> 
>>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>>   30	  global_counter += 1;		/* Set breakpoint here.  */
>>   (gdb)
>> 
>> This is better.  GDB now reports a single stop at the location of the
>> breakpoint, which is where the inferior is actually located.  However,
>> by removing the first stop notification we have lost some potentially
>> useful information about which signal caused the inferior to stop.
>> 
>> To address this I've reworked the message that is printed to include
>> the signal information.  GDB now reports this:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>>         -ex 'break 30 if (cond_fail())' \
>>         -ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>>   Error in testing condition for breakpoint 1:
>>   The program being debugged received signal SIGSEGV, Segmentation fault
>>   while in a function called from GDB.  GDB has restored the context
>>   to what it was before the call.  To change this behavior use
>>   "set unwindonsignal off".  Evaluation of the expression containing
>>   the function (cond_fail) will be abandoned.
>> 
>>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>>   30	  global_counter += 1;		/* Set breakpoint here.  */
>>   (gdb)
>> 
>> This is better, the user now sees a single stop notification at the
>> correct location, and the error message describes which signal caused
>> the inferior function call to stop.
>> 
>> However, we have lost the information about where the signal
>> occurred.  I did consider trying to include this information in the
>> error message, but, in the end, I opted not too.  I wasn't sure it was
>> worth the effort.  If the user has selected to unwind on signal, then
>> surely this implies they maybe aren't interested in debugging failed
>> inferior calls, so, hopefully, just knowing the signal name will be
>> enough.  I figure we can always add this information in later if
>> there's a demand for it.
>
> Hi Andrew,
>
> Starting with this commit, I see this failure with the native-gdbserver
> board:
>
> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>
> Do you see it?

Hi Simon,

I've been away for a while, but I'm now back and will take a look at
this regression.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on
  2023-08-29 16:37   ` Simon Marchi
  2023-09-08  8:54     ` Andrew Burgess
@ 2023-09-08 10:03     ` Andrew Burgess
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-09-08 10:03 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: pedro

Simon Marchi <simon.marchi@polymtl.ca> writes:

> On 8/8/23 10:45, Andrew Burgess via Gdb-patches wrote:
>> This recent commit:
>> 
>>   commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
>>   Date:   Wed Jul 12 21:56:50 2023 +0100
>> 
>>       gdb: avoid double stop after failed breakpoint condition check
>> 
>> Addressed a problem where two MI stopped events would be reported if a
>> breakpoint condition failed due to a signal, this commit was a
>> replacement for this commit:
>> 
>>   commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
>>   Date:   Fri Oct 14 14:53:15 2022 +0100
>> 
>>       gdb: don't always print breakpoint location after failed condition check
>> 
>> which solved the two stop problem, but only for the CLI.  Before both
>> of these commits, if a b/p condition failed due to a signal then the
>> user would see two stops reported, the first at the location where the
>> signal occurred, and the second at the location of the breakpoint.
>> 
>> By default GDB remains at the location where the signal occurred, so
>> the second reported stop can be confusing, this is the problem that
>> commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
>> extended also address the issue for MI too.
>> 
>> However, while working on another patch I realised that there was a
>> problem with GDB after the above commits.  Neither of the above
>> commits considered 'set unwindonsignal on'.  With this setting on,
>> when an inferior function call fails with a signal GDB will unwind the
>> stack back to the location where the inferior function call started.
>> In the b/p case we're looking at, the stop should be reported at the
>> location of the breakpoint, not at the location where the signal
>> occurred, and this isn't what happens.
>> 
>> This commit fixes this by ensuring that when unwindonsignal is 'on',
>> GDB reports a single stop event at the location of the breakpoint,
>> this fixes things for both CLI and MI.
>> 
>> The function call_thread_fsm::should_notify_stop is called when the
>> inferior function call completes and GDB is figuring out if the user
>> should be notified about this stop event by calling normal_stop from
>> fetch_inferior_event in infrun.c.  If normal_stop is called, then this
>> notification will be for the location where the inferior call stopped,
>> which will be the location at which the signal occurred.
>> 
>> Prior to this commit, the only time that normal_stop was not called,
>> was if the inferior function call completed successfully, this was
>> controlled by ::should_notify_stop, which only turns false when the
>> inferior function call has completed successfully.
>> 
>> In this commit I have extended the logic in ::should_notify_stop.  Now
>> there are three cases in which ::should_notify_stop will return false,
>> and we will not announce the first stop (by calling normal_stop).
>> These three reasons are:
>> 
>>   1. If the inferior function call completes successfully, this is
>>   unchanged behaviour,
>> 
>>   2. If the inferior function call stopped due to a signal and 'set
>>   unwindonsignal on' is in effect, and
>> 
>>   3. If the inferior function call stopped due to an uncaught C++
>>   exception, and 'set unwind-on-terminating-exception on' is in
>>   effect.
>> 
>> However, if we don't call normal_stop then we need to call
>> async_enable_stdin in call_thread_fsm::should_stop.  Prior to this
>> commit this was only done for the case where the inferior function
>> call completed successfully.
>> 
>> In this commit I now call ::should_notify_stop and use this to
>> determine if we need to call async_enable_stdin.  With this done we
>> now call async_enable_stdin for each of the three cases listed above,
>> which means that GDB will exit wait_sync_command_done correctly (see
>> run_inferior_call in infcall.c).
>> 
>> With these two changes the problem is mostly resolved.  However, the
>> solution isn't ideal, we've still lost some information.
>> 
>> Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
>> 2e411b8c68eb:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>>         -ex 'break 30 if (cond_fail())' \
>>         -ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>> 
>>   Program received signal SIGSEGV, Segmentation fault.
>>   0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
>>   24	  return *p;			/* Crash here.  */
>>   Error in testing breakpoint condition:
>>   The program being debugged was signaled while in a function called from GDB.
>>   GDB has restored the context to what it was before the call.
>>   To change this behavior use "set unwindonsignal off".
>>   Evaluation of the expression containing the function
>>   (cond_fail) will be abandoned.
>> 
>>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>>   30	  global_counter += 1;		/* Set breakpoint here.  */
>>   (gdb)
>> 
>> In this state we see two stop notifications, the first is where the
>> signal occurred, while the second is where the breakpoint is located.
>> As GDB has unwound the stack (thanks to unwindonsignal) the second
>> stop notification reflects where the inferior is actually located.
>> 
>> Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
>> to this:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>> 	-ex 'break 30 if (cond_fail())' \
>> 	-ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>> 
>>   Program received signal SIGSEGV, Segmentation fault.
>>   0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
>>   24	  return *p;			/* Crash here.  */
>>   Error in testing condition for breakpoint 1:
>>   The program being debugged was signaled while in a function called from GDB.
>>   GDB has restored the context to what it was before the call.
>>   To change this behavior use "set unwindonsignal off".
>>   Evaluation of the expression containing the function
>>   (cond_fail) will be abandoned.
>>   (gdb) bt 1
>>   #0  foo () at /tmp/mi-condbreak-fail.c:30
>>   (More stack frames follow...)
>>   (gdb)
>> 
>> This is the broken state.  GDB is reports the SIGSEGV location, but
>> not the unwound breakpoint location.  The final 'bt 1' shows that the
>> inferior is not located in cond_fail, which is the only location GDB
>> reported, so this is clearly wrong.
>> 
>> After implementing the fixes described above we now get this
>> behaviour:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>>         -ex 'break 30 if (cond_fail())' \
>>         -ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>>   Error in testing breakpoint condition for breakpoint 1:
>>   The program being debugged was signaled while in a function called from GDB.
>>   GDB has restored the context to what it was before the call.
>>   To change this behavior use "set unwindonsignal off".
>>   Evaluation of the expression containing the function
>>   (cond_fail) will be abandoned.
>> 
>>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>>   30	  global_counter += 1;		/* Set breakpoint here.  */
>>   (gdb)
>> 
>> This is better.  GDB now reports a single stop at the location of the
>> breakpoint, which is where the inferior is actually located.  However,
>> by removing the first stop notification we have lost some potentially
>> useful information about which signal caused the inferior to stop.
>> 
>> To address this I've reworked the message that is printed to include
>> the signal information.  GDB now reports this:
>> 
>>   $ gdb -q /tmp/mi-condbreak-fail \
>>         -ex 'set unwindonsignal on' \
>>         -ex 'break 30 if (cond_fail())' \
>>         -ex 'run'
>>   Reading symbols from /tmp/mi-condbreak-fail...
>>   Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
>>   Starting program: /tmp/mi-condbreak-fail
>>   Error in testing condition for breakpoint 1:
>>   The program being debugged received signal SIGSEGV, Segmentation fault
>>   while in a function called from GDB.  GDB has restored the context
>>   to what it was before the call.  To change this behavior use
>>   "set unwindonsignal off".  Evaluation of the expression containing
>>   the function (cond_fail) will be abandoned.
>> 
>>   Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
>>   30	  global_counter += 1;		/* Set breakpoint here.  */
>>   (gdb)
>> 
>> This is better, the user now sees a single stop notification at the
>> correct location, and the error message describes which signal caused
>> the inferior function call to stop.
>> 
>> However, we have lost the information about where the signal
>> occurred.  I did consider trying to include this information in the
>> error message, but, in the end, I opted not too.  I wasn't sure it was
>> worth the effort.  If the user has selected to unwind on signal, then
>> surely this implies they maybe aren't interested in debugging failed
>> inferior calls, so, hopefully, just knowing the signal name will be
>> enough.  I figure we can always add this information in later if
>> there's a demand for it.
>
> Hi Andrew,
>
> Starting with this commit, I see this failure with the native-gdbserver
> board:
>
> FAIL: gdb.mi/mi-condbreak-throw.exp: unwind_on_exception=off: wait for stop (unexpected output)
>
> Do you see it?

Hi,

I pushed the minor testsuite only patch below to fix this issue.

Thanks,
Andrew

---

commit 932a49fff332ba4921dc9e38cf45bf65a301f2c6
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Sep 8 10:45:08 2023 +0100

    gdb/testsuite: fix gdb.mi/mi-condbreak-throw.exp failure
    
    In commit:
    
      commit 3ce8f906be7a55d8c0375e6d360cc53b456d86ae
      Date:   Tue Aug 8 10:45:20 2023 +0100
    
          gdb: MI stopped events when unwindonsignal is on
    
    a new test, gdb.mi/mi-condbreak-throw.exp, was added.  Unfortunately,
    this test would fail when using the native-gdbserver board (and other
    similar boards).
    
    The problem was that one of the expected output patterns included some
    output from the inferior.  When using the native-gdbserver board, this
    output is not printed to GDB's tty, but is instead printed to
    gdbserver's tty, the result is that the expected output no longer
    matches, and the test fails.
    
    Additionally, as the output is actually from the C++ runtime, rather
    than the test's source file, changes to the C++ runtime could cause
    the output to change.
    
    To solve both of these issues, in this commit, I'm removing the
    reference to the inferior's output, and replacing it with '.*', which
    will skip the output if it is present, but is equally happy if the
    output is not present.
    
    After this commit gdb.mi/mi-condbreak-throw.exp now passes on all
    boards, including native-gdbserver.

diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
index 4151fa18395..7f291244e53 100644
--- a/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
+++ b/gdb/testsuite/gdb.mi/mi-condbreak-throw.exp
@@ -97,8 +97,7 @@ proc run_test { unwind_on_exception } {
 
 	mi_gdb_test "" \
 	    [multi_line \
-		 "terminate called after throwing an instance of 'int'" \
-		 "~\"\\\\nProgram\"" \
+		 ".*~\"\\\\nProgram\"" \
 		 "~\" received signal SIGABRT, Aborted\\.\\\\n\"${out_ln_re}" \
 		 "\\*stopped,reason=\"signal-received\",signal-name=\"SIGABRT\"\[^\r\n\]+frame=\\{addr=\"$::hex\",\[^\r\n\]+\\}\[^\r\n\]+" \
 		 "&\"Error in testing condition for breakpoint $bpnum:\\\\n\"" \


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-09-08 10:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-08 14:45 [PATCH 0/2] Stop notifications when unwindonsignal is on Andrew Burgess
2023-08-08 14:45 ` [PATCH 1/2] gdb/testsuite: add mi_info_frame helper proc (and use it) Andrew Burgess
2023-08-08 14:45 ` [PATCH 2/2] gdb: MI stopped events when unwindonsignal is on Andrew Burgess
2023-08-10 15:06   ` Thiago Jung Bauermann
2023-08-14 14:50     ` Andrew Burgess
2023-08-15  0:32       ` Thiago Jung Bauermann
2023-08-16 14:28         ` Andrew Burgess
2023-08-18 21:56           ` Thiago Jung Bauermann
2023-08-23  9:33             ` Andrew Burgess
2023-08-29 16:37   ` Simon Marchi
2023-09-08  8:54     ` Andrew Burgess
2023-09-08 10:03     ` Andrew Burgess

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).