aboutsummaryrefslogtreecommitdiff
path: root/gdb/testsuite/gdb.multi/pending-bp-lib.c
diff options
context:
space:
mode:
authorAndrew Burgess <aburgess@redhat.com>2023-08-22 12:25:54 +0100
committerAndrew Burgess <aburgess@redhat.com>2024-09-07 21:48:35 +0100
commit85eb08c5f05b2e7d89009ee0388e88feb0d85fe2 (patch)
treeb86dd5b0a94083238536e2f9ab5ffcdf2e0b3835 /gdb/testsuite/gdb.multi/pending-bp-lib.c
parentc6b486755e020095710c7494d029577ca967a13a (diff)
downloadgdb-85eb08c5f05b2e7d89009ee0388e88feb0d85fe2.zip
gdb-85eb08c5f05b2e7d89009ee0388e88feb0d85fe2.tar.gz
gdb-85eb08c5f05b2e7d89009ee0388e88feb0d85fe2.tar.bz2
gdb: don't set breakpoint::pspace in create_breakpoint
I spotted this code within create_breakpoint: if ((type_wanted != bp_breakpoint && type_wanted != bp_hardware_breakpoint) || thread != -1) b->pspace = current_program_space; this code is only executed when creating a pending breakpoint, and sets the breakpoint::pspace member variable. The above code gained the 'thread != -1' clause with this commit: commit cc72b2a2da6d6372cbdb1d14639a5fce84e1a325 Date: Fri Dec 23 17:06:16 2011 +0000 Introduce gdb.FinishBreakpoint in Python While the type_wanted checks were added with this commit: commit f8eba3c61629b3c03ac1f33853eab4d8507adb9c Date: Tue Dec 6 18:54:43 2011 +0000 the "ambiguous linespec" series Before this breakpoint::pspace was set unconditionally. If we look at how breakpoint::pspace is used today, some breakpoint types specifically set this field, either in their constructors, or in a wrapper function that calls the constructor. So, the watchpoint type and its sub-class set this variable, as does the catchpoint type, and all it's sub-classes. However, code_breakpoint doesn't specifically set this field within its constructor, though some sub-classes of code_breakpoint (ada_catchpoint, exception_catchpoint, internal_breakpoint, and momentary_breakpoint) do set this field. When I examine all the places that breakpoint::pspace is used, I believe that in every place where it is expected that this field is set, the breakpoint type will be one that specifically sets this field. Next, I observe two problems with the existing code. First, the above code is only hit for pending breakpoints, there's no equivalent code for non-pending breakpoints. This opens up the possibility of GDB entering non-consistent states; if a breakpoint is first created pending and then later gets a location, the pspace field will be set, while if the breakpoint is immediately non-pending, then the pspace field will never be set. Second, if we look at how breakpoint::pspace is used in the function breakpoint_program_space_exit, we see that when a program space is removed, any breakpoint with breakpoint::pspace set to the removed program space, will be deleted. This makes sense, but does mean we need to ensure breakpoint::pspace is only set for breakpoints that apply to a single program space. So, if I create a pending dprintf breakpoint (type bp_dprintf) then the breakpoint::pspace variable will be set even though the dprintf is not really tied to that one program space. As a result, when the matching program space is removed the dprintf is incorrectly removed. Also, if I create a thread specific breakpoint, then, thanks to the 'thread != -1' clause the wrong program space will be stored in breakpoint::pspace (the current program space is always used, which might not be the program space that corresponds to the selected thread), as a result, the thread specific breakpoint will be deleted when the matching program space is removed. If we look at commit cc72b2a2da6d which added the 'thread != -1' clause, we can see this change was entirely redundant, the breakpoint::pspace is also set in bpfinishpy_init after create_breakpoint has been called. As such, I think we can safely drop the 'thread != -1' clause. For the other problems, I'm proposing to be pretty aggressive - I'd like to drop the breakpoint::pspace assignment completely from create_breakpoint. Having looked at how this variable is used, I believe that it is already set elsewhere in all the cases that it is needed. Maybe this code was needed at one time, but I can't see how it's needed any more. There's tests to expose the issues I've spotted with this code, and there's no regressions in testing.
Diffstat (limited to 'gdb/testsuite/gdb.multi/pending-bp-lib.c')
0 files changed, 0 insertions, 0 deletions