From a1aa2221cbe340d23c6abf4d68cb509aa16cf8c0 Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Wed, 18 Jun 2014 10:25:47 +0100 Subject: Symptom: Using the test program gdb.base/foll-fork.c, with follow-fork-mode set to "child" and detach-on-fork set to "off", stepping or running past the fork call results in the child process running to completion, when it should just finish the single step. In addition, the breakpoint is not removed from the parent process, so if it is resumed it receives a SIGTRAP. Cause: No matter what the setting for detach-on-fork, when stepping past a fork, the single-step breakpoint (step_resume_breakpoint) is not handled correctly in the parent. The SR breakpoint is cloned for the child process, but before the clone is associated with the child it is treated as a duplicate of the original, associated wth the parent. This results in the insertion state of the original SR breakpoint and the clone being "swapped" by breakpoint.c:update_global_location_list, so that the clone is marked as inserted. In the case where the parent is not detached, the two breakpoints remain in that state. The breakpoint is never inserted in the child, because although the cloned SR breakpoint is associated with the child, it is marked as inserted. When the child is resumed, it runs to completion. The breakpoint is never removed from the parent, so that if it is resumed after the child exits, it gets a SIGTRAP. Here is the sequence of events: 1) handle_inferior_event: FORK event is recognized. 2) handle_inferior_event: detach_breakpoints removes all breakpoints from the child. 3) follow_fork: the parent SR breakpoint is cloned. Part of this procedure is to call update_global_location_list, which swaps the insertion state of the original and cloned SR breakpoints as part of ensuring that duplicate breakpoints are only inserted once. At this point the original SR breakpoint is not marked as inserted, and the clone is. The breakpoint is actually inserted in the parent but not the child. 4) follow_fork: the original breakpoint is deleted by calling delete_step_resume_breakpoint. Since the original is not marked as inserted, the actual breakpoint remains in the parent process. update_global_location_list is called again as part of the deletion. The clone is still associated with the parent, but since it is marked as enabled and inserted, the breakpoint is left in the parent. 5) follow_fork: if detach-on-fork is 'on', the actual breakpoint will be removed from the parent in target_detach, based on the cloned breakpoint still associated with the parent. Then the clone is no longer marked as inserted. In follow_inferior_reset_breakpoints the clone is associated with the child, and can be inserted. If detach-on-fork is 'off', the actual breakpoint in the parent is never removed (although the breakpoint had been deleted from the list). Since the clone continues to be marked 'inserted', the SR breakpoint is never inserted in the child. Fix: Set the cloned breakpoint as disabled from the moment it is created. This is done by modifying clone_momentary_breakpoint to take an additional argument, LOC_ENABLED, which is used as the value of the bp_location->enabled member. The clone must be disabled at that point because clone_momentary_breakpoint calls update_global_location_list, which will swap treat the clone as a duplicate of the original breakpoint if it is enabled. All the calls to clone_momentary_breakpoint had to be modified to pass '1' or '0'. I looked at implementing an enum for the enabled member, but concluded that readability would suffer because there are so many places it is used as a boolean, e.g. "if (bl->enabled)". In follow_inferior_reset_breakpoints the clone is set to enabled once it has been associated with the child process. With this, the bp_location 'inserted' member is maintained correctly throughout the follow-fork procedure and the behavior is as expected. The same treatment is given to the exception_resume_breakpoint when following a fork. Testing: Ran 'make check' on Linux x64. Along with the fix above, the coverage of the follow-fork test gdb.base/foll-fork.exp was expanded to: 1) cover all the combinations of values for follow-fork-mode and detach-on-fork 2) make sure that both user breakpoints and single-step breakpoints are propagated correctly to the child 3) check that the inferior list has the expected contents after following the fork. 4) check that unfollowed, undetached inferiors can be resumed. gdb/ 2014-06-18 Don Breazeal * breakpoint.c (set_longjmp_breakpoint): Call momentary_breakpoint_from_master with additional argument. (set_longjmp_breakpoint_for_call_dummy): Call momentary_breakpoint_from_master with additional argument. (set_std_terminate_breakpoint): Call momentary_breakpoint_from_master with additional argument. (momentary_breakpoint_from_master): Add argument to function definition and use it to initialize structure member flag. (clone_momentary_breakpoint): Call momentary_breakpoint_from_master with additional argument. * infrun.c (follow_inferior_reset_breakpoints): Clear structure member flags set in momentary_breakpoint_from_master. gdb/testsuite/ 2014-06-18 Don Breazeal * gdb.base/foll-fork.exp (default_fork_parent_follow): Deleted procedure. (explicit_fork_parent_follow): Deleted procedure. (explicit_fork_child_follow): Deleted procedure. (test_follow_fork): New procedure. (do_fork_tests): Replace calls to deleted procedures with calls to test_follow_fork and reset GDB for subsequent procedure calls. --- gdb/breakpoint.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'gdb/breakpoint.c') diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 2240f08..f3e6283 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -154,7 +154,8 @@ struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch, static struct breakpoint * momentary_breakpoint_from_master (struct breakpoint *orig, enum bptype type, - const struct breakpoint_ops *ops); + const struct breakpoint_ops *ops, + int loc_enabled); static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, int); @@ -7390,7 +7391,7 @@ set_longjmp_breakpoint (struct thread_info *tp, struct frame_id frame) /* longjmp_breakpoint_ops ensures INITIATING_FRAME is cleared again after their removal. */ clone = momentary_breakpoint_from_master (b, type, - &longjmp_breakpoint_ops); + &longjmp_breakpoint_ops, 1); clone->thread = thread; } @@ -7440,7 +7441,8 @@ set_longjmp_breakpoint_for_call_dummy (void) struct breakpoint *new_b; new_b = momentary_breakpoint_from_master (b, bp_longjmp_call_dummy, - &momentary_breakpoint_ops); + &momentary_breakpoint_ops, + 1); new_b->thread = pid_to_thread_id (inferior_ptid); /* Link NEW_B into the chain of RETVAL breakpoints. */ @@ -7533,7 +7535,7 @@ set_std_terminate_breakpoint (void) && b->type == bp_std_terminate_master) { momentary_breakpoint_from_master (b, bp_std_terminate, - &momentary_breakpoint_ops); + &momentary_breakpoint_ops, 1); } } @@ -9052,13 +9054,14 @@ set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal, } /* Make a momentary breakpoint based on the master breakpoint ORIG. - The new breakpoint will have type TYPE, and use OPS as it - breakpoint_ops. */ + The new breakpoint will have type TYPE, use OPS as its + breakpoint_ops, and will set enabled to LOC_ENABLED. */ static struct breakpoint * momentary_breakpoint_from_master (struct breakpoint *orig, enum bptype type, - const struct breakpoint_ops *ops) + const struct breakpoint_ops *ops, + int loc_enabled) { struct breakpoint *copy; @@ -9074,6 +9077,7 @@ momentary_breakpoint_from_master (struct breakpoint *orig, copy->loc->probe = orig->loc->probe; copy->loc->line_number = orig->loc->line_number; copy->loc->symtab = orig->loc->symtab; + copy->loc->enabled = loc_enabled; copy->frame_id = orig->frame_id; copy->thread = orig->thread; copy->pspace = orig->pspace; @@ -9096,7 +9100,7 @@ clone_momentary_breakpoint (struct breakpoint *orig) if (orig == NULL) return NULL; - return momentary_breakpoint_from_master (orig, orig->type, orig->ops); + return momentary_breakpoint_from_master (orig, orig->type, orig->ops, 0); } struct breakpoint * -- cgit v1.1