This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 3/3] bpstat_what removal
Hi Pedro,
attaching follow-up on your patch still using BPSTAT_WHAT_* symbols.
* Removed whitespace and other excessive text changes against FSF GDB.
* Removed the new logic for pre-determining BPSTAT_WHAT_SINGLE.
* Keep bp_shlib_event+bp_jit_event (IMO important) debug_infrun output.
No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.
Thanks,
Jan
gdb/
2010-06-24 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <pedro@codesourcery.com>
* breakpoint.c (handle_jit_event): New function.
(bpstat_what): Remove enum class, kc, ss, sn, sgl, slr, clr, sr, shl,
jit, err, table and bs_class. New variables shlib_event, jit_event,
this_action and bptype. Change bs_class assignments to this_action
assignments. new unhandled bptype internal error. Move here
shlib_event and jit_event handling from handle_inferior_event.
* breakpoint.h (enum bpstat_what_main_action): Extend the comment.
Reorder items. Remove BPSTAT_WHAT_CHECK_SHLIBS and
BPSTAT_WHAT_CHECK_JIT.
* inferior.h (debug_infrun, stop_on_solib_events): New declarations.
* infrun.c (debug_infrun, stop_on_solib_events): Remove static.
(handle_inferior_event): Reinitialize frame and gdbarch after
bpstat_what call. Move BPSTAT_WHAT_CHECK_SHLIBS and
BPSTAT_WHAT_CHECK_JIT handling to bpstat_what. Reinitialize even
gdbarch when frame gets reinitialized.
gdb/testsuite/
2010-05-17 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/nostdlib.exp, gdb.base/nostdlib.c: New.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4194,151 +4194,64 @@ bpstat_stop_status (struct address_space *aspace,
return root_bs->next;
}
-
-/* Tell what to do about this bpstat. */
-struct bpstat_what
-bpstat_what (bpstat bs)
-{
- /* Classify each bpstat as one of the following. */
- enum class
- {
- /* This bpstat element has no effect on the main_action. */
- no_effect = 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,
+static void
+handle_jit_event (void)
+{
+ struct frame_info *frame;
+ struct gdbarch *gdbarch;
- /* We hit the longjmp_resume breakpoint. */
- long_resume,
+ /* Switch terminal for any messages produced by
+ breakpoint_re_set. */
+ target_terminal_ours_for_output ();
- /* We hit the step_resume breakpoint. */
- step_resume,
+ frame = get_current_frame ();
+ gdbarch = get_frame_arch (frame);
- /* We hit the shared library event breakpoint. */
- shlib_event,
+ jit_event_handler (gdbarch);
- /* We hit the jit event breakpoint. */
- jit_event,
+ target_terminal_inferior ();
+}
- /* This is just used to count how many enums there are. */
- class_last
- };
+/* Prepare WHAT final decision for infrun. */
- /* 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 kc BPSTAT_WHAT_KEEP_CHECKING
-#define ss BPSTAT_WHAT_STOP_SILENT
-#define sn BPSTAT_WHAT_STOP_NOISY
-#define sgl BPSTAT_WHAT_SINGLE
-#define slr BPSTAT_WHAT_SET_LONGJMP_RESUME
-#define clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
-#define sr BPSTAT_WHAT_STEP_RESUME
-#define shl BPSTAT_WHAT_CHECK_SHLIBS
-#define jit BPSTAT_WHAT_CHECK_JIT
-
-/* "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. */
- /* One interesting property of this table is that wp_silent is the same
- as bp_silent and wp_noisy is the same as bp_noisy. That is because
- after stopping, the check for whether to step over a breakpoint
- (BPSTAT_WHAT_SINGLE type stuff) is handled in proceed() without
- reference to how we stopped. We retain separate wp_silent and
- bp_silent codes in case we want to change that someday.
-
- Another possibly interesting property of this table is that
- there's a partial ordering, priority-like, of the actions. Once
- you've decided that some action is appropriate, you'll never go
- back and decide something of a lower priority is better. The
- ordering is:
-
- kc < jit clr sgl shl slr sn sr ss
- sgl < jit shl slr sn sr ss
- slr < jit err shl sn sr ss
- clr < jit err shl sn sr ss
- ss < jit shl sn sr
- sn < jit shl sr
- jit < shl sr
- shl < sr
- sr <
-
- What I think this means is that we don't need a damned table
- here. If you just put the rows and columns in the right order,
- it'd look awfully regular. We could simply walk the bpstat list
- and choose the highest priority action we find, with a little
- logic to handle the 'err' cases. */
-
- /* step_resume entries: a step resume breakpoint overrides another
- breakpoint of signal handling (see comment in wait_for_inferior
- at where we set the step_resume breakpoint). */
-
- static const enum bpstat_what_main_action
- table[(int) class_last][(int) BPSTAT_WHAT_LAST] =
- {
- /* old action */
- /* kc ss sn sgl slr clr sr shl jit */
-/* no_effect */ {kc, ss, sn, sgl, slr, clr, sr, shl, jit},
-/* wp_silent */ {ss, ss, sn, ss, ss, ss, sr, shl, jit},
-/* wp_noisy */ {sn, sn, sn, sn, sn, sn, sr, shl, jit},
-/* bp_nostop */ {sgl, ss, sn, sgl, slr, slr, sr, shl, jit},
-/* bp_silent */ {ss, ss, sn, ss, ss, ss, sr, shl, jit},
-/* bp_noisy */ {sn, sn, sn, sn, sn, sn, sr, shl, jit},
-/* long_jump */ {slr, ss, sn, slr, slr, err, sr, shl, jit},
-/* long_resume */ {clr, ss, sn, err, err, err, sr, shl, jit},
-/* step_resume */ {sr, sr, sr, sr, sr, sr, sr, sr, sr },
-/* shlib */ {shl, shl, shl, shl, shl, shl, sr, shl, shl},
-/* jit_event */ {jit, jit, jit, jit, jit, jit, sr, jit, jit}
- };
+/* Decide what infrun needs to do with this bpstat. */
-#undef kc
-#undef ss
-#undef sn
-#undef sgl
-#undef slr
-#undef clr
-#undef err
-#undef sr
-#undef ts
-#undef shl
-#undef jit
- enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
+struct bpstat_what
+bpstat_what (bpstat bs)
+{
struct bpstat_what retval;
+ /* We need to defer calling `solib_add', as adding new symbols
+ resets breakpoints, which in turn deletes breakpoint locations,
+ and hence may clear unprocessed entries in the BS chain. */
+ int shlib_event = 0;
+ int jit_event = 0;
+ retval.main_action = BPSTAT_WHAT_KEEP_CHECKING;
retval.call_dummy = STOP_NONE;
+
for (; bs != NULL; bs = bs->next)
{
- enum class bs_class = no_effect;
+ /* Extract this BS's action. After processing each BS, we check
+ if its action overrides all we've seem so far. */
+ enum bpstat_what_main_action this_action = BPSTAT_WHAT_KEEP_CHECKING;
+ enum bptype bptype;
+
if (bs->breakpoint_at == NULL)
- /* I suspect this can happen if it was a momentary breakpoint
- which has since been deleted. */
- continue;
- if (bs->breakpoint_at->owner == NULL)
- bs_class = bp_nostop;
+ {
+ /* I suspect this can happen if it was a momentary
+ breakpoint which has since been deleted. */
+ bptype = bp_none;
+ }
+ else if (bs->breakpoint_at->owner == NULL)
+ bptype = bp_none;
else
- switch (bs->breakpoint_at->owner->type)
+ bptype = bs->breakpoint_at->owner->type;
+
+ switch (bptype)
{
case bp_none:
- continue;
-
+ break;
case bp_breakpoint:
case bp_hardware_breakpoint:
case bp_until:
@@ -4346,12 +4259,12 @@ bpstat_what (bpstat bs)
if (bs->stop)
{
if (bs->print)
- bs_class = bp_noisy;
+ this_action = BPSTAT_WHAT_STOP_NOISY;
else
- bs_class = bp_silent;
+ this_action = BPSTAT_WHAT_STOP_SILENT;
}
else
- bs_class = bp_nostop;
+ this_action = BPSTAT_WHAT_SINGLE;
break;
case bp_watchpoint:
case bp_hardware_watchpoint:
@@ -4360,69 +4273,79 @@ bpstat_what (bpstat bs)
if (bs->stop)
{
if (bs->print)
- bs_class = wp_noisy;
+ this_action = BPSTAT_WHAT_STOP_NOISY;
else
- bs_class = wp_silent;
+ this_action = BPSTAT_WHAT_STOP_SILENT;
}
else
- /* There was a watchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ {
+ /* There was a watchpoint, but we're not stopping.
+ This requires no further action. */
+ }
break;
case bp_longjmp:
- bs_class = long_jump;
+ this_action = BPSTAT_WHAT_SET_LONGJMP_RESUME;
break;
case bp_longjmp_resume:
- bs_class = long_resume;
+ this_action = BPSTAT_WHAT_CLEAR_LONGJMP_RESUME;
break;
case bp_step_resume:
if (bs->stop)
+ this_action = BPSTAT_WHAT_STEP_RESUME;
+ else
{
- bs_class = step_resume;
+ /* It is for the wrong frame. */
+ this_action = BPSTAT_WHAT_SINGLE;
}
- else
- /* It is for the wrong frame. */
- bs_class = bp_nostop;
break;
case bp_watchpoint_scope:
- bs_class = bp_nostop;
- break;
- case bp_shlib_event:
- bs_class = shlib_event;
- break;
- case bp_jit_event:
- bs_class = jit_event;
- break;
case bp_thread_event:
case bp_overlay_event:
case bp_longjmp_master:
case bp_std_terminate_master:
- bs_class = bp_nostop;
+ this_action = BPSTAT_WHAT_SINGLE;
break;
case bp_catchpoint:
if (bs->stop)
{
if (bs->print)
- bs_class = bp_noisy;
+ this_action = BPSTAT_WHAT_STOP_NOISY;
else
- bs_class = bp_silent;
+ this_action = BPSTAT_WHAT_STOP_SILENT;
+ }
+ else
+ {
+ /* There was a catchpoint, but we're not stopping.
+ This requires no further action. */
}
+ break;
+ case bp_shlib_event:
+ shlib_event = 1;
+
+ /* If requested, stop when the dynamic linker notifies GDB
+ of events. This allows the user to get control and place
+ breakpoints in initializer routines for dynamically
+ loaded objects (among other things). */
+ if (stop_on_solib_events)
+ this_action = BPSTAT_WHAT_STOP_NOISY;
else
- /* There was a catchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ this_action = BPSTAT_WHAT_SINGLE;
+ break;
+ case bp_jit_event:
+ jit_event = 1;
+ this_action = BPSTAT_WHAT_SINGLE;
break;
case bp_call_dummy:
/* Make sure the action is stop (silent or noisy),
so infrun.c pops the dummy frame. */
- bs_class = bp_silent;
retval.call_dummy = STOP_STACK_DUMMY;
+ this_action = BPSTAT_WHAT_STOP_SILENT;
break;
case bp_std_terminate:
/* Make sure the action is stop (silent or noisy),
so infrun.c pops the dummy frame. */
- bs_class = bp_silent;
retval.call_dummy = STOP_STD_TERMINATE;
+ this_action = BPSTAT_WHAT_STOP_SILENT;
break;
case bp_tracepoint:
case bp_fast_tracepoint:
@@ -4431,11 +4354,43 @@ bpstat_what (bpstat bs)
out already. */
internal_error (__FILE__, __LINE__,
_("bpstat_what: tracepoint encountered"));
- break;
+ default:
+ internal_error (__FILE__, __LINE__,
+ _("bpstat_what: unhandled bptype %d"), (int) bptype);
}
- current_action = table[(int) bs_class][(int) current_action];
+
+ retval.main_action = max (retval.main_action, this_action);
+ }
+
+ if (shlib_event)
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "bpstat_what: bp_shlib_event\n");
+
+ /* Check for any newly added shared libraries if we're supposed
+ to be adding them automatically. */
+
+ /* Switch terminal for any messages produced by
+ breakpoint_re_set. */
+ target_terminal_ours_for_output ();
+
+#ifdef SOLIB_ADD
+ SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
+#else
+ solib_add (NULL, 0, ¤t_target, auto_solib_add);
+#endif
+
+ target_terminal_inferior ();
+ }
+
+ if (jit_event)
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "bpstat_what: bp_jit_event\n");
+
+ handle_jit_event ();
}
- retval.main_action = current_action;
+
return retval;
}
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -563,7 +563,20 @@ extern bpstat bpstat_stop_status (struct address_space *aspace,
CORE_ADDR pc, ptid_t ptid);
/* This bpstat_what stuff tells wait_for_inferior what to do with a
- breakpoint (a challenging task). */
+ breakpoint (a challenging task).
+
+ The enum values order defines priority-like order of the actions.
+ Once you've decided that some action is appropriate, you'll never
+ go back and decide something of a lower priority is better. Each
+ of these actions is mutually exclusive with the others. That
+ means, that if you find yourself adding a new action class here and
+ wanting to tell GDB that you have two simultaneous actions to
+ handle, something is wrong, and you probably don't actually need a
+ new action type.
+
+ Note that a step resume breakpoint overrides another breakpoint of
+ signal handling (see comment in wait_for_inferior at where we set
+ the step_resume breakpoint). */
enum bpstat_what_main_action
{
@@ -572,18 +585,6 @@ enum bpstat_what_main_action
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. It's possible that this should be
removed from the main_action and put into a separate field, to more
@@ -600,18 +601,20 @@ enum bpstat_what_main_action
BPSTAT_WHAT_KEEP_CHECKING. */
BPSTAT_WHAT_CLEAR_LONGJMP_RESUME,
- /* Clear step resume breakpoint, and keep checking. */
- BPSTAT_WHAT_STEP_RESUME,
+ /* 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. */
- /* Check the dynamic linker's data structures for new libraries, then
- keep checking. */
- BPSTAT_WHAT_CHECK_SHLIBS,
+ /* Stop silently. */
+ BPSTAT_WHAT_STOP_SILENT,
- /* Check for new JITed code. */
- BPSTAT_WHAT_CHECK_JIT,
+ /* Stop and print. */
+ BPSTAT_WHAT_STOP_NOISY,
- /* This is just used to keep track of how many enums there are. */
- BPSTAT_WHAT_LAST
+ /* Clear step resume breakpoint, and keep checking. */
+ BPSTAT_WHAT_STEP_RESUME,
};
/* An enum indicating the kind of "stack dummy" stop. This is a bit
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -232,6 +232,10 @@ extern char *construct_inferior_arguments (int, char **);
/* From infrun.c */
+extern int debug_infrun;
+
+extern int stop_on_solib_events;
+
extern void start_remote (int from_tty);
extern void normal_stop (void);
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -119,7 +119,7 @@ show_debug_displaced (struct ui_file *file, int from_tty,
fprintf_filtered (file, _("Displace stepping debugging is %s.\n"), value);
}
-static int debug_infrun = 0;
+int debug_infrun = 0;
static void
show_debug_infrun (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -294,7 +294,7 @@ static struct symbol *step_start_function;
/* Nonzero if we want to give control to the user when we're notified
of shared library events by the dynamic linker. */
-static int stop_on_solib_events;
+int stop_on_solib_events;
static void
show_stop_on_solib_events (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -4057,6 +4057,12 @@ process_event_stop_test:
stop_stack_dummy = what.call_dummy;
}
+ /* If we hit an internal event that triggers symbol changes, the
+ current frame will be invalidated within bpstat_what (e.g., if
+ we hit an internal solib event). Re-fetch it. */
+ frame = get_current_frame ();
+ gdbarch = get_frame_arch (frame);
+
switch (what.main_action)
{
case BPSTAT_WHAT_SET_LONGJMP_RESUME:
@@ -4161,66 +4167,6 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
}
break;
- case BPSTAT_WHAT_CHECK_SHLIBS:
- {
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CHECK_SHLIBS\n");
-
- /* Check for any newly added shared libraries if we're
- supposed to be adding them automatically. Switch
- terminal for any messages produced by
- breakpoint_re_set. */
- target_terminal_ours_for_output ();
- /* NOTE: cagney/2003-11-25: Make certain that the target
- stack's section table is kept up-to-date. Architectures,
- (e.g., PPC64), use the section table to perform
- operations such as address => section name and hence
- require the table to contain all sections (including
- those found in shared libraries). */
-#ifdef SOLIB_ADD
- SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
-#else
- solib_add (NULL, 0, ¤t_target, auto_solib_add);
-#endif
- target_terminal_inferior ();
-
- /* If requested, stop when the dynamic linker notifies
- gdb of events. This allows the user to get control
- and place breakpoints in initializer routines for
- dynamically loaded objects (among other things). */
- if (stop_on_solib_events || stop_stack_dummy)
- {
- stop_stepping (ecs);
- return;
- }
- else
- {
- /* We want to step over this breakpoint, then keep going. */
- ecs->event_thread->stepping_over_breakpoint = 1;
- break;
- }
- }
- break;
-
- case BPSTAT_WHAT_CHECK_JIT:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CHECK_JIT\n");
-
- /* Switch terminal for any messages produced by breakpoint_re_set. */
- target_terminal_ours_for_output ();
-
- jit_event_handler (gdbarch);
-
- target_terminal_inferior ();
-
- /* We want to step over this breakpoint, then keep going. */
- ecs->event_thread->stepping_over_breakpoint = 1;
-
- break;
-
- case BPSTAT_WHAT_LAST:
- /* Not a real code, but listed here to shut up gcc -Wall. */
-
case BPSTAT_WHAT_KEEP_CHECKING:
break;
}
@@ -4357,6 +4303,7 @@ infrun: not switching back to stepped thread, it has vanished\n");
the frame cache to be re-initialized, making our FRAME variable
a dangling pointer. */
frame = get_current_frame ();
+ gdbarch = get_frame_arch (frame);
/* If stepping through a line, keep going if still within it.
--- /dev/null
+++ b/gdb/testsuite/gdb.base/nostdlib.c
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+void
+_start (void)
+{
+ extern void marker (void);
+
+ marker ();
+}
+
+void
+marker (void)
+{
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/nostdlib.exp
@@ -0,0 +1,54 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+set testfile "nostdlib"
+set srcfile ${testfile}.c
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+# default_target_compile would otherwise add "-lm" making the testcase
+# dependent on whether the system libraries are already prelinked.
+# prelink: Could not set /lib64/libm-2.11.1.so owner or mode: Operation not permitted
+set compile {
+ gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-nostdlib}
+}
+set board [target_info name]
+if [board_info $board exists mathlib] {
+ set mathlib [board_info $dest mathlib]
+ set_board_info mathlib ""
+ set err [eval $compile]
+ set_board_info mathlib $mathlib
+} else {
+ set_board_info mathlib ""
+ set err [eval $compile]
+ unset_board_info mathlib
+}
+if {$err != ""} {
+ untested ${testfile}.exp
+ return -1
+}
+
+clean_restart $executable
+
+gdb_breakpoint "*marker"
+gdb_breakpoint "*_start"
+
+gdb_run_cmd
+
+# Breakpoint 2, Stopped due to shared library event
+# _start () at ./gdb.base/nostdlib.c:20
+gdb_test "" {Breakpoint [0-9]+, .*_start .*} "stop at run"
+
+gdb_test "continue" {Breakpoint [0-9]+, marker .*} "continue to marker"