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

[Tom de Vries <tdevries@suse.de>]

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