[PATCH] [gdb/python] Fix gdb.python/py-finish-breakpoint2.exp for -m32

[PATCH] [gdb/python] Fix gdb.python/py-finish-breakpoint2.exp for -m32

[Tom de Vries]

[ Partial resubmission of an earlier submission by Andrew (
https://sourceware.org/pipermail/gdb-patches/2012-September/096347.html ), so
listing him as co-author. ]

With x86_64-linux and target board unix/-m32, we have:
...
(gdb) continue^M
Continuing.^M
Exception #10^M
^M
Breakpoint 3, throw_exception_1 (e=10) at py-finish-breakpoint2.cc:23^M
23        throw new int (e);^M
(gdb) FAIL: gdb.python/py-finish-breakpoint2.exp: \
  check FinishBreakpoint in catch()
...

The following scenario happens:
- set breakpoint in throw_exception_1, a function that throws an exception
- continue
- hit breakpoint, with call stack main.c:38 -> throw_exception_1
- set a finish breakpoint
- continue
- hit the breakpoint again, with call stack main.c:48 -> throw_exception
  -> throw_exception_1

Due to the exception, the function call did not properly terminate, and the
finish breakpoint didn't trigger.  This is expected behaviour.

However, the intention is that gdb detects this situation at the next stop
and calls the out_of_scope callback, which would result here in this test-case
in a rather confusing "exception did not finish" message.  So the problem is
that this message doesn't show up, in other words, the out_of_scope callback
is not called.

[ Note that the fact that the situation is detected only at the next stop
(wherever that happens to be could be) could be improved upon, and the earlier
submission did that by setting a longjmp breakpoint.  But I'm considering this
problem out-of-scope for this patch. ]

Note that the message does show up later, at thread exit:
...
[Inferior 1 (process 20046) exited with code 0236]^M
exception did not finish ...^M
...

The decision on whether to call the out_of_scope call back is taken in
bpfinishpy_detect_out_scope_cb, and the interesting bit is here:
...
             if (b->pspace == current_inferior ()->pspace
                 && (!target_has_registers ()
                     || frame_find_by_id (b->frame_id) == NULL))
               bpfinishpy_out_of_scope (finish_bp);
...

In the case of the thread exit, the callback triggers because
target_has_registers () == 0.

So why doesn't the callback trigger in the case of the breakpoint?

Well, the b->frame_id is the frame_id of the frame of main (the frame
in which the finish breakpoint is supposed to trigger), so AFAIU
frame_find_by_id (b->frame_id) == NULL will only be true once we've
left main, at which point I guess we don't stop till thread exit.

Fix this by saving the frame in which the finish breakpoint was created, and
using frame_find_by_id () == NULL on that frame instead, such that we have:
...
(gdb) continue^M
Continuing.^M
Exception #10^M
^M
Breakpoint 3, throw_exception_1 (e=10) at py-finish-breakpoint2.cc:23^M
23        throw new int (e);^M
exception did not finish ...^M
(gdb) FAIL: gdb.python/py-finish-breakpoint2.exp: \
  check FinishBreakpoint in catch()
...

Still, the test-case is failing because it's setup to match the behaviour that
we get on x86_64-linux with target board unix/-m64:
...
(gdb) continue^M
Continuing.^M
Exception #10^M
stopped at ExceptionFinishBreakpoint^M
(gdb) PASS: gdb.python/py-finish-breakpoint2.exp: \
  check FinishBreakpoint in catch()
...

So what happens here?  Again, due to the exception, the function call did not
properly terminate, but the finish breakpoint still triggers.  This is somewhat
unexpected.  This happens because it just so happens to be that the frame
return address at which the breakpoint is set, is also the first instruction
after the exception has been handled.  This is a know problem, filed as
PR29909, so KFAIL it, and modify the test-case to expect the out_of_scope
callback.

Also add a breakpoint after setting the finish breakpoint but before throwing
the exception, to check that we don't call the out_of_scope callback too early.

Tested on x86_64-linux, with target boards unix/-m32.

Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
PR python/27247
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27247
---
 gdb/python/py-finishbreakpoint.c              |  9 +++-
 .../gdb.python/py-finish-breakpoint2.cc       |  5 +-
 .../gdb.python/py-finish-breakpoint2.exp      | 47 +++++++++++++++++--
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 0b5dbeb5413..ea6662f0a86 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -55,6 +55,10 @@ struct finish_breakpoint_object
      the function; Py_None if the value is not computable; NULL if GDB is
      not stopped at a FinishBreakpoint.  */
   PyObject *return_value;
+
+  /* The initiating frame for this operation, used to decide when we have
+     left this frame.  */
+  struct frame_id initiating_frame;
 };
 
 extern PyTypeObject finish_breakpoint_object_type
@@ -310,6 +314,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 
   self_bpfinish->py_bp.bp->frame_id = frame_id;
   self_bpfinish->py_bp.is_finish_bp = 1;
+  self_bpfinish->initiating_frame = get_frame_id (frame);
 
   /* Bind the breakpoint with the current program space.  */
   self_bpfinish->py_bp.bp->pspace = current_program_space;
@@ -360,9 +365,11 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b,
 	{
 	  try
 	    {
+	      struct frame_id initiating_frame = finish_bp->initiating_frame;
+
 	      if (b->pspace == current_inferior ()->pspace
 		  && (!target_has_registers ()
-		      || frame_find_by_id (b->frame_id) == NULL))
+		      || frame_find_by_id (initiating_frame) == NULL))
 		bpfinishpy_out_of_scope (finish_bp);
 	    }
 	  catch (const gdb_exception &except)
diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc b/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
index 84234238f74..7e53d5393ae 100644
--- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
@@ -17,9 +17,13 @@
 
 #include <iostream>
 
+int i;
+
 void
 throw_exception_1 (int e)
 {
+  i += 1; /* Finish breakpoint is set here.  */
+  i += 1; /* Break before exception.  */
   throw new int (e);
 }
 
@@ -32,7 +36,6 @@ throw_exception (int e)
 int
 main (void)
 {
-  int i;
   try
     {
       throw_exception_1 (10);
diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
index 30758473d40..bd1e96bb4fe 100644
--- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
+++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
@@ -38,21 +38,60 @@ set pyfile [gdb_remote_download host \
 # Check FinishBreakpoints against C++ exceptions
 #
 
+gdb_breakpoint [gdb_get_line_number "Break before exception"]
 gdb_breakpoint [gdb_get_line_number "Break after exception 2"]
 
 gdb_test "source $pyfile" ".*Python script imported.*" \
          "import python scripts"
          
 gdb_breakpoint "throw_exception_1"
+
+#
+# Part 1.
+#
+
 gdb_test "continue" "Breakpoint .*throw_exception_1.*" "run to exception 1"
 
-gdb_test "python print (len(gdb.breakpoints()))" "3" "check BP count"
+# Count breakpoints before setting finishbreakpoint.
+gdb_test "python print (len(gdb.breakpoints()))" "4" "check BP count"
+
 gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" \
     "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
-gdb_test "continue" ".*stopped at ExceptionFinishBreakpoint.*" "check FinishBreakpoint in catch()"
-gdb_test "python print (len(gdb.breakpoints()))" "3" "check finish BP removal"
 
-gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second exception"
+gdb_test "continue" "Break before exception.*" \
+    "break before exception"
+
+set need_continue 0
+gdb_test_multiple "continue" "finishBP out-of-scope" {
+    -re -wrap "exception did not finish.*" {
+	# Out-of-scope.  For instance on x86_64 with unix/-m32.
+	pass $gdb_test_name
+    }
+    -re -wrap "stopped at ExceptionFinishBreakpoint.*" {
+	# Triggered despite the fact that the function call never finished.
+	# It just so happens to be that the frame return address at which the
+	# breakpoint is set, is also the first instruction after the exception
+	# has been handled.  For instance on x86_64 with unix/-m64.
+	kfail python/29909 $gdb_test_name
+	set need_continue 1
+    }
+}
+
+# Count breakpoints, check that the finishbreakpoint has been removed.
+gdb_test "python print (len(gdb.breakpoints()))" "4" "check finish BP removal"
+
+#
+# Part 2.
+#
+
+if { $need_continue } {
+    gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" \
+	"continue to second exception"
+}
 gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" \
     "init ExceptionFinishBreakpoint" "set FinishBP after the exception again"
+
+gdb_test "continue" "Break before exception.*" \
+    "break before exception again"
+
 gdb_test "continue" ".*exception did not finish.*" "FinishBreakpoint with exception thrown not caught"

base-commit: fa501b69309ccb03ec957101f24109ed7f737733
-- 
2.35.3

[committed] [gdb/python] Fix gdb.python/py-finish-breakpoint2.exp for -m32

[Tom de Vries]

On 12/16/22 14:22, Tom de Vries via Gdb-patches wrote:
> [ Partial resubmission of an earlier submission by Andrew (
> https://sourceware.org/pipermail/gdb-patches/2012-September/096347.html ), so
> listing him as co-author. ]
> 
> With x86_64-linux and target board unix/-m32, we have:
> ...
> (gdb) continue^M
> Continuing.^M
> Exception #10^M
> ^M
> Breakpoint 3, throw_exception_1 (e=10) at py-finish-breakpoint2.cc:23^M
> 23        throw new int (e);^M
> (gdb) FAIL: gdb.python/py-finish-breakpoint2.exp: \
>    check FinishBreakpoint in catch()
> ...
> 
> The following scenario happens:
> - set breakpoint in throw_exception_1, a function that throws an exception
> - continue
> - hit breakpoint, with call stack main.c:38 -> throw_exception_1
> - set a finish breakpoint
> - continue
> - hit the breakpoint again, with call stack main.c:48 -> throw_exception
>    -> throw_exception_1
> 
> Due to the exception, the function call did not properly terminate, and the
> finish breakpoint didn't trigger.  This is expected behaviour.
> 
> However, the intention is that gdb detects this situation at the next stop
> and calls the out_of_scope callback, which would result here in this test-case
> in a rather confusing "exception did not finish" message.  So the problem is
> that this message doesn't show up, in other words, the out_of_scope callback
> is not called.
> 
> [ Note that the fact that the situation is detected only at the next stop
> (wherever that happens to be could be) could be improved upon, and the earlier
> submission did that by setting a longjmp breakpoint.  But I'm considering this
> problem out-of-scope for this patch. ]
> 
> Note that the message does show up later, at thread exit:
> ...
> [Inferior 1 (process 20046) exited with code 0236]^M
> exception did not finish ...^M
> ...
> 
> The decision on whether to call the out_of_scope call back is taken in
> bpfinishpy_detect_out_scope_cb, and the interesting bit is here:
> ...
>               if (b->pspace == current_inferior ()->pspace
>                   && (!target_has_registers ()
>                       || frame_find_by_id (b->frame_id) == NULL))
>                 bpfinishpy_out_of_scope (finish_bp);
> ...
> 
> In the case of the thread exit, the callback triggers because
> target_has_registers () == 0.
> 
> So why doesn't the callback trigger in the case of the breakpoint?
> 
> Well, the b->frame_id is the frame_id of the frame of main (the frame
> in which the finish breakpoint is supposed to trigger), so AFAIU
> frame_find_by_id (b->frame_id) == NULL will only be true once we've
> left main, at which point I guess we don't stop till thread exit.
> 
> Fix this by saving the frame in which the finish breakpoint was created, and
> using frame_find_by_id () == NULL on that frame instead, such that we have:
> ...
> (gdb) continue^M
> Continuing.^M
> Exception #10^M
> ^M
> Breakpoint 3, throw_exception_1 (e=10) at py-finish-breakpoint2.cc:23^M
> 23        throw new int (e);^M
> exception did not finish ...^M
> (gdb) FAIL: gdb.python/py-finish-breakpoint2.exp: \
>    check FinishBreakpoint in catch()
> ...
> 
> Still, the test-case is failing because it's setup to match the behaviour that
> we get on x86_64-linux with target board unix/-m64:
> ...
> (gdb) continue^M
> Continuing.^M
> Exception #10^M
> stopped at ExceptionFinishBreakpoint^M
> (gdb) PASS: gdb.python/py-finish-breakpoint2.exp: \
>    check FinishBreakpoint in catch()
> ...
> 
> So what happens here?  Again, due to the exception, the function call did not
> properly terminate, but the finish breakpoint still triggers.  This is somewhat
> unexpected.  This happens because it just so happens to be that the frame
> return address at which the breakpoint is set, is also the first instruction
> after the exception has been handled.  This is a know problem, filed as
> PR29909, so KFAIL it, and modify the test-case to expect the out_of_scope
> callback.
> 
> Also add a breakpoint after setting the finish breakpoint but before throwing
> the exception, to check that we don't call the out_of_scope callback too early.
> 
> Tested on x86_64-linux, with target boards unix/-m32.
> 

Committed.

Thanks,
- Tom

> Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
> PR python/27247
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27247
> ---
>   gdb/python/py-finishbreakpoint.c              |  9 +++-
>   .../gdb.python/py-finish-breakpoint2.cc       |  5 +-
>   .../gdb.python/py-finish-breakpoint2.exp      | 47 +++++++++++++++++--
>   3 files changed, 55 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
> index 0b5dbeb5413..ea6662f0a86 100644
> --- a/gdb/python/py-finishbreakpoint.c
> +++ b/gdb/python/py-finishbreakpoint.c
> @@ -55,6 +55,10 @@ struct finish_breakpoint_object
>        the function; Py_None if the value is not computable; NULL if GDB is
>        not stopped at a FinishBreakpoint.  */
>     PyObject *return_value;
> +
> +  /* The initiating frame for this operation, used to decide when we have
> +     left this frame.  */
> +  struct frame_id initiating_frame;
>   };
>   
>   extern PyTypeObject finish_breakpoint_object_type
> @@ -310,6 +314,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>   
>     self_bpfinish->py_bp.bp->frame_id = frame_id;
>     self_bpfinish->py_bp.is_finish_bp = 1;
> +  self_bpfinish->initiating_frame = get_frame_id (frame);
>   
>     /* Bind the breakpoint with the current program space.  */
>     self_bpfinish->py_bp.bp->pspace = current_program_space;
> @@ -360,9 +365,11 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b,
>   	{
>   	  try
>   	    {
> +	      struct frame_id initiating_frame = finish_bp->initiating_frame;
> +
>   	      if (b->pspace == current_inferior ()->pspace
>   		  && (!target_has_registers ()
> -		      || frame_find_by_id (b->frame_id) == NULL))
> +		      || frame_find_by_id (initiating_frame) == NULL))
>   		bpfinishpy_out_of_scope (finish_bp);
>   	    }
>   	  catch (const gdb_exception &except)
> diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc b/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
> index 84234238f74..7e53d5393ae 100644
> --- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
> +++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.cc
> @@ -17,9 +17,13 @@
>   
>   #include <iostream>
>   
> +int i;
> +
>   void
>   throw_exception_1 (int e)
>   {
> +  i += 1; /* Finish breakpoint is set here.  */
> +  i += 1; /* Break before exception.  */
>     throw new int (e);
>   }
>   
> @@ -32,7 +36,6 @@ throw_exception (int e)
>   int
>   main (void)
>   {
> -  int i;
>     try
>       {
>         throw_exception_1 (10);
> diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
> index 30758473d40..bd1e96bb4fe 100644
> --- a/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
> +++ b/gdb/testsuite/gdb.python/py-finish-breakpoint2.exp
> @@ -38,21 +38,60 @@ set pyfile [gdb_remote_download host \
>   # Check FinishBreakpoints against C++ exceptions
>   #
>   
> +gdb_breakpoint [gdb_get_line_number "Break before exception"]
>   gdb_breakpoint [gdb_get_line_number "Break after exception 2"]
>   
>   gdb_test "source $pyfile" ".*Python script imported.*" \
>            "import python scripts"
>            
>   gdb_breakpoint "throw_exception_1"
> +
> +#
> +# Part 1.
> +#
> +
>   gdb_test "continue" "Breakpoint .*throw_exception_1.*" "run to exception 1"
>   
> -gdb_test "python print (len(gdb.breakpoints()))" "3" "check BP count"
> +# Count breakpoints before setting finishbreakpoint.
> +gdb_test "python print (len(gdb.breakpoints()))" "4" "check BP count"
> +
>   gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" \
>       "init ExceptionFinishBreakpoint" "set FinishBP after the exception"
> -gdb_test "continue" ".*stopped at ExceptionFinishBreakpoint.*" "check FinishBreakpoint in catch()"
> -gdb_test "python print (len(gdb.breakpoints()))" "3" "check finish BP removal"
>   
> -gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" "continue to second exception"
> +gdb_test "continue" "Break before exception.*" \
> +    "break before exception"
> +
> +set need_continue 0
> +gdb_test_multiple "continue" "finishBP out-of-scope" {
> +    -re -wrap "exception did not finish.*" {
> +	# Out-of-scope.  For instance on x86_64 with unix/-m32.
> +	pass $gdb_test_name
> +    }
> +    -re -wrap "stopped at ExceptionFinishBreakpoint.*" {
> +	# Triggered despite the fact that the function call never finished.
> +	# It just so happens to be that the frame return address at which the
> +	# breakpoint is set, is also the first instruction after the exception
> +	# has been handled.  For instance on x86_64 with unix/-m64.
> +	kfail python/29909 $gdb_test_name
> +	set need_continue 1
> +    }
> +}
> +
> +# Count breakpoints, check that the finishbreakpoint has been removed.
> +gdb_test "python print (len(gdb.breakpoints()))" "4" "check finish BP removal"
> +
> +#
> +# Part 2.
> +#
> +
> +if { $need_continue } {
> +    gdb_test "continue" ".*Breakpoint.* throw_exception_1.*" \
> +	"continue to second exception"
> +}
>   gdb_test "python ExceptionFinishBreakpoint(gdb.newest_frame())" \
>       "init ExceptionFinishBreakpoint" "set FinishBP after the exception again"
> +
> +gdb_test "continue" "Break before exception.*" \
> +    "break before exception again"
> +
>   gdb_test "continue" ".*exception did not finish.*" "FinishBreakpoint with exception thrown not caught"
> 
> base-commit: fa501b69309ccb03ec957101f24109ed7f737733