[v2] Handle bp deletion in Breakpoint.stop

Message ID 20250121102908.16681-1-tdevries@suse.de
State New
Headers
Series [v2] Handle bp deletion in Breakpoint.stop |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries Jan. 21, 2025, 10:29 a.m. UTC
  In the documentation of gdb.Breakpoint.stop [1] we read:
...
You should not alter the execution state of the inferior (i.e., step, next,
etc.), alter the current frame context (i.e., change the current active
frame), or alter, add or delete any breakpoint.
...

PR python/32423 reports that the failure mode of attempting to delete a
breakpoint in gdb.Breakpoint.stop is:
- actually deleting the breakpoint, followed by
- a segfault.

Improve the failure mode, by instead:
- not deleting the breakpoint, and
- throwing an error.

Add two new tests to test-case gdb.python/py-breakpoint.exp:
- one test checking that trying to self-delete from gdb.Breakpoint.stop throws
  an error, and
- one test checking that using the workaround mentioned here [2]: using
  "gdb.post_event(self.delete)" works as expected.

OTOH, while the documentation forbids it, test-case
gdb.python/py-finish-breakpoint.exp creates a breakpoint in
gdb.Breakpoint.stop, so this seems to be at least somewhat supported.

Fix this by no longer forbidding to create a breakpoint in
gdb.Breakpoint.stop.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32423

[1] https://sourceware.org/gdb/current/onlinedocs/gdb.html/Breakpoints-In-Python.html#index-Breakpoint_002estop
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=12802#c4
---
 gdb/breakpoint.c                           |  15 +++
 gdb/breakpoint.h                           |   4 +
 gdb/doc/python.texi                        |   2 +-
 gdb/infcall.c                              |   6 ++
 gdb/testsuite/gdb.python/py-breakpoint.exp | 113 +++++++++++++++++++++
 5 files changed, 139 insertions(+), 1 deletion(-)


base-commit: 2c77375c02bd09ff91308bfc52de9412fdf3035b
  

Comments

Tom de Vries Feb. 4, 2025, 1:11 p.m. UTC | #1
On 1/21/25 11:29, Tom de Vries wrote:
> In the documentation of gdb.Breakpoint.stop [1] we read:
> ...
> You should not alter the execution state of the inferior (i.e., step, next,
> etc.), alter the current frame context (i.e., change the current active
> frame), or alter, add or delete any breakpoint.
> ...
> 
> PR python/32423 reports that the failure mode of attempting to delete a
> breakpoint in gdb.Breakpoint.stop is:
> - actually deleting the breakpoint, followed by
> - a segfault.
> 
> Improve the failure mode, by instead:
> - not deleting the breakpoint, and
> - throwing an error.
> 
> Add two new tests to test-case gdb.python/py-breakpoint.exp:
> - one test checking that trying to self-delete from gdb.Breakpoint.stop throws
>    an error, and
> - one test checking that using the workaround mentioned here [2]: using
>    "gdb.post_event(self.delete)" works as expected.
> 
> OTOH, while the documentation forbids it, test-case
> gdb.python/py-finish-breakpoint.exp creates a breakpoint in
> gdb.Breakpoint.stop, so this seems to be at least somewhat supported.
> 
> Fix this by no longer forbidding to create a breakpoint in
> gdb.Breakpoint.stop.
> 
> Tested on x86_64-linux.
> 

Ping.

Thanks,
- Tom

> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32423
> 
> [1] https://sourceware.org/gdb/current/onlinedocs/gdb.html/Breakpoints-In-Python.html#index-Breakpoint_002estop
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=12802#c4
> ---
>   gdb/breakpoint.c                           |  15 +++
>   gdb/breakpoint.h                           |   4 +
>   gdb/doc/python.texi                        |   2 +-
>   gdb/infcall.c                              |   6 ++
>   gdb/testsuite/gdb.python/py-breakpoint.exp | 113 +++++++++++++++++++++
>   5 files changed, 139 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index e7fdeca91ff..f98a6578185 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5644,6 +5644,10 @@ bpstat_check_watchpoint (bpstat *bs)
>       }
>   }
>   
> +/* See breakpoint.h.  */
> +
> +bool prevent_breakpoint_deletion;
> +
>   /* For breakpoints that are currently marked as telling gdb to stop,
>      check conditions (condition proper, frame, thread and ignore count)
>      of breakpoint referred to by BS.  If we should not stop for this
> @@ -5654,6 +5658,11 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>   {
>     INFRUN_SCOPED_DEBUG_ENTER_EXIT;
>   
> +  /* Breakpoints are not allowed to be deleted while checking breakpoint
> +     conditions.  */
> +  scoped_restore restore_in_gdb_breakpoint_stop
> +    = make_scoped_restore (&prevent_breakpoint_deletion, true);
> +
>     const struct bp_location *bl;
>     struct breakpoint *b;
>     /* Assume stop.  */
> @@ -12581,6 +12590,12 @@ delete_breakpoint (struct breakpoint *bpt)
>   {
>     gdb_assert (bpt != NULL);
>   
> +  if (prevent_breakpoint_deletion)
> +    {
> +      error (_("Cannot delete breakpoint"));
> +      return;
> +    }
> +
>     /* Has this bp already been deleted?  This can happen because
>        multiple lists can hold pointers to bp's.  bpstat lists are
>        especial culprits.
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 94fba1cf602..f255b92e469 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -2103,4 +2103,8 @@ extern void enable_disable_bp_location (bp_location *loc, bool enable);
>   
>   extern void notify_breakpoint_modified (breakpoint *b);
>   
> +/* Whether breakpoints should not be deleted.  */
> +
> +extern bool prevent_breakpoint_deletion;
> +
>   #endif /* GDB_BREAKPOINT_H */
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index d66cae6f997..a1e64256f53 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -6702,7 +6702,7 @@ if one of the methods returns @code{True} but the others return
>   
>   You should not alter the execution state of the inferior (i.e.@:, step,
>   next, etc.), alter the current frame context (i.e.@:, change the current
> -active frame), or alter, add or delete any breakpoint.  As a general
> +active frame), or alter or delete any breakpoint.  As a general
>   rule, you should not alter any data within @value{GDBN} or the inferior
>   at this time.
>   
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index 6399278c6ae..ead9b27bd3b 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -1036,6 +1036,12 @@ call_function_by_hand_dummy (struct value *function,
>   {
>     INFCALL_SCOPED_DEBUG_ENTER_EXIT;
>   
> +  /* Breakpoints are not allowed to be deleted while checking breakpoint
> +     conditions, but inferior calls during breakpoint conditions require
> +     setting and deleting breakpoints.  */
> +  scoped_restore restore_in_gdb_breakpoint_stop
> +    = make_scoped_restore (&prevent_breakpoint_deletion, false);
> +
>     CORE_ADDR sp;
>     struct type *target_values_type;
>     function_call_return_method return_method = return_method_normal;
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index 6f9245c75c6..22b26ce2371 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -950,6 +950,117 @@ proc_with_prefix test_bkpt_auto_disable { } {
>       gdb_test "continue" "False.*" "auto-disabling after enable count reached"
>   }
>   
> +proc_with_prefix test_bkpt_stop_delete { } {
> +
> +    foreach_with_prefix type { gdb.Breakpoint gdb.FinishBreakpoint } {
> +	# Start with a fresh gdb.
> +	clean_restart $::testfile
> +
> +	if { ![runto_main] } {
> +	    return 0
> +	}
> +
> +	delete_breakpoints
> +
> +	gdb_test_multiline "Sub-class a breakpoint" \
> +	    "python" "" \
> +	    "class basic ($type):" "" \
> +	    "    def stop(self):" "" \
> +	    "        self.delete()" "" \
> +	    "        return True" "" \
> +	    "end" ""
> +
> +	set line_marker "Break at function add.";
> +	set bp_location [gdb_get_line_number $line_marker]
> +
> +	if { $type == "gdb.Breakpoint" } {
> +	    gdb_py_test_silent_cmd "python basic(\"$bp_location\")" \
> +		"Set breakpoint" 0
> +	} else {
> +	    gdb_breakpoint $bp_location
> +	    gdb_continue_to_breakpoint "continue to function add" \
> +		.*[string_to_regexp $line_marker].*
> +
> +	    gdb_py_test_silent_cmd "python basic()" \
> +		"Set breakpoint" 0
> +	}
> +
> +	set err_msg_prefix "Python Exception <class 'gdb.error'>"
> +	set err_msg "Cannot delete breakpoint"
> +	gdb_test "continue" \
> +	    [string_to_regexp "$err_msg_prefix: $err_msg"].* \
> +	    $err_msg
> +    }
> +}
> +
> +proc_with_prefix test_bkpt_stop_post_event_delete { } {
> +
> +    foreach_with_prefix type { gdb.Breakpoint gdb.FinishBreakpoint } {
> +	# Start with a fresh gdb.
> +	clean_restart $::testfile
> +
> +	if { ![runto_main] } {
> +	    return 0
> +	}
> +
> +	delete_breakpoints
> +
> +	gdb_test_multiline "Sub-class a breakpoint" \
> +	    "python" "" \
> +	    "class basic ($type):" "" \
> +	    "    def stop(self):" "" \
> +	    "        gdb.post_event(self.delete)" "" \
> +	    "        return True" "" \
> +	    "end" ""
> +
> +	set line_marker "Break at function add.";
> +	set bp_location [gdb_get_line_number $line_marker]
> +
> +	if { $type == "gdb.Breakpoint" } {
> +	    gdb_py_test_silent_cmd "python bp = basic(\"$bp_location\")" \
> +		"Set breakpoint" 0
> +	} else {
> +	    gdb_breakpoint $bp_location
> +	    gdb_continue_to_breakpoint "continue to function add" \
> +		.*[string_to_regexp $line_marker].*
> +
> +	    gdb_py_test_silent_cmd "python bp = basic()" \
> +		"Set breakpoint" 0
> +	}
> +
> +	set bp_num ""
> +	gdb_test_multiple "python print (bp.number)" "" {
> +	    -re -wrap "($::decimal)" {
> +		set bp_num $expect_out(1,string)
> +		pass $gdb_test_name
> +	    }
> +	}
> +
> +	if { $bp_num == "" } {
> +	    continue
> +	}
> +
> +	if { $type == "gdb.Breakpoint" } {
> +	    gdb_test "continue" "Breakpoint $::decimal, .*"
> +	} else {
> +	    # The gdb.FinishBreakpoint is a temporary breakpoint, so it's
> +	    # already auto-deleted by the time self.delete tries to delete it.
> +	    # Check for the error message.
> +	    set err_msg_prefix \
> +		[string_to_regexp "Python Exception <class 'RuntimeError'>"]
> +	    set err_msg "Breakpoint $::decimal is invalid"
> +	    set dot [string_to_regexp .]
> +	    set re "$err_msg_prefix: $err_msg$dot\r\n"
> +	    gdb_test -prompt "$::gdb_prompt $re$" \
> +		"continue"
> +	}
> +
> +	gdb_test "info breakpoints $bp_num" \
> +	    [string_to_regexp \
> +		 "No breakpoint, watchpoint, tracepoint, or catchpoint matching '$bp_num'."]
> +    }
> +}
> +
>   test_bkpt_basic
>   test_bkpt_deletion
>   test_bkpt_cond_and_cmds
> @@ -968,3 +1079,5 @@ test_bkpt_explicit_loc
>   test_bkpt_qualified
>   test_bkpt_probe
>   test_bkpt_auto_disable
> +test_bkpt_stop_delete
> +test_bkpt_stop_post_event_delete
> 
> base-commit: 2c77375c02bd09ff91308bfc52de9412fdf3035b
  
Aktemur, Tankut Baris Feb. 4, 2025, 3:43 p.m. UTC | #2
On Tuesday, January 21, 2025 11:29 AM, Tom de Vries wrote:
> In the documentation of gdb.Breakpoint.stop [1] we read:
> ...
> You should not alter the execution state of the inferior (i.e., step, next,
> etc.), alter the current frame context (i.e., change the current active
> frame), or alter, add or delete any breakpoint.
> ...
> 
> PR python/32423 reports that the failure mode of attempting to delete a
> breakpoint in gdb.Breakpoint.stop is:
> - actually deleting the breakpoint, followed by
> - a segfault.
> 
> Improve the failure mode, by instead:
> - not deleting the breakpoint, and
> - throwing an error.
> 
> Add two new tests to test-case gdb.python/py-breakpoint.exp:
> - one test checking that trying to self-delete from gdb.Breakpoint.stop throws
>   an error, and
> - one test checking that using the workaround mentioned here [2]: using
>   "gdb.post_event(self.delete)" works as expected.
> 
> OTOH, while the documentation forbids it, test-case
> gdb.python/py-finish-breakpoint.exp creates a breakpoint in
> gdb.Breakpoint.stop, so this seems to be at least somewhat supported.
> 
> Fix this by no longer forbidding to create a breakpoint in
> gdb.Breakpoint.stop.
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32423
> 
> [1] https://sourceware.org/gdb/current/onlinedocs/gdb.html/Breakpoints-In-Python.html#index-
> Breakpoint_002estop
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=12802#c4
> ---
>  gdb/breakpoint.c                           |  15 +++
>  gdb/breakpoint.h                           |   4 +
>  gdb/doc/python.texi                        |   2 +-
>  gdb/infcall.c                              |   6 ++
>  gdb/testsuite/gdb.python/py-breakpoint.exp | 113 +++++++++++++++++++++
>  5 files changed, 139 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index e7fdeca91ff..f98a6578185 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5644,6 +5644,10 @@ bpstat_check_watchpoint (bpstat *bs)
>      }
>  }
> 
> +/* See breakpoint.h.  */
> +
> +bool prevent_breakpoint_deletion;

Can we initialize this to false explicitly?  It improves readability, IMHO.

> +
>  /* For breakpoints that are currently marked as telling gdb to stop,
>     check conditions (condition proper, frame, thread and ignore count)
>     of breakpoint referred to by BS.  If we should not stop for this
> @@ -5654,6 +5658,11 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>  {
>    INFRUN_SCOPED_DEBUG_ENTER_EXIT;
> 
> +  /* Breakpoints are not allowed to be deleted while checking breakpoint
> +     conditions.  */
> +  scoped_restore restore_in_gdb_breakpoint_stop
> +    = make_scoped_restore (&prevent_breakpoint_deletion, true);
> +
>    const struct bp_location *bl;
>    struct breakpoint *b;
>    /* Assume stop.  */
> @@ -12581,6 +12590,12 @@ delete_breakpoint (struct breakpoint *bpt)
>  {
>    gdb_assert (bpt != NULL);
> 
> +  if (prevent_breakpoint_deletion)
> +    {
> +      error (_("Cannot delete breakpoint"));
> +      return;
> +    }
> +
>    /* Has this bp already been deleted?  This can happen because
>       multiple lists can hold pointers to bp's.  bpstat lists are
>       especial culprits.
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 94fba1cf602..f255b92e469 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -2103,4 +2103,8 @@ extern void enable_disable_bp_location (bp_location *loc, bool
> enable);
> 
>  extern void notify_breakpoint_modified (breakpoint *b);
> 
> +/* Whether breakpoints should not be deleted.  */
> +
> +extern bool prevent_breakpoint_deletion;
> +
>  #endif /* GDB_BREAKPOINT_H */
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index d66cae6f997..a1e64256f53 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -6702,7 +6702,7 @@ if one of the methods returns @code{True} but the others return
> 
>  You should not alter the execution state of the inferior (i.e.@:, step,
>  next, etc.), alter the current frame context (i.e.@:, change the current
> -active frame), or alter, add or delete any breakpoint.  As a general
> +active frame), or alter or delete any breakpoint.  As a general
>  rule, you should not alter any data within @value{GDBN} or the inferior
>  at this time.
> 
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index 6399278c6ae..ead9b27bd3b 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -1036,6 +1036,12 @@ call_function_by_hand_dummy (struct value *function,
>  {
>    INFCALL_SCOPED_DEBUG_ENTER_EXIT;
> 
> +  /* Breakpoints are not allowed to be deleted while checking breakpoint
> +     conditions, but inferior calls during breakpoint conditions require
> +     setting and deleting breakpoints.  */
> +  scoped_restore restore_in_gdb_breakpoint_stop
> +    = make_scoped_restore (&prevent_breakpoint_deletion, false);
> +
>    CORE_ADDR sp;
>    struct type *target_values_type;
>    function_call_return_method return_method = return_method_normal;
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-
> breakpoint.exp
> index 6f9245c75c6..22b26ce2371 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -950,6 +950,117 @@ proc_with_prefix test_bkpt_auto_disable { } {
>      gdb_test "continue" "False.*" "auto-disabling after enable count reached"
>  }
> 
> +proc_with_prefix test_bkpt_stop_delete { } {
> +
> +    foreach_with_prefix type { gdb.Breakpoint gdb.FinishBreakpoint } {
> +	# Start with a fresh gdb.
> +	clean_restart $::testfile
> +
> +	if { ![runto_main] } {
> +	    return 0
> +	}
> +
> +	delete_breakpoints
> +
> +	gdb_test_multiline "Sub-class a breakpoint" \
> +	    "python" "" \
> +	    "class basic ($type):" "" \
> +	    "    def stop(self):" "" \
> +	    "        self.delete()" "" \
> +	    "        return True" "" \
> +	    "end" ""
> +
> +	set line_marker "Break at function add.";
> +	set bp_location [gdb_get_line_number $line_marker]
> +
> +	if { $type == "gdb.Breakpoint" } {
> +	    gdb_py_test_silent_cmd "python basic(\"$bp_location\")" \
> +		"Set breakpoint" 0
> +	} else {
> +	    gdb_breakpoint $bp_location
> +	    gdb_continue_to_breakpoint "continue to function add" \
> +		.*[string_to_regexp $line_marker].*
> +
> +	    gdb_py_test_silent_cmd "python basic()" \
> +		"Set breakpoint" 0
> +	}
> +
> +	set err_msg_prefix "Python Exception <class 'gdb.error'>"
> +	set err_msg "Cannot delete breakpoint"
> +	gdb_test "continue" \
> +	    [string_to_regexp "$err_msg_prefix: $err_msg"].* \
> +	    $err_msg
> +    }
> +}
> +
> +proc_with_prefix test_bkpt_stop_post_event_delete { } {
> +
> +    foreach_with_prefix type { gdb.Breakpoint gdb.FinishBreakpoint } {

Starting here...

> +	# Start with a fresh gdb.
> +	clean_restart $::testfile
> +
> +	if { ![runto_main] } {
> +	    return 0
> +	}
> +
> +	delete_breakpoints
> +
> +	gdb_test_multiline "Sub-class a breakpoint" \
> +	    "python" "" \
> +	    "class basic ($type):" "" \
> +	    "    def stop(self):" "" \
> +	    "        gdb.post_event(self.delete)" "" \
> +	    "        return True" "" \
> +	    "end" ""
> +
> +	set line_marker "Break at function add.";
> +	set bp_location [gdb_get_line_number $line_marker]
> +
> +	if { $type == "gdb.Breakpoint" } {
> +	    gdb_py_test_silent_cmd "python bp = basic(\"$bp_location\")" \
> +		"Set breakpoint" 0
> +	} else {
> +	    gdb_breakpoint $bp_location
> +	    gdb_continue_to_breakpoint "continue to function add" \
> +		.*[string_to_regexp $line_marker].*
> +
> +	    gdb_py_test_silent_cmd "python bp = basic()" \
> +		"Set breakpoint" 0
> +	}

... upto here, we have the same code as above, except the action we take
inside the 'stop' method (i.e. `gdb.post_event` or `self.delete`).  Perhaps
we can extract the code section to a `proc setup {type action}` and reuse
in both places.

Regards,
-Baris

> +
> +	set bp_num ""
> +	gdb_test_multiple "python print (bp.number)" "" {
> +	    -re -wrap "($::decimal)" {
> +		set bp_num $expect_out(1,string)
> +		pass $gdb_test_name
> +	    }
> +	}
> +
> +	if { $bp_num == "" } {
> +	    continue
> +	}
> +
> +	if { $type == "gdb.Breakpoint" } {
> +	    gdb_test "continue" "Breakpoint $::decimal, .*"
> +	} else {
> +	    # The gdb.FinishBreakpoint is a temporary breakpoint, so it's
> +	    # already auto-deleted by the time self.delete tries to delete it.
> +	    # Check for the error message.
> +	    set err_msg_prefix \
> +		[string_to_regexp "Python Exception <class 'RuntimeError'>"]
> +	    set err_msg "Breakpoint $::decimal is invalid"
> +	    set dot [string_to_regexp .]
> +	    set re "$err_msg_prefix: $err_msg$dot\r\n"
> +	    gdb_test -prompt "$::gdb_prompt $re$" \
> +		"continue"
> +	}
> +
> +	gdb_test "info breakpoints $bp_num" \
> +	    [string_to_regexp \
> +		 "No breakpoint, watchpoint, tracepoint, or catchpoint matching '$bp_num'."]
> +    }
> +}
> +
>  test_bkpt_basic
>  test_bkpt_deletion
>  test_bkpt_cond_and_cmds
> @@ -968,3 +1079,5 @@ test_bkpt_explicit_loc
>  test_bkpt_qualified
>  test_bkpt_probe
>  test_bkpt_auto_disable
> +test_bkpt_stop_delete
> +test_bkpt_stop_post_event_delete
> 
> base-commit: 2c77375c02bd09ff91308bfc52de9412fdf3035b
> --
> 2.43.0

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index e7fdeca91ff..f98a6578185 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5644,6 +5644,10 @@  bpstat_check_watchpoint (bpstat *bs)
     }
 }
 
+/* See breakpoint.h.  */
+
+bool prevent_breakpoint_deletion;
+
 /* For breakpoints that are currently marked as telling gdb to stop,
    check conditions (condition proper, frame, thread and ignore count)
    of breakpoint referred to by BS.  If we should not stop for this
@@ -5654,6 +5658,11 @@  bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
 {
   INFRUN_SCOPED_DEBUG_ENTER_EXIT;
 
+  /* Breakpoints are not allowed to be deleted while checking breakpoint
+     conditions.  */
+  scoped_restore restore_in_gdb_breakpoint_stop
+    = make_scoped_restore (&prevent_breakpoint_deletion, true);
+
   const struct bp_location *bl;
   struct breakpoint *b;
   /* Assume stop.  */
@@ -12581,6 +12590,12 @@  delete_breakpoint (struct breakpoint *bpt)
 {
   gdb_assert (bpt != NULL);
 
+  if (prevent_breakpoint_deletion)
+    {
+      error (_("Cannot delete breakpoint"));
+      return;
+    }
+
   /* Has this bp already been deleted?  This can happen because
      multiple lists can hold pointers to bp's.  bpstat lists are
      especial culprits.
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 94fba1cf602..f255b92e469 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -2103,4 +2103,8 @@  extern void enable_disable_bp_location (bp_location *loc, bool enable);
 
 extern void notify_breakpoint_modified (breakpoint *b);
 
+/* Whether breakpoints should not be deleted.  */
+
+extern bool prevent_breakpoint_deletion;
+
 #endif /* GDB_BREAKPOINT_H */
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d66cae6f997..a1e64256f53 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -6702,7 +6702,7 @@  if one of the methods returns @code{True} but the others return
 
 You should not alter the execution state of the inferior (i.e.@:, step,
 next, etc.), alter the current frame context (i.e.@:, change the current
-active frame), or alter, add or delete any breakpoint.  As a general
+active frame), or alter or delete any breakpoint.  As a general
 rule, you should not alter any data within @value{GDBN} or the inferior
 at this time.
 
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 6399278c6ae..ead9b27bd3b 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1036,6 +1036,12 @@  call_function_by_hand_dummy (struct value *function,
 {
   INFCALL_SCOPED_DEBUG_ENTER_EXIT;
 
+  /* Breakpoints are not allowed to be deleted while checking breakpoint
+     conditions, but inferior calls during breakpoint conditions require
+     setting and deleting breakpoints.  */
+  scoped_restore restore_in_gdb_breakpoint_stop
+    = make_scoped_restore (&prevent_breakpoint_deletion, false);
+
   CORE_ADDR sp;
   struct type *target_values_type;
   function_call_return_method return_method = return_method_normal;
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index 6f9245c75c6..22b26ce2371 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -950,6 +950,117 @@  proc_with_prefix test_bkpt_auto_disable { } {
     gdb_test "continue" "False.*" "auto-disabling after enable count reached"
 }
 
+proc_with_prefix test_bkpt_stop_delete { } {
+
+    foreach_with_prefix type { gdb.Breakpoint gdb.FinishBreakpoint } {
+	# Start with a fresh gdb.
+	clean_restart $::testfile
+
+	if { ![runto_main] } {
+	    return 0
+	}
+
+	delete_breakpoints
+
+	gdb_test_multiline "Sub-class a breakpoint" \
+	    "python" "" \
+	    "class basic ($type):" "" \
+	    "    def stop(self):" "" \
+	    "        self.delete()" "" \
+	    "        return True" "" \
+	    "end" ""
+
+	set line_marker "Break at function add.";
+	set bp_location [gdb_get_line_number $line_marker]
+
+	if { $type == "gdb.Breakpoint" } {
+	    gdb_py_test_silent_cmd "python basic(\"$bp_location\")" \
+		"Set breakpoint" 0
+	} else {
+	    gdb_breakpoint $bp_location
+	    gdb_continue_to_breakpoint "continue to function add" \
+		.*[string_to_regexp $line_marker].*
+
+	    gdb_py_test_silent_cmd "python basic()" \
+		"Set breakpoint" 0
+	}
+
+	set err_msg_prefix "Python Exception <class 'gdb.error'>"
+	set err_msg "Cannot delete breakpoint"
+	gdb_test "continue" \
+	    [string_to_regexp "$err_msg_prefix: $err_msg"].* \
+	    $err_msg
+    }
+}
+
+proc_with_prefix test_bkpt_stop_post_event_delete { } {
+
+    foreach_with_prefix type { gdb.Breakpoint gdb.FinishBreakpoint } {
+	# Start with a fresh gdb.
+	clean_restart $::testfile
+
+	if { ![runto_main] } {
+	    return 0
+	}
+
+	delete_breakpoints
+
+	gdb_test_multiline "Sub-class a breakpoint" \
+	    "python" "" \
+	    "class basic ($type):" "" \
+	    "    def stop(self):" "" \
+	    "        gdb.post_event(self.delete)" "" \
+	    "        return True" "" \
+	    "end" ""
+
+	set line_marker "Break at function add.";
+	set bp_location [gdb_get_line_number $line_marker]
+
+	if { $type == "gdb.Breakpoint" } {
+	    gdb_py_test_silent_cmd "python bp = basic(\"$bp_location\")" \
+		"Set breakpoint" 0
+	} else {
+	    gdb_breakpoint $bp_location
+	    gdb_continue_to_breakpoint "continue to function add" \
+		.*[string_to_regexp $line_marker].*
+
+	    gdb_py_test_silent_cmd "python bp = basic()" \
+		"Set breakpoint" 0
+	}
+
+	set bp_num ""
+	gdb_test_multiple "python print (bp.number)" "" {
+	    -re -wrap "($::decimal)" {
+		set bp_num $expect_out(1,string)
+		pass $gdb_test_name
+	    }
+	}
+
+	if { $bp_num == "" } {
+	    continue
+	}
+
+	if { $type == "gdb.Breakpoint" } {
+	    gdb_test "continue" "Breakpoint $::decimal, .*"
+	} else {
+	    # The gdb.FinishBreakpoint is a temporary breakpoint, so it's
+	    # already auto-deleted by the time self.delete tries to delete it.
+	    # Check for the error message.
+	    set err_msg_prefix \
+		[string_to_regexp "Python Exception <class 'RuntimeError'>"]
+	    set err_msg "Breakpoint $::decimal is invalid"
+	    set dot [string_to_regexp .]
+	    set re "$err_msg_prefix: $err_msg$dot\r\n"
+	    gdb_test -prompt "$::gdb_prompt $re$" \
+		"continue"
+	}
+
+	gdb_test "info breakpoints $bp_num" \
+	    [string_to_regexp \
+		 "No breakpoint, watchpoint, tracepoint, or catchpoint matching '$bp_num'."]
+    }
+}
+
 test_bkpt_basic
 test_bkpt_deletion
 test_bkpt_cond_and_cmds
@@ -968,3 +1079,5 @@  test_bkpt_explicit_loc
 test_bkpt_qualified
 test_bkpt_probe
 test_bkpt_auto_disable
+test_bkpt_stop_delete
+test_bkpt_stop_post_event_delete