From cabd4da684520537eb21d0d0af95cf5ba3efff30 Mon Sep 17 00:00:00 2001 From: Jim Kingdon Date: Fri, 9 Apr 1993 03:17:45 +0000 Subject: [PATCH] * breakpoint.h (struct bpstat): Remove momentary field. Remove bpstat_momentary_breakpoint. This was always kludgy and is no longer used. * breakpoint.h: Add enum bpstat_what. breakpoint.h (struct bpstat), breakpoint.c (bpstat_stop_status): stop and print fields of bpstat now per-breakpoint, not just one for the whole chain. breakpoint.{c,h} (bpstat_what): New function. breakpoint.h: Remove bpstat_stop and bpstat_should_print. infrun.c: Replace switch (stop_bpstat->breakpoint_at->type) with call to bpstat_what. README: Remove watchpoint/breakpoint bug from known bugs. * breakpoint.h: Prototype bpstat_find_breakpoint. --- gdb/ChangeLog | 18 +++++ gdb/breakpoint.c | 186 ++++++++++++++++++++++++++++++++++++++--------- gdb/breakpoint.h | 74 +++++++++++++++---- 3 files changed, 226 insertions(+), 52 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5629727996..618f527b36 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,21 @@ +Thu Apr 8 10:15:10 1993 Jim Kingdon (kingdon@lioth.cygnus.com) + + * breakpoint.h (struct bpstat): Remove momentary field. + Remove bpstat_momentary_breakpoint. This was always kludgy + and is no longer used. + + * breakpoint.h: Add enum bpstat_what. + breakpoint.h (struct bpstat), breakpoint.c (bpstat_stop_status): + stop and print fields of bpstat now per-breakpoint, not just + one for the whole chain. + breakpoint.{c,h} (bpstat_what): New function. + breakpoint.h: Remove bpstat_stop and bpstat_should_print. + infrun.c: Replace switch (stop_bpstat->breakpoint_at->type) + with call to bpstat_what. + README: Remove watchpoint/breakpoint bug from known bugs. + + * breakpoint.h: Prototype bpstat_find_breakpoint. + Thu Apr 8 16:01:21 1993 Fred Fish (fnf@cygnus.com) * symtab.c (find_methods, gdb_mangle_name): Note that functions diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 678ea2b285..05e787e616 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -705,16 +705,6 @@ print_it_normal (bs) || (bs->breakpoint_at->type != bp_breakpoint && bs->breakpoint_at->type != bp_watchpoint)) return 0; - - /* If bpstat_stop_status says don't print, OK, we won't. An example - circumstance is when we single-stepped for both a watchpoint and - for a "stepi" instruction. The bpstat says that the watchpoint - explains the stop, but we shouldn't print because the watchpoint's - value didn't change -- and the real reason we are stopping here - rather than continuing to step (as the watchpoint would've had us do) - is because of the "stepi". */ - if (!bs->print) - return 0; if (bs->breakpoint_at->type == bp_breakpoint) { @@ -766,7 +756,7 @@ bpstat_print (bs) if (bs->next) return bpstat_print (bs->next); - fprintf_filtered (stderr, "gdb internal error: in bpstat_print\n"); + /* We reached the end of the chain without printing anything. */ return 0; } @@ -796,7 +786,6 @@ bpstat_alloc (b, cbs) bs->breakpoint_at = b; /* If the condition is false, etc., don't do the commands. */ bs->commands = NULL; - bs->momentary = b->disposition == delete; bs->old_val = NULL; bs->print_it = print_it_normal; return bs; @@ -872,12 +861,21 @@ which its expression is valid.\n", bs->breakpoint_at->number); /* This is used when everything which needs to be printed has already been printed. But we still want to print the frame. */ static int -print_it_noop (bs) +print_it_done (bs) bpstat bs; { return 0; } +/* This is used when nothing should be printed for this bpstat entry. */ + +static int +print_it_noop (bs) + bpstat bs; +{ + return -1; +} + /* Determine whether we stopped at a breakpoint, etc, or whether we don't understand this stop. Result is a chain of bpstat's such that: @@ -903,8 +901,6 @@ bpstat_stop_status (pc, frame_address) FRAME_ADDR frame_address; { register struct breakpoint *b; - int stop = 0; - int print = 0; CORE_ADDR bp_addr; #if DECR_PC_AFTER_BREAK != 0 || defined (SHIFT_INST_REGS) /* True if we've hit a breakpoint (as opposed to a watchpoint). */ @@ -920,9 +916,6 @@ bpstat_stop_status (pc, frame_address) ALL_BREAKPOINTS (b) { - int this_bp_stop; - int this_bp_print; - if (b->enable == disabled) continue; @@ -933,8 +926,8 @@ bpstat_stop_status (pc, frame_address) bs = bpstat_alloc (b, bs); /* Alloc a bpstat to explain stop */ - this_bp_stop = 1; - this_bp_print = 1; + bs->stop = 1; + bs->print = 1; if (b->type == bp_watchpoint) { @@ -946,7 +939,7 @@ bpstat_stop_status (pc, frame_address) { case WP_DISABLED: /* We've already printed what needs to be printed. */ - bs->print_it = print_it_noop; + bs->print_it = print_it_done; /* Stop. */ break; case WP_VALUE_CHANGED: @@ -954,6 +947,8 @@ bpstat_stop_status (pc, frame_address) break; case WP_VALUE_NOT_CHANGED: /* Don't stop. */ + bs->print_it = print_it_noop; + bs->stop = 0; continue; default: /* Can't happen. */ @@ -963,7 +958,7 @@ bpstat_stop_status (pc, frame_address) b->enable = disabled; printf_filtered ("Watchpoint %d disabled.\n", b->number); /* We've already printed what needs to be printed. */ - bs->print_it = print_it_noop; + bs->print_it = print_it_done; /* Stop. */ break; } @@ -974,7 +969,7 @@ bpstat_stop_status (pc, frame_address) #endif if (b->frame && b->frame != frame_address) - this_bp_stop = 0; + bs->stop = 0; else { int value_is_zero; @@ -992,12 +987,12 @@ bpstat_stop_status (pc, frame_address) } if (b->cond && value_is_zero) { - this_bp_stop = 0; + bs->stop = 0; } else if (b->ignore_count > 0) { b->ignore_count--; - this_bp_stop = 0; + bs->stop = 0; } else { @@ -1006,27 +1001,24 @@ bpstat_stop_status (pc, frame_address) b->enable = disabled; bs->commands = b->commands; if (b->silent) - this_bp_print = 0; + bs->print = 0; if (bs->commands && STREQ ("silent", bs->commands->line)) { bs->commands = bs->commands->next; - this_bp_print = 0; + bs->print = 0; } } } - if (this_bp_stop) - stop = 1; - if (this_bp_print) - print = 1; + /* Print nothing for this entry if we dont stop or if we dont print. */ + if (bs->stop == 0 || bs->print == 0) + bs->print_it = print_it_noop; } bs->next = NULL; /* Terminate the chain */ bs = root_bs->next; /* Re-grab the head of the chain */ +#if DECR_PC_AFTER_BREAK != 0 || defined (SHIFT_INST_REGS) if (bs) { - bs->stop = stop; - bs->print = print; -#if DECR_PC_AFTER_BREAK != 0 || defined (SHIFT_INST_REGS) if (real_breakpoint) { *pc = bp_addr; @@ -1044,10 +1036,131 @@ bpstat_stop_status (pc, frame_address) write_pc (bp_addr); #endif /* No SHIFT_INST_REGS. */ } -#endif /* DECR_PC_AFTER_BREAK != 0. */ } +#endif /* DECR_PC_AFTER_BREAK != 0. */ return bs; } + +/* Tell what to do about this bpstat. */ +enum bpstat_what +bpstat_what (bs) + bpstat bs; +{ + /* Classify each bpstat as one of the following. */ + enum class { + /* There was a watchpoint, but we're not stopping. */ + wp_nostop = 0, + + /* There was a watchpoint, stop but don't print. */ + wp_silent, + + /* There was a watchpoint, stop and print. */ + wp_noisy, + + /* There was a breakpoint but we're not stopping. */ + bp_nostop, + + /* There was a breakpoint, stop but don't print. */ + bp_silent, + + /* There was a breakpoint, stop and print. */ + bp_noisy, + + /* We hit the longjmp breakpoint. */ + long_jump, + + /* We hit the longjmp_resume breakpoint. */ + long_resume, + + /* This is just used to count how many enums there are. */ + class_last + }; + + /* Here is the table which drives this routine. So that we can + format it pretty, we define some abbreviations for the + enum bpstat_what codes. */ +#define keep_c BPSTAT_WHAT_KEEP_CHECKING +#define stop_s BPSTAT_WHAT_STOP_SILENT +#define stop_n BPSTAT_WHAT_STOP_NOISY +#define single BPSTAT_WHAT_SINGLE +#define setlr BPSTAT_WHAT_SET_LONGJMP_RESUME +#define clrlr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME +#define clrlrs BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE +/* "Can't happen." Might want to print an error message. + abort() is not out of the question, but chances are GDB is just + a bit confused, not unusable. */ +#define err BPSTAT_WHAT_STOP_NOISY + + /* Given an old action and a class, come up with a new action. */ + static const enum bpstat_what + table[(int)class_last][(int)BPSTAT_WHAT_LAST] = + { + /* old action */ + /* keep_c stop_s stop_n single setlr clrlr clrlrs */ + +/*wp_nostop*/ {keep_c, stop_s, stop_n, single, setlr , clrlr , clrlrs}, +/*wp_silent*/ {stop_s, stop_s, stop_n, stop_s, stop_s, stop_s, stop_s}, +/*wp_noisy*/ {stop_n, stop_n, stop_n, stop_n, stop_n, stop_n, stop_n}, +/*bp_nostop*/ {single, stop_s, stop_n, single, setlr , clrlrs, clrlrs}, +/*bp_silent*/ {stop_s, stop_s, stop_n, stop_s, stop_s, stop_s, stop_s}, +/*bp_noisy*/ {stop_n, stop_n, stop_n, stop_n, stop_n, stop_n, stop_n}, +/*long_jump*/ {setlr , stop_s, stop_n, setlr , err , err , err }, +/*long_resume*/ {clrlr , stop_s, stop_n, clrlrs, err , err , err } + }; +#undef keep_c +#undef stop_s +#undef stop_n +#undef single +#undef setlr +#undef clrlr +#undef clrlrs +#undef err + enum bpstat_what current_action = BPSTAT_WHAT_KEEP_CHECKING; + + for (; bs != NULL; bs = bs->next) + { + enum class bs_class; + if (bs->breakpoint_at == NULL) + /* I suspect this can happen if it was a momentary breakpoint + which has since been deleted. */ + continue; + switch (bs->breakpoint_at->type) + { + case bp_breakpoint: + case bp_until: + case bp_finish: + if (bs->stop) + { + if (bs->print) + bs_class = bp_noisy; + else + bs_class = bp_silent; + } + else + bs_class = bp_nostop; + break; + case bp_watchpoint: + if (bs->stop) + { + if (bs->print) + bs_class = wp_noisy; + else + bs_class = wp_silent; + } + else + bs_class = wp_nostop; + break; + case bp_longjmp: + bs_class = long_jump; + break; + case bp_longjmp_resume: + bs_class = long_resume; + break; + } + current_action = table[(int)bs_class][(int)current_action]; + } + return current_action; +} /* Nonzero if we should step constantly (e.g. watchpoints on machines without hardware support). This isn't related to a specific bpstat, @@ -2339,7 +2452,8 @@ breakpoint_re_set_one (bint) s = b->cond_string; b->cond = parse_exp_1 (&s, (struct block *)0, 0); } - mention (b); + if (b->enable == enabled) + mention (b); break; default: diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index d05e0e785b..3949777c69 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -50,21 +50,56 @@ extern bpstat bpstat_copy PARAMS ((bpstat)); /* FIXME: prototypes uses equivalence between FRAME_ADDR and CORE_ADDR */ extern bpstat bpstat_stop_status PARAMS ((CORE_ADDR *, CORE_ADDR)); -/* Nonzero if we should print the frame. */ -#define bpstat_should_print(bs) ((bs) != NULL && (bs)->print) +/* Return values from bpstat_what. */ +enum bpstat_what { + /* Perform various other tests; that is, this bpstat does not + say to perform any action (e.g. failed watchpoint and nothing + else). */ + BPSTAT_WHAT_KEEP_CHECKING, + + /* Rather than distinguish between noisy and silent stops here, it + might be cleaner to have bpstat_print make that decision (also + taking into account stop_print_frame and source_only). But the + implications are a bit scary (interaction with auto-displays, etc.), + so I won't try it. */ + + /* Stop silently. */ + BPSTAT_WHAT_STOP_SILENT, + + /* Stop and print. */ + BPSTAT_WHAT_STOP_NOISY, + + /* Remove breakpoints, single step once, then put them back in and + go back to what we were doing. */ + BPSTAT_WHAT_SINGLE, + + /* Set longjmp_resume breakpoint, remove all other breakpoints, + and continue. The "remove all other breakpoints" part is required + if we are also stepping over another breakpoint as well as doing + the longjmp handling. */ + BPSTAT_WHAT_SET_LONGJMP_RESUME, + + /* Clear longjmp_resume breakpoint, then handle as + BPSTAT_WHAT_KEEP_CHECKING. */ + BPSTAT_WHAT_CLEAR_LONGJMP_RESUME, + + /* Clear longjmp_resume breakpoint, then handle as BPSTAT_WHAT_SINGLE. */ + BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE, + + /* This is just used to keep track of how many enums there are. */ + BPSTAT_WHAT_LAST +}; -/* Nonzero if we should stop. */ -#define bpstat_stop(bs) ((bs) != NULL && (bs)->stop) +/* Tell what to do about this bpstat. */ +enum bpstat_what bpstat_what PARAMS ((bpstat)); /* Find the bpstat associated with a breakpoint. NULL otherwise. */ -bpstat bpstat_find_breakpoint(/* bpstat, breakpoint */); - -/* Nonzero if we hit a momentary breakpoint. */ -#define bpstat_momentary_breakpoint(bs) ((bs) != NULL && (bs)->momentary) +bpstat bpstat_find_breakpoint PARAMS ((bpstat, struct breakpoint *)); /* Nonzero if a signal that we got in wait() was due to circumstances explained by the BS. */ -/* Currently that is true iff we have hit a breakpoint. */ +/* Currently that is true if we have hit a breakpoint, or if there is + a watchpoint enabled. */ #define bpstat_explains_signal(bs) ((bs) != NULL) /* Nonzero if we should step constantly (e.g. watchpoints on machines @@ -102,15 +137,17 @@ struct bpstat struct command_line *commands; /* Old value associated with a watchpoint. */ value old_val; - /* Nonzero if we should print the frame. Only significant for the first - bpstat in the chain. */ + + /* Nonzero if this breakpoint tells us to print the frame. */ char print; - /* Nonzero if we should stop. Only significant for the first bpstat in - the chain. */ + + /* Nonzero if this breakpoint tells us to stop. */ char stop; - /* Nonzero if we hit a momentary breakpoint. Only significant for the - first bpstat in the chain. */ - char momentary; + + /* Function called by bpstat_print to print stuff associated with + this element of the bpstat chain. Returns 0 or 1 just like + bpstat_print, or -1 if it can't deal with it. */ + int (*print_it) PARAMS((bpstat bs)); }; /* Type of breakpoint. */ @@ -121,6 +158,11 @@ struct bpstat 2) step-resume (for 'next'ing over subroutine calls), 3) call-dummy (the breakpoint at the end of a subroutine stub that gdb uses to call functions in the target). + + I definately agree with (2) and (3); I'm not as sure about (1) + (it is a low-level thing, perhaps the best thing is that it looks + as much as possible like a single-step to wait_for_inferior) + -kingdon, 8 Apr 93. */ enum bptype { -- 2.34.1