From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7689 invoked by alias); 18 Nov 2007 11:41:04 -0000 Received: (qmail 7680 invoked by uid 22791); 18 Nov 2007 11:41:02 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 18 Nov 2007 11:40:59 +0000 Received: (qmail 21565 invoked from network); 18 Nov 2007 11:40:56 -0000 Received: from unknown (HELO wind.local) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 18 Nov 2007 11:40:56 -0000 From: Vladimir Prus Date: Sun, 18 Nov 2007 11:41:00 -0000 Subject: [RFA] Stop infrun from tracking breakpoint insertion status. To: gdb-patches@sources.redhat.com X-TUID: 09662c99246415f7 X-Length: 10942 X-UID: 45 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200711181440.50407.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-11/txt/msg00358.txt.bz2 The infrun.c module goes to great pains in order to maintain "breakpoints are inserted" flag. However, it turns out that in a number of cases, it does not really need to check that flag and remove_breakpoints does no harm of no breakpoint is inserted; in a number of cases, the code actually is interested in a specific breakpoint, and in remaining 3 cases where "breakpoints are inserted" flag is essential for code logic, it's best to keep that flag in breakpoint.c. No regressions. OK? - Volodya The checks of breakpoints_inserted before calling remove_breakpoints are removed, as remove_breakpoint won't touch uninserted breakpoints. In a number of places, we're interested if a breakpoint is inserted at particular PC, and we now use breakpoint_inserted_here_p. In a few places, breakpoints_inserted indicates if infrun.c expects breakpoints to be inserted in the target, and that state is now kept in breakpoint.c * breakpoint.h (breakpoints_meant_to_be_inserted): New declaration. * breakpoint.c (breakpoints_meant_to_be_inserted_p): New. (breakpoints_meant_to_be_inserted): New. * infrun.c (breakpoints_inserted): Remove. (resume): Don't check for breakpoints_inserted before remove_hw_watchpoints. Use breakpoint_inserted_here_p. (proceed, init_wait_for_inferior): Don't set breakpoints_inserted. (handle_inferior_event): Don't use breakpoints_inserted. Use breakpoints_meant_to_be_inserted and breakpoints_inserted_here_p. (insert_step_resume_breakpoint_at_sal, keep_going): Use breakpoints_meant_to_be_inserted. Don't set breakpoints_inserted. (normal_stop): Don't check for breakpoints_inserted. Don't set breakpoints_inserted. --- gdb/breakpoint.c | 8 ++++++++ gdb/breakpoint.h | 7 +++++++ gdb/infrun.c | 53 ++++++++++++++--------------------------------------- 3 files changed, 29 insertions(+), 39 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index e81ec20..ef55a3d 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -296,6 +296,8 @@ int breakpoint_count; /* Pointer to current exception event record */ static struct exception_event_record *current_exception_event; +static int breakpoints_meant_to_be_inserted_p; + /* This function returns a pointer to the string representation of the pathname of the dynamically-linked library that has just been loaded. @@ -1312,6 +1314,12 @@ remove_breakpoints (void) } int +breakpoints_meant_to_be_inserted (void) +{ + return breakpoints_meant_to_be_inserted_p; +} + +int remove_hw_watchpoints (void) { struct bp_location *b; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index f13d41b..71515bc 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -862,4 +862,11 @@ extern int deprecated_remove_raw_breakpoint (void *); target. */ int watchpoints_triggered (struct target_waitstatus *); +/* Returns non-zero if insert_breakpoints was previously called, + and remove_breakpoints was not called after that. + This function allows to figure out if we meant that all breakpoints + be inserted in inferior. If so, any new breakpoints possibly + created must be inserted as well. */ +extern int breakpoints_meant_to_be_inserted (void); + #endif /* !defined (BREAKPOINT_H) */ diff --git a/gdb/infrun.c b/gdb/infrun.c index 85d889a..5191e0f 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -214,10 +214,6 @@ static unsigned char *signal_program; static struct cmd_list_element *stop_command; -/* Nonzero if breakpoints are now inserted in the inferior. */ - -static int breakpoints_inserted; - /* Function inferior was in as of last step command. */ static struct symbol *step_start_function; @@ -519,7 +515,7 @@ resume (int step, enum target_signal sig) Work around the problem by removing hardware watchpoints if a step is requested, GDB will check for a hardware watchpoint trigger after the step anyway. */ - if (CANNOT_STEP_HW_WATCHPOINTS && step && breakpoints_inserted) + if (CANNOT_STEP_HW_WATCHPOINTS && step) remove_hw_watchpoints (); @@ -634,7 +630,7 @@ a command like `return' or `jump' to continue execution.")); /* Most targets can step a breakpoint instruction, thus executing it normally. But if this one cannot, just continue and we will hit it anyway. */ - if (step && breakpoints_inserted && breakpoint_here_p (read_pc ())) + if (step && breakpoint_inserted_here_p (read_pc ())) step = 0; } target_resume (resume_ptid, step, sig); @@ -783,12 +779,7 @@ proceed (CORE_ADDR addr, enum target_signal siggnal, int step) Continue it automatically and insert breakpoints then. */ trap_expected = 1; else - { - insert_breakpoints (); - /* If we get here there was no call to error() in - insert breakpoints -- so they were inserted. */ - breakpoints_inserted = 1; - } + insert_breakpoints (); if (siggnal != TARGET_SIGNAL_DEFAULT) stop_signal = siggnal; @@ -884,7 +875,6 @@ init_wait_for_inferior (void) /* These are meaningless until the first time through wait_for_inferior. */ prev_pc = 0; - breakpoints_inserted = 0; breakpoint_init_inferior (inf_starting); /* Don't confuse first call to proceed(). */ @@ -1334,10 +1324,8 @@ handle_inferior_event (struct execution_control_state *ecs) /* Remove breakpoints, SOLIB_ADD might adjust breakpoint addresses via breakpoint_re_set. */ - breakpoints_were_inserted = breakpoints_inserted; - if (breakpoints_inserted) - remove_breakpoints (); - breakpoints_inserted = 0; + breakpoints_were_inserted = breakpoints_meant_to_be_inserted (); + remove_breakpoints (); /* Check for any newly added shared libraries if we're supposed to be adding them automatically. Switch @@ -1381,10 +1369,7 @@ handle_inferior_event (struct execution_control_state *ecs) /* Reinsert breakpoints and continue. */ if (breakpoints_were_inserted) - { - insert_breakpoints (); - breakpoints_inserted = 1; - } + insert_breakpoints (); } /* If we are skipping through a shell, or through shared library @@ -1670,7 +1655,7 @@ handle_inferior_event (struct execution_control_state *ecs) /* Check if a regular breakpoint has been hit before checking for a potential single step breakpoint. Otherwise, GDB will not see this breakpoint hit when stepping onto breakpoints. */ - if (breakpoints_inserted && breakpoint_here_p (stop_pc)) + if (breakpoint_inserted_here_p (stop_pc)) { ecs->random_signal = 0; if (!breakpoint_thread_match (stop_pc, ecs->ptid)) @@ -1783,7 +1768,6 @@ handle_inferior_event (struct execution_control_state *ecs) } else { /* Single step */ - breakpoints_inserted = 0; if (!ptid_equal (inferior_ptid, ecs->ptid)) context_switch (ecs); ecs->waiton_ptid = ecs->ptid; @@ -1945,7 +1929,7 @@ handle_inferior_event (struct execution_control_state *ecs) stack. */ if (stop_signal == TARGET_SIGNAL_TRAP - || (breakpoints_inserted + || (breakpoint_inserted_here_p (stop_pc) && (stop_signal == TARGET_SIGNAL_ILL || stop_signal == TARGET_SIGNAL_SEGV || stop_signal == TARGET_SIGNAL_EMT)) @@ -2076,8 +2060,8 @@ process_event_stop_test: stop_signal = TARGET_SIGNAL_0; if (prev_pc == read_pc () - && !breakpoints_inserted && breakpoint_here_p (read_pc ()) + && !breakpoint_inserted_here_p (read_pc ()) && step_resume_breakpoint == NULL) { /* We were just starting a new sequence, attempting to @@ -2150,7 +2134,6 @@ process_event_stop_test: fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n"); disable_longjmp_breakpoint (); remove_breakpoints (); - breakpoints_inserted = 0; if (!gdbarch_get_longjmp_target_p (current_gdbarch) || !gdbarch_get_longjmp_target (current_gdbarch, get_current_frame (), &jmp_buf_pc)) @@ -2176,7 +2159,6 @@ process_event_stop_test: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n"); remove_breakpoints (); - breakpoints_inserted = 0; disable_longjmp_breakpoint (); ecs->handling_longjmp = 0; /* FIXME */ if (what.main_action == BPSTAT_WHAT_CLEAR_LONGJMP_RESUME) @@ -2186,9 +2168,7 @@ process_event_stop_test: case BPSTAT_WHAT_SINGLE: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n"); - if (breakpoints_inserted) - remove_breakpoints (); - breakpoints_inserted = 0; + remove_breakpoints (); ecs->another_trap = 1; /* Still need to check other stuff, at least the case where we are stepping and step out of the right range. */ @@ -2250,7 +2230,6 @@ process_event_stop_test: to doing that. */ ecs->step_after_step_resume_breakpoint = 0; remove_breakpoints (); - breakpoints_inserted = 0; ecs->another_trap = 1; keep_going (ecs); return; @@ -2265,9 +2244,7 @@ process_event_stop_test: /* Remove breakpoints, we eventually want to step over the shlib event breakpoint, and SOLIB_ADD might adjust breakpoint addresses via breakpoint_re_set. */ - if (breakpoints_inserted) - remove_breakpoints (); - breakpoints_inserted = 0; + remove_breakpoints (); /* Check for any newly added shared libraries if we're supposed to be adding them automatically. Switch @@ -2870,7 +2847,7 @@ insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal, step_resume_breakpoint = set_momentary_breakpoint (sr_sal, sr_id, bp_step_resume); - if (breakpoints_inserted) + if (breakpoints_meant_to_be_inserted ()) insert_breakpoints (); } @@ -2970,7 +2947,7 @@ keep_going (struct execution_control_state *ecs) We're going to run this baby now! */ - if (!breakpoints_inserted && !ecs->another_trap) + if (!breakpoints_meant_to_be_inserted () && !ecs->another_trap) { /* Stop stepping when inserting breakpoints has failed. */ @@ -2979,7 +2956,6 @@ keep_going (struct execution_control_state *ecs) stop_stepping (ecs); return; } - breakpoints_inserted = 1; } trap_expected = ecs->another_trap; @@ -3172,7 +3148,7 @@ normal_stop (void) gdbarch_decr_pc_after_break needs to just go away. */ deprecated_update_frame_pc_hack (get_current_frame (), read_pc ()); - if (target_has_execution && breakpoints_inserted) + if (target_has_execution) { if (remove_breakpoints ()) { @@ -3183,7 +3159,6 @@ It might be running in another process.\n\ Further execution is probably impossible.\n")); } } - breakpoints_inserted = 0; /* Delete the breakpoint we stopped at, if it wants to be deleted. Delete any breakpoint that is to be deleted at the next stop. */ -- 1.5.3.5