* [patch 3/3] bpstat_what removal
@ 2010-05-03 20:02 Jan Kratochvil
2010-05-04 14:10 ` Stan Shebs
0 siblings, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-05-03 20:02 UTC (permalink / raw)
To: gdb-patches
Hi,
the simple idea is to inline bpstat_what into handle_inferior_event. This
removes enum bpstat_what_main_action and struct bpstat_what currently acting
just as an interface between these two functions.
solib_add() needs to stay delayed as it has too disturbing effect on the
content of stop_bpstat.
breakpoint_type_name is there just for DEBUG_INFRUN.
Thanks,
Jan
gdb/
2010-05-03 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (bpstat_what): Remove.
(breakpoint_type_name): New.
* breakpoint.h (enum bpstat_what_main_action, struct bpstat_what)
(bpstat_what): Remove.
(breakpoint_type_name): New prototype.
* infrun.c (handle_inferior_event): Clear frame and gdbarch before
deciding what action to take. New variables bs, print_frame_max,
stop_step_max, perform_max and perform_shlib. Replace the bpstat_what
call by new inlined deciding code. Reinitialize even gdbarch when
frame gets reinitialized.
gdb/testsuite/
2010-05-03 Jan Kratochvil <jan.kratochvil@redhat.com>
* break-solib-event.exp: New.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4161,250 +4161,6 @@ bpstat_stop_status (struct address_space *aspace,
return root_bs->next;
}
\f
-/* 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,
-
- /* We hit the longjmp_resume breakpoint. */
- long_resume,
-
- /* We hit the step_resume breakpoint. */
- step_resume,
-
- /* We hit the shared library event breakpoint. */
- shlib_event,
-
- /* We hit the jit event breakpoint. */
- jit_event,
-
- /* 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 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}
- };
-
-#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 retval;
-
- retval.call_dummy = STOP_NONE;
- for (; bs != NULL; bs = bs->next)
- {
- enum class bs_class = no_effect;
- 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;
- else
- switch (bs->breakpoint_at->owner->type)
- {
- case bp_none:
- continue;
-
- case bp_breakpoint:
- case bp_hardware_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:
- case bp_hardware_watchpoint:
- case bp_read_watchpoint:
- case bp_access_watchpoint:
- if (bs->stop)
- {
- if (bs->print)
- bs_class = wp_noisy;
- else
- bs_class = wp_silent;
- }
- else
- /* There was a watchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
- break;
- case bp_longjmp:
- bs_class = long_jump;
- break;
- case bp_longjmp_resume:
- bs_class = long_resume;
- break;
- case bp_step_resume:
- if (bs->stop)
- {
- bs_class = step_resume;
- }
- 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;
- break;
- case bp_catchpoint:
- if (bs->stop)
- {
- if (bs->print)
- bs_class = bp_noisy;
- else
- bs_class = bp_silent;
- }
- else
- /* There was a catchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
- 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;
- 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;
- break;
- case bp_tracepoint:
- case bp_fast_tracepoint:
- /* Tracepoint hits should not be reported back to GDB, and
- if one got through somehow, it should have been filtered
- out already. */
- internal_error (__FILE__, __LINE__,
- _("bpstat_what: tracepoint encountered"));
- break;
- }
- current_action = table[(int) bs_class][(int) current_action];
- }
- retval.main_action = current_action;
- return retval;
-}
-
/* Nonzero if we should step constantly (e.g. watchpoints on machines
without hardware support). This isn't related to a specific bpstat,
just to things like whether watchpoints are set. */
@@ -10999,6 +10755,65 @@ save_command (char *arg, int from_tty)
help_list (save_cmdlist, "save ", -1, gdb_stdout);
}
+const char *
+breakpoint_type_name (enum bptype bptype)
+{
+ switch (bptype)
+ {
+ case bp_none:
+ return "bp_none";
+ case bp_breakpoint:
+ return "bp_breakpoint";
+ case bp_hardware_breakpoint:
+ return "bp_hardware_breakpoint";
+ case bp_until:
+ return "bp_until";
+ case bp_finish:
+ return "bp_finish";
+ case bp_watchpoint:
+ return "bp_watchpoint";
+ case bp_hardware_watchpoint:
+ return "bp_hardware_watchpoint";
+ case bp_read_watchpoint:
+ return "bp_read_watchpoint";
+ case bp_access_watchpoint:
+ return "bp_access_watchpoint";
+ case bp_longjmp:
+ return "bp_longjmp";
+ case bp_longjmp_resume:
+ return "bp_longjmp_resume";
+ case bp_step_resume:
+ return "bp_step_resume";
+ case bp_watchpoint_scope:
+ return "bp_watchpoint_scope";
+ case bp_call_dummy:
+ return "bp_call_dummy";
+ case bp_std_terminate:
+ return "bp_std_terminate";
+ case bp_shlib_event:
+ return "bp_shlib_event";
+ case bp_thread_event:
+ return "bp_thread_event";
+ case bp_overlay_event:
+ return "bp_overlay_event";
+ case bp_longjmp_master:
+ return "bp_longjmp_master";
+ case bp_std_terminate_master:
+ return "bp_std_terminate_master";
+ case bp_catchpoint:
+ return "bp_catchpoint";
+ case bp_tracepoint:
+ return "bp_tracepoint";
+ case bp_fast_tracepoint:
+ return "bp_fast_tracepoint";
+ case bp_jit_event:
+ return "bp_jit_event";
+ }
+ internal_error (__FILE__, __LINE__, _("Invalid breakpoint type %d"),
+ (int) bptype);
+ return NULL;
+}
+
void
_initialize_breakpoint (void)
{
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -561,58 +561,6 @@ extern bpstat bpstat_copy (bpstat);
extern bpstat bpstat_stop_status (struct address_space *aspace,
CORE_ADDR pc, ptid_t ptid);
\f
-/* This bpstat_what stuff tells wait_for_inferior what to do with a
- breakpoint (a challenging task). */
-
-enum bpstat_what_main_action
- {
- /* 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. It's possible that this should be
- removed from the main_action and put into a separate field, to more
- cleanly handle BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE. */
- 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 step resume breakpoint, and keep checking. */
- BPSTAT_WHAT_STEP_RESUME,
-
- /* Check the dynamic linker's data structures for new libraries, then
- keep checking. */
- BPSTAT_WHAT_CHECK_SHLIBS,
-
- /* Check for new JITed code. */
- BPSTAT_WHAT_CHECK_JIT,
-
- /* This is just used to keep track of how many enums there are. */
- BPSTAT_WHAT_LAST
- };
-
/* An enum indicating the kind of "stack dummy" stop. This is a bit
of a misnomer because only one kind of truly a stack dummy. */
enum stop_stack_kind
@@ -627,17 +575,6 @@ enum stop_stack_kind
STOP_STD_TERMINATE
};
-struct bpstat_what
- {
- enum bpstat_what_main_action main_action;
-
- /* Did we hit a call dummy breakpoint? This only goes with a main_action
- of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
- continuing from a call dummy without popping the frame is not a
- useful one). */
- enum stop_stack_kind call_dummy;
- };
-
/* The possible return values for print_bpstat, print_it_normal, print_it_done,
print_it_noop. bpstat_print depends on ther ordering where each item is an
information subset of the previous one. */
@@ -650,9 +587,6 @@ enum print_stop_action
PRINT_UNKNOWN
};
-/* Tell what to do about this bpstat. */
-struct bpstat_what bpstat_what (bpstat);
-\f
/* Find the bpstat associated with a breakpoint. NULL otherwise. */
bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *);
@@ -1050,4 +984,6 @@ extern void check_tracepoint_command (char *line, void *closure);
extern void start_rbreak_breakpoints (void);
extern void end_rbreak_breakpoints (void);
+extern const char *breakpoint_type_name (enum bptype bptype);
+
#endif /* !defined (BREAKPOINT_H) */
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3947,185 +3947,357 @@ process_event_stop_test:
return;
}
+ /* Breakpoints may get deleted and created in the block below. It calls
+ reinit_frame_cache thus invalidating current_frame. In this block one
+ needs to explicitly get_current_frame. */
+ frame = NULL;
+ gdbarch = NULL;
+
/* Handle cases caused by hitting a breakpoint. */
{
- CORE_ADDR jmp_buf_pc;
- struct bpstat_what what;
-
- what = bpstat_what (ecs->event_thread->stop_bpstat);
-
- if (what.call_dummy)
+ bpstat bs;
+ enum print_frame
{
- stop_stack_dummy = what.call_dummy;
+ /* pf_default is pf_yes. */
+ pf_default,
+ /* stop_print_frame value 0. */
+ pf_no,
+ /* stop_print_frame value 1. */
+ pf_yes,
}
-
- switch (what.main_action)
+ print_frame_max = pf_default;
+ enum stop_step
{
- case BPSTAT_WHAT_SET_LONGJMP_RESUME:
- /* If we hit the breakpoint at longjmp while stepping, we
- install a momentary breakpoint at the target of the
- jmp_buf. */
-
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
-
- ecs->event_thread->stepping_over_breakpoint = 1;
-
- if (!gdbarch_get_longjmp_target_p (gdbarch)
- || !gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
+ /* ss_default is ss_print_yes. */
+ ss_default,
+ /* ecs->event_thread->stop_step value 1. */
+ ss_print_no,
+ /* ecs->event_thread->stop_step value 0. */
+ ss_print_yes,
+ }
+ stop_step_max = ss_default;
+ enum perform
+ {
+ pe_undef,
+ /* Break from this block and check other possibilities why to stop. */
+ pe_check_more,
+ /* Call stop_stepping (ecs). */
+ pe_stop,
+ /* Like pe_stop but also print_stop_reason (END_STEPPING_RANGE, 0). */
+ pe_stop_end_range,
+ /* Call keep_going (ecs) and return without breaking from this block
+ and checking other possibilities why to stop. Some operations need
+ to finish before an already stepped on breakpoint is displayed to
+ the user. */
+ pe_going,
+ }
+ perform_max = pe_check_more;
+ /* solib_add may call breakpoint_re_set which would clear many
+ BREAKPOINT_AT entries still going to be processed. breakpoint_re_set
+ does not keep the same bp_location's even if they actually do not
+ change. */
+ int perform_shlib = 0;
+
+ for (bs = ecs->event_thread->stop_bpstat; bs != NULL; bs = bs->next)
+ {
+ /* Decisions for this specific BS, they get mapped to their *_max
+ variants at the end of this BS processing. */
+ enum print_frame print_frame = pf_default;
+ enum stop_step stop_step = ss_default;
+ enum perform perform = pe_undef;
+ enum bptype bptype;
+
+ if (bs->breakpoint_at == NULL)
{
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "\
-infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
- keep_going (ecs);
- return;
+ /* 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)
+ {
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ bptype = bp_none;
+ }
+ else
+ bptype = bs->breakpoint_at->owner->type;
- /* We're going to replace the current step-resume breakpoint
- with a longjmp-resume breakpoint. */
- delete_step_resume_breakpoint (ecs->event_thread);
-
- /* Insert a breakpoint at resume address. */
- insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
-
- keep_going (ecs);
- return;
-
- case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
-
- gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
- delete_step_resume_breakpoint (ecs->event_thread);
-
- ecs->event_thread->stop_step = 1;
- print_stop_reason (END_STEPPING_RANGE, 0);
- stop_stepping (ecs);
- return;
+ fprintf_unfiltered (gdb_stdlog, "infrun: %s\n",
+ breakpoint_type_name (bptype));
- case BPSTAT_WHAT_SINGLE:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
- ecs->event_thread->stepping_over_breakpoint = 1;
- /* Still need to check other stuff, at least the case
- where we are stepping and step out of the right range. */
- break;
+ switch (bptype)
+ {
+ case bp_none:
+ perform = pe_check_more;
+ break;
+ case bp_breakpoint:
+ case bp_hardware_breakpoint:
+ case bp_until:
+ case bp_finish:
+ if (bs->stop)
+ {
+ print_frame = bs->print ? pf_yes : pf_no;
+ perform = pe_stop;
+ }
+ else
+ {
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_check_more;
+ }
+ break;
+ case bp_watchpoint:
+ case bp_hardware_watchpoint:
+ case bp_read_watchpoint:
+ case bp_access_watchpoint:
+ case bp_catchpoint:
+ if (bs->stop)
+ {
+ print_frame = bs->print ? pf_yes : pf_no;
+ perform = pe_stop;
+ }
+ else
+ {
+ /* There was a watchpoint or catchpoint, but we're not
+ stopping. This requires no further action. */
+ perform = pe_check_more;
+ }
+ break;
+ case bp_longjmp:
+ {
+ struct frame_info *frame = get_current_frame ();
+ struct gdbarch *gdbarch = get_frame_arch (frame);
- case BPSTAT_WHAT_STOP_NOISY:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
- stop_print_frame = 1;
+ /* If we hit the breakpoint at longjmp while stepping, we
+ install a momentary breakpoint at the target of the
+ jmp_buf. */
- /* We are about to nuke the step_resume_breakpointt via the
- cleanup chain, so no need to worry about it here. */
+ CORE_ADDR jmp_buf_pc;
+ if (gdbarch_get_longjmp_target_p (gdbarch)
+ && gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
+ {
+ /* We're going to replace the current step-resume breakpoint
+ with a longjmp-resume breakpoint. */
+ delete_step_resume_breakpoint (ecs->event_thread);
- stop_stepping (ecs);
- return;
+ /* Insert a breakpoint at resume address. */
+ insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
+ }
- case BPSTAT_WHAT_STOP_SILENT:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
- stop_print_frame = 0;
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_going;
+ }
+ break;
+ case bp_longjmp_resume:
+ gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
+ delete_step_resume_breakpoint (ecs->event_thread);
+ stop_step = ss_print_no;
+ perform = pe_stop_end_range;
+ break;
+ case bp_step_resume:
+ if (bs->stop)
+ {
+ delete_step_resume_breakpoint (ecs->event_thread);
+ if (ecs->event_thread->step_after_step_resume_breakpoint)
+ {
+ /* Back when the step-resume breakpoint was inserted, we
+ were trying to single-step off a breakpoint. Go back
+ to doing that. pe_going must override pe_check_more so
+ that we do not stop again on that breakpoint. */
+ ecs->event_thread->step_after_step_resume_breakpoint = 0;
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_going;
+ }
+ else if (stop_pc == ecs->stop_func_start
+ && execution_direction == EXEC_REVERSE)
+ {
+ /* We are stepping over a function call in reverse, and
+ just hit the step-resume breakpoint at the start
+ address of the function. Go back to single-stepping,
+ which should take us back to the function call. */
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_going;
+ }
+ else
+ perform = pe_check_more;
+ }
+ else
+ {
+ /* It is for the wrong frame. */
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_check_more;
+ }
+ break;
+ case bp_watchpoint_scope:
+ case bp_thread_event:
+ case bp_overlay_event:
+ case bp_longjmp_master:
+ case bp_std_terminate_master:
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_check_more;
+ break;
+ case bp_shlib_event:
+ perform_shlib = 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 || stop_stack_dummy)
+ perform = pe_stop;
+ else
+ {
+ /* We want to step over this breakpoint, then keep going. */
+ ecs->event_thread->stepping_over_breakpoint = 1;
+ perform = pe_check_more;
+ }
+ break;
+ case bp_jit_event:
+ /* Switch terminal for any messages produced by breakpoint_re_set. */
+ target_terminal_ours_for_output ();
- /* We are about to nuke the step_resume_breakpoin via the
- cleanup chain, so no need to worry about it here. */
+ {
+ struct frame_info *frame = get_current_frame ();
+ struct gdbarch *gdbarch = get_frame_arch (frame);
- stop_stepping (ecs);
- return;
+ jit_event_handler (gdbarch);
+ }
- case BPSTAT_WHAT_STEP_RESUME:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
+ target_terminal_inferior ();
- delete_step_resume_breakpoint (ecs->event_thread);
- if (ecs->event_thread->step_after_step_resume_breakpoint)
- {
- /* Back when the step-resume breakpoint was inserted, we
- were trying to single-step off a breakpoint. Go back
- to doing that. */
- ecs->event_thread->step_after_step_resume_breakpoint = 0;
+ /* We want to step over this breakpoint, then keep going. */
ecs->event_thread->stepping_over_breakpoint = 1;
- keep_going (ecs);
- return;
- }
- if (stop_pc == ecs->stop_func_start
- && execution_direction == EXEC_REVERSE)
- {
- /* We are stepping over a function call in reverse, and
- just hit the step-resume breakpoint at the start
- address of the function. Go back to single-stepping,
- which should take us back to the function call. */
- ecs->event_thread->stepping_over_breakpoint = 1;
- keep_going (ecs);
- return;
+ perform = pe_check_more;
+ break;
+ case bp_call_dummy:
+ /* Make sure the action is stop (silent or noisy),
+ so infrun.c pops the dummy frame. */
+ stop_stack_dummy = STOP_STACK_DUMMY;
+ print_frame = pf_no;
+ perform = pe_stop;
+ break;
+ case bp_std_terminate:
+ /* Make sure the action is stop (silent or noisy),
+ so infrun.c pops the dummy frame. */
+ stop_stack_dummy = STOP_STD_TERMINATE;
+ print_frame = pf_no;
+ perform = pe_stop;
+ break;
+ case bp_tracepoint:
+ case bp_fast_tracepoint:
+ /* Tracepoint hits should not be reported back to GDB, and
+ if one got through somehow, it should have been filtered
+ out already. */
+ internal_error (__FILE__, __LINE__,
+ _("handle_inferior_event: tracepoint encountered"));
+ break;
+ default:
+ internal_error (__FILE__, __LINE__,
+ _("handle_inferior_event: Unhandled bptype %s"),
+ breakpoint_type_name (bptype));
+ break;
}
- break;
- case BPSTAT_WHAT_CHECK_SHLIBS:
- {
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CHECK_SHLIBS\n");
+ /* PERFORM must be always decided. */
+ if (perform == pe_undef)
+ internal_error (__FILE__, __LINE__,
+ _("handle_inferior_event: Unset perform, bptype %s"),
+ breakpoint_type_name (bptype));
- /* 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). */
+ if (debug_infrun)
+ {
+ const char *bptype_s = breakpoint_type_name (bptype);
+
+ if (print_frame != pf_default)
+ fprintf_unfiltered (gdb_stdlog, "infrun: %s: print_frame %s\n",
+ bptype_s, print_frame == pf_no ? "pf_no"
+ : "pf_yes");
+ if (stop_step != ss_default)
+ fprintf_unfiltered (gdb_stdlog, "infrun: %s: stop_step %s\n",
+ bptype_s, stop_step_max == ss_print_no
+ ? "ss_print_no (stop_step 1)"
+ : "ss_print_yes (stop_step 0)");
+ fprintf_unfiltered (gdb_stdlog, "infrun: %s: perform %s\n",
+ bptype_s,
+ perform == pe_going
+ ? "pe_going"
+ : perform == pe_check_more
+ ? "pe_check_more"
+ : perform == pe_stop
+ ? "pe_stop" : "pe_stop_end_range");
+ }
+
+ if (print_frame_max < print_frame)
+ print_frame_max = print_frame;
+ if (stop_step_max < stop_step)
+ stop_step_max = stop_step;
+ if (perform_max < perform)
+ perform_max = perform;
+ }
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ _("infrun: summary: print_frame %s\n"
+ "infrun: summary: stop_step %s\n"
+ "infrun: summary: perform %s\n"),
+ print_frame_max == pf_default
+ ? "pf_default (pf_yes)"
+ : print_frame_max == pf_no ? "pf_no" : "pf_yes",
+ stop_step_max == ss_default
+ ? "ss_default (ss_print_yes (stop_step 0))"
+ : stop_step_max == ss_print_no
+ ? "ss_print_no (stop_step 1)"
+ : "ss_print_yes (stop_step 0)",
+ perform_max == pe_check_more
+ ? "pe_check_more"
+ : perform_max == pe_stop
+ ? "pe_stop" : perform_max == pe_stop_end_range
+ ? "pe_stop_end_range" : "pe_going");
+ if (print_frame_max == pf_default)
+ print_frame_max = pf_yes;
+ if (stop_step_max == ss_default)
+ stop_step_max = ss_print_yes;
+ gdb_assert (perform_max != pe_undef);
+
+ if (perform_shlib)
+ {
+ /* 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);
+ SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
#else
- solib_add (NULL, 0, ¤t_target, auto_solib_add);
+ 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. */
+ target_terminal_inferior ();
+ }
- case BPSTAT_WHAT_KEEP_CHECKING:
+ stop_print_frame = print_frame_max == pf_yes;
+ ecs->event_thread->stop_step = stop_step_max == ss_print_no;
+ switch (perform_max)
+ {
+ case pe_check_more:
+ /* Still need to check other stuff, at least the case
+ where we are stepping and step out of the right range. */
break;
- }
+ case pe_stop_end_range:
+ print_stop_reason (END_STEPPING_RANGE, 0);
+ /* FALLTHRU */
+ case pe_stop:
+ /* We are about to nuke the step_resume_breakpointt via the
+ cleanup chain, so no need to worry about it here. */
+ stop_stepping (ecs);
+ return;
+ case pe_going:
+ keep_going (ecs);
+ return;
+ }
}
/* We come here if we hit a breakpoint but should not
@@ -4258,6 +4430,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/break-solib-event.exp
@@ -0,0 +1,90 @@
+# 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/>.
+
+if {[skip_shlib_tests]} {
+ return 0
+}
+
+set testfile "break-solib-event"
+set libfile "unloadshr"
+set libfile2 "unloadshr2"
+set libname "${testfile}-{libfile}.sl"
+set libname2 "${testfile}-{libfile2}.sl"
+set libsrcfile ${libfile}.c
+set libsrcfile2 ${libfile2}.c
+set srcfile $srcdir/$subdir/unload.c
+set executable $testfile
+set binfile $objdir/$subdir/$executable
+set shlibdir ${objdir}/${subdir}
+set libsrc $srcdir/$subdir/$libfile.c
+set libsrc2 $srcdir/$subdir/$libfile2.c
+set lib_sl $objdir/$subdir/$libname
+set lib_sl2 $objdir/$subdir/$libname2
+set lib_dlopen [shlib_target_file ${libname}]
+set lib_dlopen2 [shlib_target_file ${libname2}]
+set lib_syms [shlib_symbol_file ${libname}]
+set lib_syms2 [shlib_symbol_file ${libname2}]
+
+if [get_compiler_info ${binfile}] {
+ return -1
+}
+
+set lib_opts debug
+set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME\=\"${lib_dlopen}\" additional_flags=-DSHLIB_NAME2\=\"${lib_dlopen2}\"]
+
+if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+ || [gdb_compile_shlib $libsrc2 $lib_sl2 $lib_opts] != ""
+ || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
+ untested "Couldn't compile $libsrc or $libsrc2 or $srcfile."
+ return -1
+}
+
+clean_restart $executable
+gdb_load_shlibs $lib_sl $lib_sl2
+
+gdb_test "set stop-on-solib-events 1"
+
+set event_msg "Stopped due to shared library event"
+
+gdb_run_cmd
+gdb_test "" "${event_msg}.*" "stop at event"
+
+# gdb_breakpoint {*$pc} or {*$} creates a "floating" breakpoint - changing its
+# position on breakpoint_re_set (which happens on bp_shlib_event).
+set test {p/x $pc}
+set event ""
+gdb_test_multiple $test $test {
+ -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
+ set event $expect_out(1,string)
+ pass $test
+ }
+}
+gdb_breakpoint "*$event"
+
+set test {commands $bpnum}
+gdb_test_multiple $test $test {
+ -re "\r\n>$" {
+ pass $test
+ }
+}
+set test {echo event-hit\n}
+gdb_test_multiple $test $test {
+ -re "\r\n>$" {
+ pass $test
+ }
+}
+gdb_test "end"
+
+gdb_test "continue" "${event_msg}.*event-hit.*"
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-05-03 20:02 [patch 3/3] bpstat_what removal Jan Kratochvil
@ 2010-05-04 14:10 ` Stan Shebs
2010-05-07 16:17 ` Jan Kratochvil
0 siblings, 1 reply; 21+ messages in thread
From: Stan Shebs @ 2010-05-04 14:10 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> Hi,
>
> the simple idea is to inline bpstat_what into handle_inferior_event. This
> removes enum bpstat_what_main_action and struct bpstat_what currently acting
> just as an interface between these two functions.
>
There's a reason for this actually, which is that it helps keep the
myriad of random breakpoint types from infecting the rest of GDB.
Breakpoint types are visible globally, and individual breakpoint types
are mentioned here and there in the code, but I think it's worthwhile to
keep the type enumerations / switches in breakpoint.c as much as possible.
There is a pretty good chance that we're going to be doing some
refactoring on how breakpoint types are handled - people are interested
in the idea of "tracing watchpoints" (or "watching tracepoints" :-) )
for instance - so there's a practical reason to provide interfaces that
define the net effect of types, rather than requiring callers to know
aracane details of each.
Stan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-05-04 14:10 ` Stan Shebs
@ 2010-05-07 16:17 ` Jan Kratochvil
2010-05-07 16:26 ` Pedro Alves
0 siblings, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-05-07 16:17 UTC (permalink / raw)
To: Stan Shebs; +Cc: gdb-patches
On Tue, 04 May 2010 16:09:52 +0200, Stan Shebs wrote:
> Jan Kratochvil wrote:
> >Hi,
> >
> >the simple idea is to inline bpstat_what into handle_inferior_event. This
> >removes enum bpstat_what_main_action and struct bpstat_what currently acting
> >just as an interface between these two functions.
>
> There's a reason for this actually, which is that it helps keep the
> myriad of random breakpoint types from infecting the rest of GDB.
This way from existing 24 bp_* breakpoint types you create new artificial
9 BPSTAT_* types with associated 3 STOP_* types, therefore effectively you
create new artificial 11 BPSTAT/STOP_* breakpoint events.
> Breakpoint types are visible globally, and individual breakpoint
> types are mentioned here and there in the code, but I think it's
> worthwhile to keep the type enumerations / switches in breakpoint.c
> as much as possible.
I understand the idea. If there would be 3 or 4 BPSTAT/STOP_* events I would
say it is worth it. But 11 BPSTAT/STOP_* events I find as a too thick
interface to overweight the cost of a new artificial interface at all.
While this citation may be controversial I find it appropriate here:
% And my point is that multiple interfaces are BAD.
%
% There is one interface we _have_ to have: the traditional
% [subject replaced>>>] bp_* types [<<<] one. That one we can't get away from.
%
% "Multiple interfaces" on its own is just confusion with no upside.
%
% You need a _reason_ to have other interfaces. They need to have that
% killer feature. Just being "different" is not a feature at all.
-- Linus Torvalds
Currently already the bp_* types references are not encapsulated in
breakpoint.[ch]:
386 breakpoint.[ch]
51 other files
= 12%
By inlining bpstat_what from breakpoint.c into infrun.c thus moving the 24
bp_* types references from the first line to the second line it gets worse: 18%
Still 12% to 18% is not any radical design change.
Moreover I do not notice in which file which function resides at least myself.
With ctags navigation (+vim :grep -rw) one jumps the references without
noticing any file boundaries. I understand some cscope or some Emacs
navigation is even more seamless.
> There is a pretty good chance that we're going to be doing some
> refactoring on how breakpoint types are handled - people are
> interested in the idea of "tracing watchpoints" (or "watching
> tracepoints" :-) ) for instance - so there's a practical reason to
> provide interfaces that define the net effect of types, rather than
> requiring callers to know aracane details of each.
I see the opposite examples where new breakpoint types always require also new
infrun behavior:
off-trunk archer-jankratochvil-ifunc:
bp_gnu_ifunc_resolver: Place new breakpoint at the return point.
bp_gnu_ifunc_resolver_return: Move target breakpoint to the right address.
off-trunk archer-pmuldoon-next-over-throw2 [by tromey]:
bp_exception: Place bp_exception_resume breakpoint.
bp_exception_resume: Delete the breakpoint and check if we should stop.
These cases mean that each new bp_* type requires new BPSTAT/STOP_* event,
thus nullifying the artificial bpstat_what interface separation.
Still I can keep the BPSTAT_* interface, remove the STOP_* symbols, still
remove the state machine table and keep the bp_* -> BPSTAT_* conversion in
existing bpstat_what. The state machine table has been the primary target of
this patchset. The BPSTAT_* symbols should be still renamed to more
technically describe both their infrun effect and their bp_* types relation.
Thanks,
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-05-07 16:17 ` Jan Kratochvil
@ 2010-05-07 16:26 ` Pedro Alves
2010-05-07 17:02 ` Jan Kratochvil
0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2010-05-07 16:26 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil, Stan Shebs
On Friday 07 May 2010 17:16:48, Jan Kratochvil wrote:
> This way from existing 24 bp_* breakpoint types you create new artificial
> 9 BPSTAT_* types with associated 3 STOP_* types, therefore effectively you
> create new artificial 11 BPSTAT/STOP_* breakpoint events.
The abstraction is a bit broken currently. Most prominantely,
all the 'enum bpstat_what_main_action's should be mutually
exclusive with each other; at least
BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT should _not_ be
a bpstat_what. Checking for internal events is independent of
whether to stop or not (noisily or not), and to single-step over
a breakpoint or not, which is what mostly infrun cares about.
--
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-05-07 16:26 ` Pedro Alves
@ 2010-05-07 17:02 ` Jan Kratochvil
2010-05-07 17:17 ` Pedro Alves
0 siblings, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-05-07 17:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
On Fri, 07 May 2010 18:25:54 +0200, Pedro Alves wrote:
> On Friday 07 May 2010 17:16:48, Jan Kratochvil wrote:
> > This way from existing 24 bp_* breakpoint types you create new artificial
> > 9 BPSTAT_* types with associated 3 STOP_* types, therefore effectively you
> > create new artificial 11 BPSTAT/STOP_* breakpoint events.
>
> The abstraction is a bit broken currently. Most prominantely,
> all the 'enum bpstat_what_main_action's should be mutually
> exclusive with each other; at least
> BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT should _not_ be
> a bpstat_what. Checking for internal events is independent of
> whether to stop or not (noisily or not), and to single-step over
> a breakpoint or not, which is what mostly infrun cares about.
This is one of the reasons of my patch. This was/is the problem of PR 9436
that BPSTAT_WHAT_CHECK_SHLIBS overrides other breakpoint types.
If these actions will no longer be executed immediately in infrun.c there will
have to be more boolean perform_* flags to specify delayed execution of
actions depending on the bp_* types. There is now only perform_shlib (for
former BPSTAT_WHAT_CHECK_SHLIBS) but there will be also perform_jit (and for
ifunc or next-over-throw more such flags). stepping_over_breakpoint setting
may need more abstraction or also just another
perform_stepping_over_breakpoint flag, not sure now.
Thanks,
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-05-07 17:02 ` Jan Kratochvil
@ 2010-05-07 17:17 ` Pedro Alves
2010-05-17 21:46 ` Jan Kratochvil
0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2010-05-07 17:17 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Stan Shebs
On Friday 07 May 2010 17:33:18, Jan Kratochvil wrote:
> This is one of the reasons of my patch. This was/is the problem of PR 9436
> that BPSTAT_WHAT_CHECK_SHLIBS overrides other breakpoint types.
>
> If these actions will no longer be executed immediately in infrun.c there will
> have to be more boolean perform_* flags to specify delayed execution of
> actions depending on the bp_* types. There is now only perform_shlib (for
> former BPSTAT_WHAT_CHECK_SHLIBS) but there will be also perform_jit (and for
> ifunc or next-over-throw more such flags). stepping_over_breakpoint setting
> may need more abstraction or also just another
> perform_stepping_over_breakpoint flag, not sure now.
(assume I mean both shlib events and jit events below)
Why does infrun have to know about checking solib events at
all? Checking for new loaded solibs looks like a detail of this
internal breakpoint. There's not much of inferior run control
related to it. It would seem to me that breakpoint.c could handle
it instead; for example, II have a feeling it should be possible to add
a new breakpoint_ops for shlib_events, similar to the "catch catch"
implementation. And with enough breakpoint_ops abstraction we could
even get rid of the shlib_event breakpoint type, and move the whole
shlib breakpoint definition to solib.c, like ada-lang.c handles
its catchpoints.
--
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-05-07 17:17 ` Pedro Alves
@ 2010-05-17 21:46 ` Jan Kratochvil
2010-06-12 17:02 ` [patch 3/3] bpstat_what removal [rediff] Jan Kratochvil
0 siblings, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-05-17 21:46 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
On Fri, 07 May 2010 19:16:54 +0200, Pedro Alves wrote:
> Why does infrun have to know about checking solib events at
> all? Checking for new loaded solibs looks like a detail of this
> internal breakpoint. There's not much of inferior run control
> related to it. It would seem to me that breakpoint.c could handle
> it instead;
OK, split it now for doing everything doable in breakpoint.c's bpstat_what()
there while the core inferior run control is still left in infrun.c's
handle_inferior_event(). Some of the events are just flagged in bpstat_what()
and delayed to later execution at handle_inferior_event() by:
+ unsigned stepping_over_breakpoint : 1;
+ unsigned bp_longjmp : 1;
+ unsigned bp_longjmp_resume : 1;
+ unsigned bp_step_resume_on_stop : 1;
Also found a better testcase of PR 9436 really turning FSF GDB HEAD FAIL->PASS:
(gdb) run
Starting program: .../gdb/testsuite/gdb.base/nostdlib
Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
28 {
(gdb) FAIL: gdb.base/nostdlib.exp: stop at run
->
Starting program: .../gdb/testsuite/gdb.base/nostdlib
Breakpoint 2, Stopped due to shared library event
_start () at ./gdb.base/nostdlib.c:20
20 {
(gdb) PASS: gdb.base/nostdlib.exp: stop at run
continue
Continuing.
Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
28 {
(gdb) PASS: gdb.base/nostdlib.exp: continue to marker
I have some other local modification on top of it fixing inferior calls where
currently on -nostdlib executables "shlib events" clash with "call dummy".
> for example, II have a feeling it should be possible to add
> a new breakpoint_ops for shlib_events, similar to the "catch catch"
> implementation. And with enough breakpoint_ops abstraction we could
> even get rid of the shlib_event breakpoint type, and move the whole
> shlib breakpoint definition to solib.c, like ada-lang.c handles
> its catchpoints.
Yes; doable later in a different patch.
Thanks,
Jan
gdb/
2010-05-17 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (breakpoint_type_name, bpstat_what_merge)
(bpstat_what_finalize): New functions.
(bpstat_what): Replace.
* breakpoint.h (enum bpstat_what_main_action): Remove.
(struct bpstat_what): Replace.
(bpstat_what): Update comment.
* inferior.h (stop_on_solib_events): New declaration.
(bpstat_what_debug): New prototype.
* infrun.c (stop_on_solib_events): Remove static.
(bpstat_what_debug): New function.
(handle_inferior_event): Clear frame and gdbarch before deciding what
action to take. Remove variable jmp_buf_pc. Replace block with
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
@@ -4150,235 +4150,257 @@ bpstat_stop_status (struct address_space *aspace,
return root_bs->next;
}
-\f
-/* 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,
+/* Return BPTYPE text representation for the purpose of debug messages. */
- /* 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 const char *
+breakpoint_type_name (enum bptype bptype)
+{
+ switch (bptype)
+ {
+ case bp_none:
+ return "bp_none";
+ case bp_breakpoint:
+ return "bp_breakpoint";
+ case bp_hardware_breakpoint:
+ return "bp_hardware_breakpoint";
+ case bp_until:
+ return "bp_until";
+ case bp_finish:
+ return "bp_finish";
+ case bp_watchpoint:
+ return "bp_watchpoint";
+ case bp_hardware_watchpoint:
+ return "bp_hardware_watchpoint";
+ case bp_read_watchpoint:
+ return "bp_read_watchpoint";
+ case bp_access_watchpoint:
+ return "bp_access_watchpoint";
+ case bp_longjmp:
+ return "bp_longjmp";
+ case bp_longjmp_resume:
+ return "bp_longjmp_resume";
+ case bp_step_resume:
+ return "bp_step_resume";
+ case bp_watchpoint_scope:
+ return "bp_watchpoint_scope";
+ case bp_call_dummy:
+ return "bp_call_dummy";
+ case bp_std_terminate:
+ return "bp_std_terminate";
+ case bp_shlib_event:
+ return "bp_shlib_event";
+ case bp_thread_event:
+ return "bp_thread_event";
+ case bp_overlay_event:
+ return "bp_overlay_event";
+ case bp_longjmp_master:
+ return "bp_longjmp_master";
+ case bp_std_terminate_master:
+ return "bp_std_terminate_master";
+ case bp_catchpoint:
+ return "bp_catchpoint";
+ case bp_tracepoint:
+ return "bp_tracepoint";
+ case bp_fast_tracepoint:
+ return "bp_fast_tracepoint";
+ case bp_jit_event:
+ return "bp_jit_event";
+ }
+ internal_error (__FILE__, __LINE__, _("Invalid breakpoint type %d"),
+ (int) bptype);
+ return NULL;
+}
- /* We hit the longjmp_resume breakpoint. */
- long_resume,
+/* Return the more significant events from both A and B. */
- /* We hit the step_resume breakpoint. */
- step_resume,
+struct bpstat_what
+bpstat_what_merge (struct bpstat_what a, struct bpstat_what b)
+{
+ struct bpstat_what retval;
- /* We hit the shared library event breakpoint. */
- shlib_event,
+ retval.print_frame = max (a.print_frame, b.print_frame);
+ retval.stop_step = max (a.stop_step, b.stop_step);
+ retval.perform = max (a.perform, b.perform);
+ retval.stepping_over_breakpoint = max (a.stepping_over_breakpoint,
+ b.stepping_over_breakpoint);
+ retval.bp_longjmp = max (a.bp_longjmp, b.bp_longjmp);
+ retval.bp_longjmp_resume = max (a.bp_longjmp_resume, b.bp_longjmp_resume);
+ retval.bp_step_resume_on_stop = max (a.bp_step_resume_on_stop,
+ b.bp_step_resume_on_stop);
- /* We hit the jit event breakpoint. */
- jit_event,
+ return retval;
+}
- /* 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}
- };
+static void
+bpstat_what_finalize (struct bpstat_what *what)
+{
+ if (what->print_frame == pf_default)
+ what->print_frame = pf_yes;
+ if (what->stop_step == ss_default)
+ what->stop_step = ss_print_yes;
+ gdb_assert (what->perform != pe_undef);
+}
-#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;
+/* Tell what to do about this bpstat. */
+struct bpstat_what
+bpstat_what (bpstat bs)
+{
struct bpstat_what retval;
+ /* solib_add may call breakpoint_re_set which would clear many
+ BREAKPOINT_AT entries still going to be processed. breakpoint_re_set
+ does not keep the same bp_location's even if they actually do not
+ change. */
+ int perform_shlib = 0;
+
+ memset (&retval, 0, sizeof (retval));
+ retval.print_frame = pf_default;
+ retval.stop_step = ss_default;
+ retval.perform = pe_default;
- retval.call_dummy = STOP_NONE;
for (; bs != NULL; bs = bs->next)
{
- enum class bs_class = no_effect;
+ /* Decisions for this specific BS, they get mapped to their *_max
+ variants at the end of this BS processing. */
+ struct bpstat_what this;
+ enum bptype bptype;
+
+ memset (&this, 0, sizeof (this));
+ this.print_frame = pf_default;
+ this.stop_step = ss_default;
+ this.perform = pe_undef;
+
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)
+ {
+ this.stepping_over_breakpoint = 1;
+ bptype = bp_none;
+ }
else
- switch (bs->breakpoint_at->owner->type)
+ bptype = bs->breakpoint_at->owner->type;
+
+ switch (bptype)
{
case bp_none:
- continue;
-
+ this.perform = pe_check_more;
+ break;
case bp_breakpoint:
case bp_hardware_breakpoint:
case bp_until:
case bp_finish:
if (bs->stop)
{
- if (bs->print)
- bs_class = bp_noisy;
- else
- bs_class = bp_silent;
+ this.print_frame = bs->print ? pf_yes : pf_no;
+ this.perform = pe_stop;
}
else
- bs_class = bp_nostop;
+ {
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
+ }
break;
case bp_watchpoint:
case bp_hardware_watchpoint:
case bp_read_watchpoint:
case bp_access_watchpoint:
+ case bp_catchpoint:
if (bs->stop)
{
- if (bs->print)
- bs_class = wp_noisy;
- else
- bs_class = wp_silent;
+ this.print_frame = bs->print ? pf_yes : pf_no;
+ this.perform = pe_stop;
}
else
- /* There was a watchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ {
+ /* There was a watchpoint or catchpoint, but we're not
+ stopping. This requires no further action. */
+ this.perform = pe_check_more;
+ }
break;
case bp_longjmp:
- bs_class = long_jump;
+ this.bp_longjmp = 1;
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_going;
break;
case bp_longjmp_resume:
- bs_class = long_resume;
+ this.bp_longjmp_resume = 1;
+ this.stop_step = ss_print_no;
+ this.perform = pe_stop_end_range;
break;
case bp_step_resume:
if (bs->stop)
{
- bs_class = step_resume;
+ this.bp_step_resume_on_stop = 1;
+ gdb_assert (pe_check_more < pe_going);
+ this.perform = pe_check_more;
}
else
- /* It is for the wrong frame. */
- bs_class = bp_nostop;
+ {
+ /* It is for the wrong frame. */
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
+ }
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.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
break;
- case bp_catchpoint:
- if (bs->stop)
+ case bp_shlib_event:
+ perform_shlib = 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 || stop_stack_dummy)
+ this.perform = pe_stop;
+ else
{
- if (bs->print)
- bs_class = bp_noisy;
- else
- bs_class = bp_silent;
+ /* We want to step over this breakpoint, then keep going. */
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
}
- else
- /* There was a catchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ break;
+ case bp_jit_event:
+ /* Switch terminal for any messages produced by breakpoint_re_set. */
+ target_terminal_ours_for_output ();
+
+ {
+ struct frame_info *frame = get_current_frame ();
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+
+ jit_event_handler (gdbarch);
+ }
+
+ target_terminal_inferior ();
+
+ /* We want to step over this breakpoint, then keep going. */
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
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;
+ stop_stack_dummy = STOP_STACK_DUMMY;
+ this.print_frame = pf_no;
+ this.perform = pe_stop;
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;
+ stop_stack_dummy = STOP_STD_TERMINATE;
+ this.print_frame = pf_no;
+ this.perform = pe_stop;
break;
case bp_tracepoint:
case bp_fast_tracepoint:
@@ -4388,10 +4410,49 @@ bpstat_what (bpstat bs)
internal_error (__FILE__, __LINE__,
_("bpstat_what: tracepoint encountered"));
break;
+ default:
+ internal_error (__FILE__, __LINE__,
+ _("bpstat_what: Unhandled bptype %s"),
+ breakpoint_type_name (bptype));
+ break;
}
- current_action = table[(int) bs_class][(int) current_action];
+
+ /* THIS.PERFORM must be always decided. */
+ if (this.perform == pe_undef)
+ internal_error (__FILE__, __LINE__,
+ _("bpstat_what: Unset perform, bptype %s"),
+ breakpoint_type_name (bptype));
+
+ bpstat_what_debug (this, breakpoint_type_name (bptype), 0);
+
+ retval = bpstat_what_merge (retval, this);
}
- retval.main_action = current_action;
+
+ if (perform_shlib)
+ {
+ /* 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 ();
+ }
+
+ bpstat_what_debug (retval, _("summary"), 1);
+
+ bpstat_what_finalize (&retval);
+
return retval;
}
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -564,53 +564,49 @@ extern bpstat bpstat_stop_status (struct address_space *aspace,
/* This bpstat_what stuff tells wait_for_inferior what to do with a
breakpoint (a challenging task). */
-enum bpstat_what_main_action
+struct 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. It's possible that this should be
- removed from the main_action and put into a separate field, to more
- cleanly handle BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE. */
- 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 step resume breakpoint, and keep checking. */
- BPSTAT_WHAT_STEP_RESUME,
-
- /* Check the dynamic linker's data structures for new libraries, then
- keep checking. */
- BPSTAT_WHAT_CHECK_SHLIBS,
-
- /* Check for new JITed code. */
- BPSTAT_WHAT_CHECK_JIT,
-
- /* This is just used to keep track of how many enums there are. */
- BPSTAT_WHAT_LAST
+ enum print_frame
+ {
+ /* pf_default is pf_yes. */
+ pf_default,
+ /* stop_print_frame value 0. */
+ pf_no,
+ /* stop_print_frame value 1. */
+ pf_yes,
+ }
+ print_frame;
+ enum stop_step
+ {
+ /* ss_default is ss_print_yes. */
+ ss_default,
+ /* ecs->event_thread->stop_step value 1. */
+ ss_print_no,
+ /* ecs->event_thread->stop_step value 0. */
+ ss_print_yes,
+ }
+ stop_step;
+ enum perform
+ {
+ pe_undef,
+ /* Break from this block and check other possibilities why to stop. */
+ pe_check_more,
+ pe_default = pe_check_more,
+ /* Call stop_stepping (ecs). */
+ pe_stop,
+ /* Like pe_stop but also print_stop_reason (END_STEPPING_RANGE, 0). */
+ pe_stop_end_range,
+ /* Call keep_going (ecs) and return without breaking from this block
+ and checking other possibilities why to stop. Some operations need
+ to finish before an already stepped on breakpoint is displayed to
+ the user. */
+ pe_going,
+ }
+ perform;
+ unsigned stepping_over_breakpoint : 1;
+ unsigned bp_longjmp : 1;
+ unsigned bp_longjmp_resume : 1;
+ unsigned bp_step_resume_on_stop : 1;
};
/* An enum indicating the kind of "stack dummy" stop. This is a bit
@@ -627,17 +623,6 @@ enum stop_stack_kind
STOP_STD_TERMINATE
};
-struct bpstat_what
- {
- enum bpstat_what_main_action main_action;
-
- /* Did we hit a call dummy breakpoint? This only goes with a main_action
- of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
- continuing from a call dummy without popping the frame is not a
- useful one). */
- enum stop_stack_kind call_dummy;
- };
-
/* The possible return values for print_bpstat, print_it_normal, print_it_done,
print_it_noop. bpstat_print depends on ther ordering where each item is an
information subset of the previous one. */
@@ -650,7 +635,7 @@ enum print_stop_action
PRINT_UNKNOWN
};
-/* Tell what to do about this bpstat. */
+/* Tell what to do about this list of bpstats. */
struct bpstat_what bpstat_what (bpstat);
\f
/* Find the bpstat associated with a breakpoint. NULL otherwise. */
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -232,10 +232,15 @@ extern char *construct_inferior_arguments (int, char **);
/* From infrun.c */
+extern int stop_on_solib_events;
+
extern void start_remote (int from_tty);
extern void normal_stop (void);
+extern void bpstat_what_debug (struct bpstat_what what, const char *event,
+ int print_defaults);
+
extern int signal_stop_state (int);
extern int signal_print_state (int);
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -215,7 +215,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)
@@ -2847,6 +2847,45 @@ handle_syscall_event (struct execution_control_state *ecs)
return 1;
}
+/* Print debug information about WHAT on DEBUG_INFRUN. EVENT contains string
+ describing what WHAT is for. PRINT_DEFAULTS will force printing event the
+ WHAT fields which contain an unchanged default value. */
+
+void
+bpstat_what_debug (struct bpstat_what what, const char *event,
+ int print_defaults)
+{
+ if (! debug_infrun)
+ return;
+
+ if (print_defaults || what.print_frame != pf_default)
+ fprintf_unfiltered (gdb_stdlog, _("infrun: %s: print_frame %s\n"), event,
+ what.print_frame == pf_default
+ ? "pf_default (pf_yes)"
+ : what.print_frame == pf_no ? "pf_no" : "pf_yes");
+
+ if (print_defaults || what.stop_step != ss_default)
+ fprintf_unfiltered (gdb_stdlog, _("infrun: %s: stop_step %s\n"), event,
+ what.stop_step == ss_default
+ ? "ss_default (ss_print_yes (stop_step 0))"
+ : what.stop_step == ss_print_no
+ ? "ss_print_no (stop_step 1)"
+ : "ss_print_yes (stop_step 0)");
+
+ gdb_assert (what.perform != pe_undef);
+ fprintf_unfiltered (gdb_stdlog, _("infrun: %s: perform %s\n"), event,
+ what.perform == pe_check_more
+ ? "pe_check_more"
+ : what.perform == pe_stop
+ ? "pe_stop" : what.perform == pe_stop_end_range
+ ? "pe_stop_end_range" : "pe_going");
+
+ if (print_defaults || what.stepping_over_breakpoint)
+ fprintf_unfiltered (gdb_stdlog,
+ _("infrun: %s: stepping_over_breakpoint %s\n"), event,
+ what.stepping_over_breakpoint ? "yes" : "no");
+}
+
/* Given an execution control state that has been freshly filled in
by an event from the inferior, figure out what it means and take
appropriate action. */
@@ -3954,185 +3993,104 @@ process_event_stop_test:
return;
}
- /* Handle cases caused by hitting a breakpoint. */
+ /* Breakpoints may get deleted and created in the block below. It calls
+ reinit_frame_cache thus invalidating current_frame. In this block one
+ needs to explicitly get_current_frame. */
+ frame = NULL;
+ gdbarch = NULL;
+
{
- CORE_ADDR jmp_buf_pc;
struct bpstat_what what;
what = bpstat_what (ecs->event_thread->stop_bpstat);
- if (what.call_dummy)
+ if (what.bp_longjmp)
{
- stop_stack_dummy = what.call_dummy;
- }
+ struct frame_info *frame = get_current_frame ();
+ struct gdbarch *gdbarch = get_frame_arch (frame);
- switch (what.main_action)
- {
- case BPSTAT_WHAT_SET_LONGJMP_RESUME:
/* If we hit the breakpoint at longjmp while stepping, we
install a momentary breakpoint at the target of the
jmp_buf. */
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
-
- ecs->event_thread->stepping_over_breakpoint = 1;
-
- if (!gdbarch_get_longjmp_target_p (gdbarch)
- || !gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
+ CORE_ADDR jmp_buf_pc;
+ if (gdbarch_get_longjmp_target_p (gdbarch)
+ && gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
{
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "\
-infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
- keep_going (ecs);
- return;
- }
-
- /* We're going to replace the current step-resume breakpoint
- with a longjmp-resume breakpoint. */
- delete_step_resume_breakpoint (ecs->event_thread);
-
- /* Insert a breakpoint at resume address. */
- insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
-
- keep_going (ecs);
- return;
+ /* We're going to replace the current step-resume breakpoint
+ with a longjmp-resume breakpoint. */
+ delete_step_resume_breakpoint (ecs->event_thread);
- case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
+ /* Insert a breakpoint at resume address. */
+ insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
+ }
+ }
+ if (what.bp_longjmp_resume)
+ {
gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
delete_step_resume_breakpoint (ecs->event_thread);
+ }
- ecs->event_thread->stop_step = 1;
- print_stop_reason (END_STEPPING_RANGE, 0);
- stop_stepping (ecs);
- return;
-
- case BPSTAT_WHAT_SINGLE:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
- ecs->event_thread->stepping_over_breakpoint = 1;
- /* Still need to check other stuff, at least the case
- where we are stepping and step out of the right range. */
- break;
-
- case BPSTAT_WHAT_STOP_NOISY:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
- stop_print_frame = 1;
-
- /* We are about to nuke the step_resume_breakpointt via the
- cleanup chain, so no need to worry about it here. */
-
- stop_stepping (ecs);
- return;
-
- case BPSTAT_WHAT_STOP_SILENT:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
- stop_print_frame = 0;
-
- /* We are about to nuke the step_resume_breakpoin via the
- cleanup chain, so no need to worry about it here. */
-
- stop_stepping (ecs);
- return;
-
- case BPSTAT_WHAT_STEP_RESUME:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
-
+ if (what.bp_step_resume_on_stop)
+ {
delete_step_resume_breakpoint (ecs->event_thread);
if (ecs->event_thread->step_after_step_resume_breakpoint)
{
/* Back when the step-resume breakpoint was inserted, we
were trying to single-step off a breakpoint. Go back
- to doing that. */
+ to doing that. pe_going must override pe_check_more so
+ that we do not stop again on that breakpoint. */
ecs->event_thread->step_after_step_resume_breakpoint = 0;
- ecs->event_thread->stepping_over_breakpoint = 1;
- keep_going (ecs);
- return;
+ what.stepping_over_breakpoint = 1;
+ gdb_assert (pe_check_more < pe_going);
+ if (what.perform < pe_going)
+ what.perform = pe_going;
}
- if (stop_pc == ecs->stop_func_start
- && execution_direction == EXEC_REVERSE)
+ else if (stop_pc == ecs->stop_func_start
+ && execution_direction == EXEC_REVERSE)
{
/* We are stepping over a function call in reverse, and
just hit the step-resume breakpoint at the start
address of the function. Go back to single-stepping,
which should take us back to the function call. */
- ecs->event_thread->stepping_over_breakpoint = 1;
- keep_going (ecs);
- return;
+ what.stepping_over_breakpoint = 1;
+ gdb_assert (pe_check_more < pe_going);
+ if (what.perform < pe_going)
+ what.perform = pe_going;
}
- 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 ();
+ else
+ {
+ if (what.perform < pe_check_more)
+ what.perform = pe_check_more;
+ }
+ }
- /* We want to step over this breakpoint, then keep going. */
- ecs->event_thread->stepping_over_breakpoint = 1;
+ stop_print_frame = what.print_frame == pf_yes;
- break;
+ ecs->event_thread->stop_step = what.stop_step == ss_print_no;
- case BPSTAT_WHAT_LAST:
- /* Not a real code, but listed here to shut up gcc -Wall. */
+ if (what.stepping_over_breakpoint)
+ ecs->event_thread->stepping_over_breakpoint = 1;
- case BPSTAT_WHAT_KEEP_CHECKING:
+ switch (what.perform)
+ {
+ case pe_check_more:
+ /* Still need to check other stuff, at least the case
+ where we are stepping and step out of the right range. */
break;
- }
+ case pe_stop_end_range:
+ print_stop_reason (END_STEPPING_RANGE, 0);
+ /* FALLTHRU */
+ case pe_stop:
+ /* We are about to nuke the step_resume_breakpointt via the
+ cleanup chain, so no need to worry about it here. */
+ stop_stepping (ecs);
+ return;
+ case pe_going:
+ keep_going (ecs);
+ return;
+ }
}
/* We come here if we hit a breakpoint but should not
@@ -4266,6 +4224,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"
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-05-17 21:46 ` Jan Kratochvil
@ 2010-06-12 17:02 ` Jan Kratochvil
2010-06-15 15:08 ` Pedro Alves
0 siblings, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-12 17:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
Hi,
originally posted as:
Re: [patch 3/3] bpstat_what removal
http://sourceware.org/ml/gdb-patches/2010-05/msg00370.html
Rediffed only.
------------------------------------------------------------------------------
On Mon, 17 May 2010 23:45:07 +0200, Jan Kratochvil wrote:
On Fri, 07 May 2010 19:16:54 +0200, Pedro Alves wrote:
> Why does infrun have to know about checking solib events at
> all? Checking for new loaded solibs looks like a detail of this
> internal breakpoint. There's not much of inferior run control
> related to it. It would seem to me that breakpoint.c could handle
> it instead;
OK, split it now for doing everything doable in breakpoint.c's bpstat_what()
there while the core inferior run control is still left in infrun.c's
handle_inferior_event(). Some of the events are just flagged in bpstat_what()
and delayed to later execution at handle_inferior_event() by:
+ unsigned stepping_over_breakpoint : 1;
+ unsigned bp_longjmp : 1;
+ unsigned bp_longjmp_resume : 1;
+ unsigned bp_step_resume_on_stop : 1;
Also found a better testcase of PR 9436 really turning FSF GDB HEAD FAIL->PASS:
(gdb) run
Starting program: .../gdb/testsuite/gdb.base/nostdlib
Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
28 {
(gdb) FAIL: gdb.base/nostdlib.exp: stop at run
->
Starting program: .../gdb/testsuite/gdb.base/nostdlib
Breakpoint 2, Stopped due to shared library event
_start () at ./gdb.base/nostdlib.c:20
20 {
(gdb) PASS: gdb.base/nostdlib.exp: stop at run
continue
Continuing.
Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
28 {
(gdb) PASS: gdb.base/nostdlib.exp: continue to marker
I have some other local modification on top of it fixing inferior calls where
currently on -nostdlib executables "shlib events" clash with "call dummy".
> for example, II have a feeling it should be possible to add
> a new breakpoint_ops for shlib_events, similar to the "catch catch"
> implementation. And with enough breakpoint_ops abstraction we could
> even get rid of the shlib_event breakpoint type, and move the whole
> shlib breakpoint definition to solib.c, like ada-lang.c handles
> its catchpoints.
Yes; doable later in a different patch.
Thanks,
Jan
gdb/
2010-05-17 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (breakpoint_type_name, bpstat_what_merge)
(bpstat_what_finalize): New functions.
(bpstat_what): Replace.
* breakpoint.h (enum bpstat_what_main_action): Remove.
(struct bpstat_what): Replace.
(bpstat_what): Update comment.
* inferior.h (stop_on_solib_events): New declaration.
(bpstat_what_debug): New prototype.
* infrun.c (stop_on_solib_events): Remove static.
(bpstat_what_debug): New function.
(handle_inferior_event): Clear frame and gdbarch before deciding what
action to take. Remove variable jmp_buf_pc. Replace block with
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
@@ -4187,235 +4187,257 @@ bpstat_stop_status (struct address_space *aspace,
return root_bs->next;
}
-\f
-/* 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,
+/* Return BPTYPE text representation for the purpose of debug messages. */
- /* 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 const char *
+breakpoint_type_name (enum bptype bptype)
+{
+ switch (bptype)
+ {
+ case bp_none:
+ return "bp_none";
+ case bp_breakpoint:
+ return "bp_breakpoint";
+ case bp_hardware_breakpoint:
+ return "bp_hardware_breakpoint";
+ case bp_until:
+ return "bp_until";
+ case bp_finish:
+ return "bp_finish";
+ case bp_watchpoint:
+ return "bp_watchpoint";
+ case bp_hardware_watchpoint:
+ return "bp_hardware_watchpoint";
+ case bp_read_watchpoint:
+ return "bp_read_watchpoint";
+ case bp_access_watchpoint:
+ return "bp_access_watchpoint";
+ case bp_longjmp:
+ return "bp_longjmp";
+ case bp_longjmp_resume:
+ return "bp_longjmp_resume";
+ case bp_step_resume:
+ return "bp_step_resume";
+ case bp_watchpoint_scope:
+ return "bp_watchpoint_scope";
+ case bp_call_dummy:
+ return "bp_call_dummy";
+ case bp_std_terminate:
+ return "bp_std_terminate";
+ case bp_shlib_event:
+ return "bp_shlib_event";
+ case bp_thread_event:
+ return "bp_thread_event";
+ case bp_overlay_event:
+ return "bp_overlay_event";
+ case bp_longjmp_master:
+ return "bp_longjmp_master";
+ case bp_std_terminate_master:
+ return "bp_std_terminate_master";
+ case bp_catchpoint:
+ return "bp_catchpoint";
+ case bp_tracepoint:
+ return "bp_tracepoint";
+ case bp_fast_tracepoint:
+ return "bp_fast_tracepoint";
+ case bp_jit_event:
+ return "bp_jit_event";
+ }
+ internal_error (__FILE__, __LINE__, _("Invalid breakpoint type %d"),
+ (int) bptype);
+ return NULL;
+}
- /* We hit the longjmp_resume breakpoint. */
- long_resume,
+/* Return the more significant events from both A and B. */
- /* We hit the step_resume breakpoint. */
- step_resume,
+struct bpstat_what
+bpstat_what_merge (struct bpstat_what a, struct bpstat_what b)
+{
+ struct bpstat_what retval;
- /* We hit the shared library event breakpoint. */
- shlib_event,
+ retval.print_frame = max (a.print_frame, b.print_frame);
+ retval.stop_step = max (a.stop_step, b.stop_step);
+ retval.perform = max (a.perform, b.perform);
+ retval.stepping_over_breakpoint = max (a.stepping_over_breakpoint,
+ b.stepping_over_breakpoint);
+ retval.bp_longjmp = max (a.bp_longjmp, b.bp_longjmp);
+ retval.bp_longjmp_resume = max (a.bp_longjmp_resume, b.bp_longjmp_resume);
+ retval.bp_step_resume_on_stop = max (a.bp_step_resume_on_stop,
+ b.bp_step_resume_on_stop);
- /* We hit the jit event breakpoint. */
- jit_event,
+ return retval;
+}
- /* 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}
- };
+static void
+bpstat_what_finalize (struct bpstat_what *what)
+{
+ if (what->print_frame == pf_default)
+ what->print_frame = pf_yes;
+ if (what->stop_step == ss_default)
+ what->stop_step = ss_print_yes;
+ gdb_assert (what->perform != pe_undef);
+}
-#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;
+/* Tell what to do about this bpstat. */
+struct bpstat_what
+bpstat_what (bpstat bs)
+{
struct bpstat_what retval;
+ /* solib_add may call breakpoint_re_set which would clear many
+ BREAKPOINT_AT entries still going to be processed. breakpoint_re_set
+ does not keep the same bp_location's even if they actually do not
+ change. */
+ int perform_shlib = 0;
+
+ memset (&retval, 0, sizeof (retval));
+ retval.print_frame = pf_default;
+ retval.stop_step = ss_default;
+ retval.perform = pe_default;
- retval.call_dummy = STOP_NONE;
for (; bs != NULL; bs = bs->next)
{
- enum class bs_class = no_effect;
+ /* Decisions for this specific BS, they get mapped to their *_max
+ variants at the end of this BS processing. */
+ struct bpstat_what this;
+ enum bptype bptype;
+
+ memset (&this, 0, sizeof (this));
+ this.print_frame = pf_default;
+ this.stop_step = ss_default;
+ this.perform = pe_undef;
+
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)
+ {
+ this.stepping_over_breakpoint = 1;
+ bptype = bp_none;
+ }
else
- switch (bs->breakpoint_at->owner->type)
+ bptype = bs->breakpoint_at->owner->type;
+
+ switch (bptype)
{
case bp_none:
- continue;
-
+ this.perform = pe_check_more;
+ break;
case bp_breakpoint:
case bp_hardware_breakpoint:
case bp_until:
case bp_finish:
if (bs->stop)
{
- if (bs->print)
- bs_class = bp_noisy;
- else
- bs_class = bp_silent;
+ this.print_frame = bs->print ? pf_yes : pf_no;
+ this.perform = pe_stop;
}
else
- bs_class = bp_nostop;
+ {
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
+ }
break;
case bp_watchpoint:
case bp_hardware_watchpoint:
case bp_read_watchpoint:
case bp_access_watchpoint:
+ case bp_catchpoint:
if (bs->stop)
{
- if (bs->print)
- bs_class = wp_noisy;
- else
- bs_class = wp_silent;
+ this.print_frame = bs->print ? pf_yes : pf_no;
+ this.perform = pe_stop;
}
else
- /* There was a watchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ {
+ /* There was a watchpoint or catchpoint, but we're not
+ stopping. This requires no further action. */
+ this.perform = pe_check_more;
+ }
break;
case bp_longjmp:
- bs_class = long_jump;
+ this.bp_longjmp = 1;
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_going;
break;
case bp_longjmp_resume:
- bs_class = long_resume;
+ this.bp_longjmp_resume = 1;
+ this.stop_step = ss_print_no;
+ this.perform = pe_stop_end_range;
break;
case bp_step_resume:
if (bs->stop)
{
- bs_class = step_resume;
+ this.bp_step_resume_on_stop = 1;
+ gdb_assert (pe_check_more < pe_going);
+ this.perform = pe_check_more;
}
else
- /* It is for the wrong frame. */
- bs_class = bp_nostop;
+ {
+ /* It is for the wrong frame. */
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
+ }
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.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
break;
- case bp_catchpoint:
- if (bs->stop)
+ case bp_shlib_event:
+ perform_shlib = 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 || stop_stack_dummy)
+ this.perform = pe_stop;
+ else
{
- if (bs->print)
- bs_class = bp_noisy;
- else
- bs_class = bp_silent;
+ /* We want to step over this breakpoint, then keep going. */
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
}
- else
- /* There was a catchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ break;
+ case bp_jit_event:
+ /* Switch terminal for any messages produced by breakpoint_re_set. */
+ target_terminal_ours_for_output ();
+
+ {
+ struct frame_info *frame = get_current_frame ();
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+
+ jit_event_handler (gdbarch);
+ }
+
+ target_terminal_inferior ();
+
+ /* We want to step over this breakpoint, then keep going. */
+ this.stepping_over_breakpoint = 1;
+ this.perform = pe_check_more;
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;
+ stop_stack_dummy = STOP_STACK_DUMMY;
+ this.print_frame = pf_no;
+ this.perform = pe_stop;
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;
+ stop_stack_dummy = STOP_STD_TERMINATE;
+ this.print_frame = pf_no;
+ this.perform = pe_stop;
break;
case bp_tracepoint:
case bp_fast_tracepoint:
@@ -4425,10 +4447,49 @@ bpstat_what (bpstat bs)
internal_error (__FILE__, __LINE__,
_("bpstat_what: tracepoint encountered"));
break;
+ default:
+ internal_error (__FILE__, __LINE__,
+ _("bpstat_what: Unhandled bptype %s"),
+ breakpoint_type_name (bptype));
+ break;
}
- current_action = table[(int) bs_class][(int) current_action];
+
+ /* THIS.PERFORM must be always decided. */
+ if (this.perform == pe_undef)
+ internal_error (__FILE__, __LINE__,
+ _("bpstat_what: Unset perform, bptype %s"),
+ breakpoint_type_name (bptype));
+
+ bpstat_what_debug (this, breakpoint_type_name (bptype), 0);
+
+ retval = bpstat_what_merge (retval, this);
}
- retval.main_action = current_action;
+
+ if (perform_shlib)
+ {
+ /* 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 ();
+ }
+
+ bpstat_what_debug (retval, _("summary"), 1);
+
+ bpstat_what_finalize (&retval);
+
return retval;
}
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -565,53 +565,49 @@ extern bpstat bpstat_stop_status (struct address_space *aspace,
/* This bpstat_what stuff tells wait_for_inferior what to do with a
breakpoint (a challenging task). */
-enum bpstat_what_main_action
+struct 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. It's possible that this should be
- removed from the main_action and put into a separate field, to more
- cleanly handle BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE. */
- 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 step resume breakpoint, and keep checking. */
- BPSTAT_WHAT_STEP_RESUME,
-
- /* Check the dynamic linker's data structures for new libraries, then
- keep checking. */
- BPSTAT_WHAT_CHECK_SHLIBS,
-
- /* Check for new JITed code. */
- BPSTAT_WHAT_CHECK_JIT,
-
- /* This is just used to keep track of how many enums there are. */
- BPSTAT_WHAT_LAST
+ enum print_frame
+ {
+ /* pf_default is pf_yes. */
+ pf_default,
+ /* stop_print_frame value 0. */
+ pf_no,
+ /* stop_print_frame value 1. */
+ pf_yes,
+ }
+ print_frame;
+ enum stop_step
+ {
+ /* ss_default is ss_print_yes. */
+ ss_default,
+ /* ecs->event_thread->stop_step value 1. */
+ ss_print_no,
+ /* ecs->event_thread->stop_step value 0. */
+ ss_print_yes,
+ }
+ stop_step;
+ enum perform
+ {
+ pe_undef,
+ /* Break from this block and check other possibilities why to stop. */
+ pe_check_more,
+ pe_default = pe_check_more,
+ /* Call stop_stepping (ecs). */
+ pe_stop,
+ /* Like pe_stop but also print_stop_reason (END_STEPPING_RANGE, 0). */
+ pe_stop_end_range,
+ /* Call keep_going (ecs) and return without breaking from this block
+ and checking other possibilities why to stop. Some operations need
+ to finish before an already stepped on breakpoint is displayed to
+ the user. */
+ pe_going,
+ }
+ perform;
+ unsigned stepping_over_breakpoint : 1;
+ unsigned bp_longjmp : 1;
+ unsigned bp_longjmp_resume : 1;
+ unsigned bp_step_resume_on_stop : 1;
};
/* An enum indicating the kind of "stack dummy" stop. This is a bit
@@ -628,17 +624,6 @@ enum stop_stack_kind
STOP_STD_TERMINATE
};
-struct bpstat_what
- {
- enum bpstat_what_main_action main_action;
-
- /* Did we hit a call dummy breakpoint? This only goes with a main_action
- of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
- continuing from a call dummy without popping the frame is not a
- useful one). */
- enum stop_stack_kind call_dummy;
- };
-
/* The possible return values for print_bpstat, print_it_normal,
print_it_done, print_it_noop. */
enum print_stop_action
@@ -649,7 +634,7 @@ enum print_stop_action
PRINT_NOTHING
};
-/* Tell what to do about this bpstat. */
+/* Tell what to do about this list of bpstats. */
struct bpstat_what bpstat_what (bpstat);
\f
/* Find the bpstat associated with a breakpoint. NULL otherwise. */
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -232,10 +232,15 @@ extern char *construct_inferior_arguments (int, char **);
/* From infrun.c */
+extern int stop_on_solib_events;
+
extern void start_remote (int from_tty);
extern void normal_stop (void);
+extern void bpstat_what_debug (struct bpstat_what what, const char *event,
+ int print_defaults);
+
extern int signal_stop_state (int);
extern int signal_print_state (int);
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -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)
@@ -2926,6 +2926,45 @@ handle_syscall_event (struct execution_control_state *ecs)
return 1;
}
+/* Print debug information about WHAT on DEBUG_INFRUN. EVENT contains string
+ describing what WHAT is for. PRINT_DEFAULTS will force printing event the
+ WHAT fields which contain an unchanged default value. */
+
+void
+bpstat_what_debug (struct bpstat_what what, const char *event,
+ int print_defaults)
+{
+ if (! debug_infrun)
+ return;
+
+ if (print_defaults || what.print_frame != pf_default)
+ fprintf_unfiltered (gdb_stdlog, _("infrun: %s: print_frame %s\n"), event,
+ what.print_frame == pf_default
+ ? "pf_default (pf_yes)"
+ : what.print_frame == pf_no ? "pf_no" : "pf_yes");
+
+ if (print_defaults || what.stop_step != ss_default)
+ fprintf_unfiltered (gdb_stdlog, _("infrun: %s: stop_step %s\n"), event,
+ what.stop_step == ss_default
+ ? "ss_default (ss_print_yes (stop_step 0))"
+ : what.stop_step == ss_print_no
+ ? "ss_print_no (stop_step 1)"
+ : "ss_print_yes (stop_step 0)");
+
+ gdb_assert (what.perform != pe_undef);
+ fprintf_unfiltered (gdb_stdlog, _("infrun: %s: perform %s\n"), event,
+ what.perform == pe_check_more
+ ? "pe_check_more"
+ : what.perform == pe_stop
+ ? "pe_stop" : what.perform == pe_stop_end_range
+ ? "pe_stop_end_range" : "pe_going");
+
+ if (print_defaults || what.stepping_over_breakpoint)
+ fprintf_unfiltered (gdb_stdlog,
+ _("infrun: %s: stepping_over_breakpoint %s\n"), event,
+ what.stepping_over_breakpoint ? "yes" : "no");
+}
+
/* Given an execution control state that has been freshly filled in
by an event from the inferior, figure out what it means and take
appropriate action. */
@@ -4033,185 +4072,104 @@ process_event_stop_test:
return;
}
- /* Handle cases caused by hitting a breakpoint. */
+ /* Breakpoints may get deleted and created in the block below. It calls
+ reinit_frame_cache thus invalidating current_frame. In this block one
+ needs to explicitly get_current_frame. */
+ frame = NULL;
+ gdbarch = NULL;
+
{
- CORE_ADDR jmp_buf_pc;
struct bpstat_what what;
what = bpstat_what (ecs->event_thread->stop_bpstat);
- if (what.call_dummy)
+ if (what.bp_longjmp)
{
- stop_stack_dummy = what.call_dummy;
- }
+ struct frame_info *frame = get_current_frame ();
+ struct gdbarch *gdbarch = get_frame_arch (frame);
- switch (what.main_action)
- {
- case BPSTAT_WHAT_SET_LONGJMP_RESUME:
/* If we hit the breakpoint at longjmp while stepping, we
install a momentary breakpoint at the target of the
jmp_buf. */
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
-
- ecs->event_thread->stepping_over_breakpoint = 1;
-
- if (!gdbarch_get_longjmp_target_p (gdbarch)
- || !gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
+ CORE_ADDR jmp_buf_pc;
+ if (gdbarch_get_longjmp_target_p (gdbarch)
+ && gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
{
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "\
-infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
- keep_going (ecs);
- return;
- }
-
- /* We're going to replace the current step-resume breakpoint
- with a longjmp-resume breakpoint. */
- delete_step_resume_breakpoint (ecs->event_thread);
-
- /* Insert a breakpoint at resume address. */
- insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
-
- keep_going (ecs);
- return;
+ /* We're going to replace the current step-resume breakpoint
+ with a longjmp-resume breakpoint. */
+ delete_step_resume_breakpoint (ecs->event_thread);
- case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
+ /* Insert a breakpoint at resume address. */
+ insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
+ }
+ }
+ if (what.bp_longjmp_resume)
+ {
gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
delete_step_resume_breakpoint (ecs->event_thread);
+ }
- ecs->event_thread->stop_step = 1;
- print_stop_reason (END_STEPPING_RANGE, 0);
- stop_stepping (ecs);
- return;
-
- case BPSTAT_WHAT_SINGLE:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
- ecs->event_thread->stepping_over_breakpoint = 1;
- /* Still need to check other stuff, at least the case
- where we are stepping and step out of the right range. */
- break;
-
- case BPSTAT_WHAT_STOP_NOISY:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
- stop_print_frame = 1;
-
- /* We are about to nuke the step_resume_breakpointt via the
- cleanup chain, so no need to worry about it here. */
-
- stop_stepping (ecs);
- return;
-
- case BPSTAT_WHAT_STOP_SILENT:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
- stop_print_frame = 0;
-
- /* We are about to nuke the step_resume_breakpoin via the
- cleanup chain, so no need to worry about it here. */
-
- stop_stepping (ecs);
- return;
-
- case BPSTAT_WHAT_STEP_RESUME:
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
-
+ if (what.bp_step_resume_on_stop)
+ {
delete_step_resume_breakpoint (ecs->event_thread);
if (ecs->event_thread->step_after_step_resume_breakpoint)
{
/* Back when the step-resume breakpoint was inserted, we
were trying to single-step off a breakpoint. Go back
- to doing that. */
+ to doing that. pe_going must override pe_check_more so
+ that we do not stop again on that breakpoint. */
ecs->event_thread->step_after_step_resume_breakpoint = 0;
- ecs->event_thread->stepping_over_breakpoint = 1;
- keep_going (ecs);
- return;
+ what.stepping_over_breakpoint = 1;
+ gdb_assert (pe_check_more < pe_going);
+ if (what.perform < pe_going)
+ what.perform = pe_going;
}
- if (stop_pc == ecs->stop_func_start
- && execution_direction == EXEC_REVERSE)
+ else if (stop_pc == ecs->stop_func_start
+ && execution_direction == EXEC_REVERSE)
{
/* We are stepping over a function call in reverse, and
just hit the step-resume breakpoint at the start
address of the function. Go back to single-stepping,
which should take us back to the function call. */
- ecs->event_thread->stepping_over_breakpoint = 1;
- keep_going (ecs);
- return;
+ what.stepping_over_breakpoint = 1;
+ gdb_assert (pe_check_more < pe_going);
+ if (what.perform < pe_going)
+ what.perform = pe_going;
}
- 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 ();
+ else
+ {
+ if (what.perform < pe_check_more)
+ what.perform = pe_check_more;
+ }
+ }
- /* We want to step over this breakpoint, then keep going. */
- ecs->event_thread->stepping_over_breakpoint = 1;
+ stop_print_frame = what.print_frame == pf_yes;
- break;
+ ecs->event_thread->stop_step = what.stop_step == ss_print_no;
- case BPSTAT_WHAT_LAST:
- /* Not a real code, but listed here to shut up gcc -Wall. */
+ if (what.stepping_over_breakpoint)
+ ecs->event_thread->stepping_over_breakpoint = 1;
- case BPSTAT_WHAT_KEEP_CHECKING:
+ switch (what.perform)
+ {
+ case pe_check_more:
+ /* Still need to check other stuff, at least the case
+ where we are stepping and step out of the right range. */
break;
- }
+ case pe_stop_end_range:
+ print_stop_reason (END_STEPPING_RANGE, 0);
+ /* FALLTHRU */
+ case pe_stop:
+ /* We are about to nuke the step_resume_breakpointt via the
+ cleanup chain, so no need to worry about it here. */
+ stop_stepping (ecs);
+ return;
+ case pe_going:
+ keep_going (ecs);
+ return;
+ }
}
/* We come here if we hit a breakpoint but should not
@@ -4345,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"
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-12 17:02 ` [patch 3/3] bpstat_what removal [rediff] Jan Kratochvil
@ 2010-06-15 15:08 ` Pedro Alves
2010-06-15 21:54 ` Jan Kratochvil
0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2010-06-15 15:08 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Stan Shebs
On Saturday 12 June 2010 18:01:37, Jan Kratochvil wrote:
> Hi,
>
> originally posted as:
> Re: [patch 3/3] bpstat_what removal
> http://sourceware.org/ml/gdb-patches/2010-05/msg00370.html
>
> Rediffed only.
Do you think it would be hard to split this into smaller pieces? It
would help a lot --- at least me. :-) I've tried to review this a couple
of times already, but it looks tricky and easy to miss something.
- It should be possible to fix PR 9436 without removing bpstat_main_action.
In fact, it's not clear to me yet why is the new interface better. But I
can be convinced. I probably just need it explained to me.
To recap, IMO, the current problem with bpstat_main_action is that a few of
its values aren't really independent and mutually exclusive
with the others -- BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT.
How about fixing the PR that way, and adding the new testcase without all
the revamping? That'd be surely a step in the right direction.
- I feel that even getting rid of the table bpstat_what_main_action
table could be done without changing the interface between breakpoint.c
and infrun.c (removing bpstat_main_action), and other way around too.
That is, it feels like we could tackle these changes independently.
Not sure though. Let me know what you think, and if you disagree, I'll
try harder at reviewing this.
>
> ------------------------------------------------------------------------------
> On Mon, 17 May 2010 23:45:07 +0200, Jan Kratochvil wrote:
>
> On Fri, 07 May 2010 19:16:54 +0200, Pedro Alves wrote:
> > Why does infrun have to know about checking solib events at
> > all? Checking for new loaded solibs looks like a detail of this
> > internal breakpoint. There's not much of inferior run control
> > related to it. It would seem to me that breakpoint.c could handle
> > it instead;
>
> OK, split it now for doing everything doable in breakpoint.c's bpstat_what()
> there while the core inferior run control is still left in infrun.c's
> handle_inferior_event(). Some of the events are just flagged in bpstat_what()
> and delayed to later execution at handle_inferior_event() by:
>
> + unsigned stepping_over_breakpoint : 1;
> + unsigned bp_longjmp : 1;
> + unsigned bp_longjmp_resume : 1;
> + unsigned bp_step_resume_on_stop : 1;
>
> Also found a better testcase of PR 9436 really turning FSF GDB HEAD FAIL->PASS:
> (gdb) run
> Starting program: .../gdb/testsuite/gdb.base/nostdlib
> Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
> 28 {
> (gdb) FAIL: gdb.base/nostdlib.exp: stop at run
> ->
> Starting program: .../gdb/testsuite/gdb.base/nostdlib
> Breakpoint 2, Stopped due to shared library event
> _start () at ./gdb.base/nostdlib.c:20
> 20 {
> (gdb) PASS: gdb.base/nostdlib.exp: stop at run
> continue
> Continuing.
> Breakpoint 1, marker () at ./gdb.base/nostdlib.c:28
> 28 {
> (gdb) PASS: gdb.base/nostdlib.exp: continue to marker
>
> I have some other local modification on top of it fixing inferior calls where
> currently on -nostdlib executables "shlib events" clash with "call dummy".
>
>
> > for example, II have a feeling it should be possible to add
> > a new breakpoint_ops for shlib_events, similar to the "catch catch"
> > implementation. And with enough breakpoint_ops abstraction we could
> > even get rid of the shlib_event breakpoint type, and move the whole
> > shlib breakpoint definition to solib.c, like ada-lang.c handles
> > its catchpoints.
>
> Yes; doable later in a different patch.
>
>
> Thanks,
> Jan
>
>
> gdb/
> 2010-05-17 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * breakpoint.c (breakpoint_type_name, bpstat_what_merge)
> (bpstat_what_finalize): New functions.
> (bpstat_what): Replace.
> * breakpoint.h (enum bpstat_what_main_action): Remove.
> (struct bpstat_what): Replace.
> (bpstat_what): Update comment.
> * inferior.h (stop_on_solib_events): New declaration.
> (bpstat_what_debug): New prototype.
> * infrun.c (stop_on_solib_events): Remove static.
> (bpstat_what_debug): New function.
> (handle_inferior_event): Clear frame and gdbarch before deciding what
> action to take. Remove variable jmp_buf_pc. Replace block with
> 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
> @@ -4187,235 +4187,257 @@ bpstat_stop_status (struct address_space *aspace,
>
> return root_bs->next;
> }
> -\f
> -/* 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,
> +/* Return BPTYPE text representation for the purpose of debug messages. */
>
> - /* 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 const char *
> +breakpoint_type_name (enum bptype bptype)
> +{
> + switch (bptype)
> + {
> + case bp_none:
> + return "bp_none";
> + case bp_breakpoint:
> + return "bp_breakpoint";
> + case bp_hardware_breakpoint:
> + return "bp_hardware_breakpoint";
> + case bp_until:
> + return "bp_until";
> + case bp_finish:
> + return "bp_finish";
> + case bp_watchpoint:
> + return "bp_watchpoint";
> + case bp_hardware_watchpoint:
> + return "bp_hardware_watchpoint";
> + case bp_read_watchpoint:
> + return "bp_read_watchpoint";
> + case bp_access_watchpoint:
> + return "bp_access_watchpoint";
> + case bp_longjmp:
> + return "bp_longjmp";
> + case bp_longjmp_resume:
> + return "bp_longjmp_resume";
> + case bp_step_resume:
> + return "bp_step_resume";
> + case bp_watchpoint_scope:
> + return "bp_watchpoint_scope";
> + case bp_call_dummy:
> + return "bp_call_dummy";
> + case bp_std_terminate:
> + return "bp_std_terminate";
> + case bp_shlib_event:
> + return "bp_shlib_event";
> + case bp_thread_event:
> + return "bp_thread_event";
> + case bp_overlay_event:
> + return "bp_overlay_event";
> + case bp_longjmp_master:
> + return "bp_longjmp_master";
> + case bp_std_terminate_master:
> + return "bp_std_terminate_master";
> + case bp_catchpoint:
> + return "bp_catchpoint";
> + case bp_tracepoint:
> + return "bp_tracepoint";
> + case bp_fast_tracepoint:
> + return "bp_fast_tracepoint";
> + case bp_jit_event:
> + return "bp_jit_event";
> + }
> + internal_error (__FILE__, __LINE__, _("Invalid breakpoint type %d"),
> + (int) bptype);
> + return NULL;
> +}
>
> - /* We hit the longjmp_resume breakpoint. */
> - long_resume,
> +/* Return the more significant events from both A and B. */
>
> - /* We hit the step_resume breakpoint. */
> - step_resume,
> +struct bpstat_what
> +bpstat_what_merge (struct bpstat_what a, struct bpstat_what b)
> +{
> + struct bpstat_what retval;
>
> - /* We hit the shared library event breakpoint. */
> - shlib_event,
> + retval.print_frame = max (a.print_frame, b.print_frame);
> + retval.stop_step = max (a.stop_step, b.stop_step);
> + retval.perform = max (a.perform, b.perform);
> + retval.stepping_over_breakpoint = max (a.stepping_over_breakpoint,
> + b.stepping_over_breakpoint);
> + retval.bp_longjmp = max (a.bp_longjmp, b.bp_longjmp);
> + retval.bp_longjmp_resume = max (a.bp_longjmp_resume, b.bp_longjmp_resume);
> + retval.bp_step_resume_on_stop = max (a.bp_step_resume_on_stop,
> + b.bp_step_resume_on_stop);
>
> - /* We hit the jit event breakpoint. */
> - jit_event,
> + return retval;
> +}
>
> - /* 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}
> - };
> +static void
> +bpstat_what_finalize (struct bpstat_what *what)
> +{
> + if (what->print_frame == pf_default)
> + what->print_frame = pf_yes;
> + if (what->stop_step == ss_default)
> + what->stop_step = ss_print_yes;
> + gdb_assert (what->perform != pe_undef);
> +}
>
> -#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;
> +/* Tell what to do about this bpstat. */
> +struct bpstat_what
> +bpstat_what (bpstat bs)
> +{
> struct bpstat_what retval;
> + /* solib_add may call breakpoint_re_set which would clear many
> + BREAKPOINT_AT entries still going to be processed. breakpoint_re_set
> + does not keep the same bp_location's even if they actually do not
> + change. */
> + int perform_shlib = 0;
> +
> + memset (&retval, 0, sizeof (retval));
> + retval.print_frame = pf_default;
> + retval.stop_step = ss_default;
> + retval.perform = pe_default;
>
> - retval.call_dummy = STOP_NONE;
> for (; bs != NULL; bs = bs->next)
> {
> - enum class bs_class = no_effect;
> + /* Decisions for this specific BS, they get mapped to their *_max
> + variants at the end of this BS processing. */
> + struct bpstat_what this;
> + enum bptype bptype;
> +
> + memset (&this, 0, sizeof (this));
> + this.print_frame = pf_default;
> + this.stop_step = ss_default;
> + this.perform = pe_undef;
> +
> 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)
> + {
> + this.stepping_over_breakpoint = 1;
> + bptype = bp_none;
> + }
> else
> - switch (bs->breakpoint_at->owner->type)
> + bptype = bs->breakpoint_at->owner->type;
> +
> + switch (bptype)
> {
> case bp_none:
> - continue;
> -
> + this.perform = pe_check_more;
> + break;
> case bp_breakpoint:
> case bp_hardware_breakpoint:
> case bp_until:
> case bp_finish:
> if (bs->stop)
> {
> - if (bs->print)
> - bs_class = bp_noisy;
> - else
> - bs_class = bp_silent;
> + this.print_frame = bs->print ? pf_yes : pf_no;
> + this.perform = pe_stop;
> }
> else
> - bs_class = bp_nostop;
> + {
> + this.stepping_over_breakpoint = 1;
> + this.perform = pe_check_more;
> + }
> break;
> case bp_watchpoint:
> case bp_hardware_watchpoint:
> case bp_read_watchpoint:
> case bp_access_watchpoint:
> + case bp_catchpoint:
> if (bs->stop)
> {
> - if (bs->print)
> - bs_class = wp_noisy;
> - else
> - bs_class = wp_silent;
> + this.print_frame = bs->print ? pf_yes : pf_no;
> + this.perform = pe_stop;
> }
> else
> - /* There was a watchpoint, but we're not stopping.
> - This requires no further action. */
> - bs_class = no_effect;
> + {
> + /* There was a watchpoint or catchpoint, but we're not
> + stopping. This requires no further action. */
> + this.perform = pe_check_more;
> + }
> break;
> case bp_longjmp:
> - bs_class = long_jump;
> + this.bp_longjmp = 1;
> + this.stepping_over_breakpoint = 1;
> + this.perform = pe_going;
> break;
> case bp_longjmp_resume:
> - bs_class = long_resume;
> + this.bp_longjmp_resume = 1;
> + this.stop_step = ss_print_no;
> + this.perform = pe_stop_end_range;
> break;
> case bp_step_resume:
> if (bs->stop)
> {
> - bs_class = step_resume;
> + this.bp_step_resume_on_stop = 1;
> + gdb_assert (pe_check_more < pe_going);
> + this.perform = pe_check_more;
> }
> else
> - /* It is for the wrong frame. */
> - bs_class = bp_nostop;
> + {
> + /* It is for the wrong frame. */
> + this.stepping_over_breakpoint = 1;
> + this.perform = pe_check_more;
> + }
> 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.stepping_over_breakpoint = 1;
> + this.perform = pe_check_more;
> break;
> - case bp_catchpoint:
> - if (bs->stop)
> + case bp_shlib_event:
> + perform_shlib = 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 || stop_stack_dummy)
> + this.perform = pe_stop;
> + else
> {
> - if (bs->print)
> - bs_class = bp_noisy;
> - else
> - bs_class = bp_silent;
> + /* We want to step over this breakpoint, then keep going. */
> + this.stepping_over_breakpoint = 1;
> + this.perform = pe_check_more;
> }
> - else
> - /* There was a catchpoint, but we're not stopping.
> - This requires no further action. */
> - bs_class = no_effect;
> + break;
> + case bp_jit_event:
> + /* Switch terminal for any messages produced by breakpoint_re_set. */
> + target_terminal_ours_for_output ();
> +
> + {
> + struct frame_info *frame = get_current_frame ();
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> +
> + jit_event_handler (gdbarch);
> + }
> +
> + target_terminal_inferior ();
> +
> + /* We want to step over this breakpoint, then keep going. */
> + this.stepping_over_breakpoint = 1;
> + this.perform = pe_check_more;
> 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;
> + stop_stack_dummy = STOP_STACK_DUMMY;
> + this.print_frame = pf_no;
> + this.perform = pe_stop;
> 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;
> + stop_stack_dummy = STOP_STD_TERMINATE;
> + this.print_frame = pf_no;
> + this.perform = pe_stop;
> break;
> case bp_tracepoint:
> case bp_fast_tracepoint:
> @@ -4425,10 +4447,49 @@ bpstat_what (bpstat bs)
> internal_error (__FILE__, __LINE__,
> _("bpstat_what: tracepoint encountered"));
> break;
> + default:
> + internal_error (__FILE__, __LINE__,
> + _("bpstat_what: Unhandled bptype %s"),
> + breakpoint_type_name (bptype));
> + break;
> }
> - current_action = table[(int) bs_class][(int) current_action];
> +
> + /* THIS.PERFORM must be always decided. */
> + if (this.perform == pe_undef)
> + internal_error (__FILE__, __LINE__,
> + _("bpstat_what: Unset perform, bptype %s"),
> + breakpoint_type_name (bptype));
> +
> + bpstat_what_debug (this, breakpoint_type_name (bptype), 0);
> +
> + retval = bpstat_what_merge (retval, this);
> }
> - retval.main_action = current_action;
> +
> + if (perform_shlib)
> + {
> + /* 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 ();
> + }
> +
> + bpstat_what_debug (retval, _("summary"), 1);
> +
> + bpstat_what_finalize (&retval);
> +
> return retval;
> }
>
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -565,53 +565,49 @@ extern bpstat bpstat_stop_status (struct address_space *aspace,
> /* This bpstat_what stuff tells wait_for_inferior what to do with a
> breakpoint (a challenging task). */
>
> -enum bpstat_what_main_action
> +struct 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. It's possible that this should be
> - removed from the main_action and put into a separate field, to more
> - cleanly handle BPSTAT_WHAT_CLEAR_LONGJMP_RESUME_SINGLE. */
> - 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 step resume breakpoint, and keep checking. */
> - BPSTAT_WHAT_STEP_RESUME,
> -
> - /* Check the dynamic linker's data structures for new libraries, then
> - keep checking. */
> - BPSTAT_WHAT_CHECK_SHLIBS,
> -
> - /* Check for new JITed code. */
> - BPSTAT_WHAT_CHECK_JIT,
> -
> - /* This is just used to keep track of how many enums there are. */
> - BPSTAT_WHAT_LAST
> + enum print_frame
> + {
> + /* pf_default is pf_yes. */
> + pf_default,
> + /* stop_print_frame value 0. */
> + pf_no,
> + /* stop_print_frame value 1. */
> + pf_yes,
> + }
> + print_frame;
> + enum stop_step
> + {
> + /* ss_default is ss_print_yes. */
> + ss_default,
> + /* ecs->event_thread->stop_step value 1. */
> + ss_print_no,
> + /* ecs->event_thread->stop_step value 0. */
> + ss_print_yes,
> + }
> + stop_step;
> + enum perform
> + {
> + pe_undef,
> + /* Break from this block and check other possibilities why to stop. */
> + pe_check_more,
> + pe_default = pe_check_more,
> + /* Call stop_stepping (ecs). */
> + pe_stop,
> + /* Like pe_stop but also print_stop_reason (END_STEPPING_RANGE, 0). */
> + pe_stop_end_range,
> + /* Call keep_going (ecs) and return without breaking from this block
> + and checking other possibilities why to stop. Some operations need
> + to finish before an already stepped on breakpoint is displayed to
> + the user. */
> + pe_going,
> + }
> + perform;
> + unsigned stepping_over_breakpoint : 1;
> + unsigned bp_longjmp : 1;
> + unsigned bp_longjmp_resume : 1;
> + unsigned bp_step_resume_on_stop : 1;
> };
>
> /* An enum indicating the kind of "stack dummy" stop. This is a bit
> @@ -628,17 +624,6 @@ enum stop_stack_kind
> STOP_STD_TERMINATE
> };
>
> -struct bpstat_what
> - {
> - enum bpstat_what_main_action main_action;
> -
> - /* Did we hit a call dummy breakpoint? This only goes with a main_action
> - of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
> - continuing from a call dummy without popping the frame is not a
> - useful one). */
> - enum stop_stack_kind call_dummy;
> - };
> -
> /* The possible return values for print_bpstat, print_it_normal,
> print_it_done, print_it_noop. */
> enum print_stop_action
> @@ -649,7 +634,7 @@ enum print_stop_action
> PRINT_NOTHING
> };
>
> -/* Tell what to do about this bpstat. */
> +/* Tell what to do about this list of bpstats. */
> struct bpstat_what bpstat_what (bpstat);
> \f
> /* Find the bpstat associated with a breakpoint. NULL otherwise. */
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -232,10 +232,15 @@ extern char *construct_inferior_arguments (int, char **);
>
> /* From infrun.c */
>
> +extern int stop_on_solib_events;
> +
> extern void start_remote (int from_tty);
>
> extern void normal_stop (void);
>
> +extern void bpstat_what_debug (struct bpstat_what what, const char *event,
> + int print_defaults);
> +
> extern int signal_stop_state (int);
>
> extern int signal_print_state (int);
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -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)
> @@ -2926,6 +2926,45 @@ handle_syscall_event (struct execution_control_state *ecs)
> return 1;
> }
>
> +/* Print debug information about WHAT on DEBUG_INFRUN. EVENT contains string
> + describing what WHAT is for. PRINT_DEFAULTS will force printing event the
> + WHAT fields which contain an unchanged default value. */
> +
> +void
> +bpstat_what_debug (struct bpstat_what what, const char *event,
> + int print_defaults)
> +{
> + if (! debug_infrun)
> + return;
> +
> + if (print_defaults || what.print_frame != pf_default)
> + fprintf_unfiltered (gdb_stdlog, _("infrun: %s: print_frame %s\n"), event,
> + what.print_frame == pf_default
> + ? "pf_default (pf_yes)"
> + : what.print_frame == pf_no ? "pf_no" : "pf_yes");
> +
> + if (print_defaults || what.stop_step != ss_default)
> + fprintf_unfiltered (gdb_stdlog, _("infrun: %s: stop_step %s\n"), event,
> + what.stop_step == ss_default
> + ? "ss_default (ss_print_yes (stop_step 0))"
> + : what.stop_step == ss_print_no
> + ? "ss_print_no (stop_step 1)"
> + : "ss_print_yes (stop_step 0)");
> +
> + gdb_assert (what.perform != pe_undef);
> + fprintf_unfiltered (gdb_stdlog, _("infrun: %s: perform %s\n"), event,
> + what.perform == pe_check_more
> + ? "pe_check_more"
> + : what.perform == pe_stop
> + ? "pe_stop" : what.perform == pe_stop_end_range
> + ? "pe_stop_end_range" : "pe_going");
> +
> + if (print_defaults || what.stepping_over_breakpoint)
> + fprintf_unfiltered (gdb_stdlog,
> + _("infrun: %s: stepping_over_breakpoint %s\n"), event,
> + what.stepping_over_breakpoint ? "yes" : "no");
> +}
> +
> /* Given an execution control state that has been freshly filled in
> by an event from the inferior, figure out what it means and take
> appropriate action. */
> @@ -4033,185 +4072,104 @@ process_event_stop_test:
> return;
> }
>
> - /* Handle cases caused by hitting a breakpoint. */
> + /* Breakpoints may get deleted and created in the block below. It calls
> + reinit_frame_cache thus invalidating current_frame. In this block one
> + needs to explicitly get_current_frame. */
> + frame = NULL;
> + gdbarch = NULL;
> +
> {
> - CORE_ADDR jmp_buf_pc;
> struct bpstat_what what;
>
> what = bpstat_what (ecs->event_thread->stop_bpstat);
>
> - if (what.call_dummy)
> + if (what.bp_longjmp)
> {
> - stop_stack_dummy = what.call_dummy;
> - }
> + struct frame_info *frame = get_current_frame ();
> + struct gdbarch *gdbarch = get_frame_arch (frame);
>
> - switch (what.main_action)
> - {
> - case BPSTAT_WHAT_SET_LONGJMP_RESUME:
> /* If we hit the breakpoint at longjmp while stepping, we
> install a momentary breakpoint at the target of the
> jmp_buf. */
>
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog,
> - "infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");
> -
> - ecs->event_thread->stepping_over_breakpoint = 1;
> -
> - if (!gdbarch_get_longjmp_target_p (gdbarch)
> - || !gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
> + CORE_ADDR jmp_buf_pc;
> + if (gdbarch_get_longjmp_target_p (gdbarch)
> + && gdbarch_get_longjmp_target (gdbarch, frame, &jmp_buf_pc))
> {
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog, "\
> -infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
> - keep_going (ecs);
> - return;
> - }
> -
> - /* We're going to replace the current step-resume breakpoint
> - with a longjmp-resume breakpoint. */
> - delete_step_resume_breakpoint (ecs->event_thread);
> -
> - /* Insert a breakpoint at resume address. */
> - insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
> -
> - keep_going (ecs);
> - return;
> + /* We're going to replace the current step-resume breakpoint
> + with a longjmp-resume breakpoint. */
> + delete_step_resume_breakpoint (ecs->event_thread);
>
> - case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog,
> - "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
> + /* Insert a breakpoint at resume address. */
> + insert_longjmp_resume_breakpoint (gdbarch, jmp_buf_pc);
> + }
> + }
>
> + if (what.bp_longjmp_resume)
> + {
> gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL);
> delete_step_resume_breakpoint (ecs->event_thread);
> + }
>
> - ecs->event_thread->stop_step = 1;
> - print_stop_reason (END_STEPPING_RANGE, 0);
> - stop_stepping (ecs);
> - return;
> -
> - case BPSTAT_WHAT_SINGLE:
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
> - ecs->event_thread->stepping_over_breakpoint = 1;
> - /* Still need to check other stuff, at least the case
> - where we are stepping and step out of the right range. */
> - break;
> -
> - case BPSTAT_WHAT_STOP_NOISY:
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
> - stop_print_frame = 1;
> -
> - /* We are about to nuke the step_resume_breakpointt via the
> - cleanup chain, so no need to worry about it here. */
> -
> - stop_stepping (ecs);
> - return;
> -
> - case BPSTAT_WHAT_STOP_SILENT:
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
> - stop_print_frame = 0;
> -
> - /* We are about to nuke the step_resume_breakpoin via the
> - cleanup chain, so no need to worry about it here. */
> -
> - stop_stepping (ecs);
> - return;
> -
> - case BPSTAT_WHAT_STEP_RESUME:
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
> -
> + if (what.bp_step_resume_on_stop)
> + {
> delete_step_resume_breakpoint (ecs->event_thread);
> if (ecs->event_thread->step_after_step_resume_breakpoint)
> {
> /* Back when the step-resume breakpoint was inserted, we
> were trying to single-step off a breakpoint. Go back
> - to doing that. */
> + to doing that. pe_going must override pe_check_more so
> + that we do not stop again on that breakpoint. */
> ecs->event_thread->step_after_step_resume_breakpoint = 0;
> - ecs->event_thread->stepping_over_breakpoint = 1;
> - keep_going (ecs);
> - return;
> + what.stepping_over_breakpoint = 1;
> + gdb_assert (pe_check_more < pe_going);
> + if (what.perform < pe_going)
> + what.perform = pe_going;
> }
> - if (stop_pc == ecs->stop_func_start
> - && execution_direction == EXEC_REVERSE)
> + else if (stop_pc == ecs->stop_func_start
> + && execution_direction == EXEC_REVERSE)
> {
> /* We are stepping over a function call in reverse, and
> just hit the step-resume breakpoint at the start
> address of the function. Go back to single-stepping,
> which should take us back to the function call. */
> - ecs->event_thread->stepping_over_breakpoint = 1;
> - keep_going (ecs);
> - return;
> + what.stepping_over_breakpoint = 1;
> + gdb_assert (pe_check_more < pe_going);
> + if (what.perform < pe_going)
> + what.perform = pe_going;
> }
> - 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 ();
> + else
> + {
> + if (what.perform < pe_check_more)
> + what.perform = pe_check_more;
> + }
> + }
>
> - /* We want to step over this breakpoint, then keep going. */
> - ecs->event_thread->stepping_over_breakpoint = 1;
> + stop_print_frame = what.print_frame == pf_yes;
>
> - break;
> + ecs->event_thread->stop_step = what.stop_step == ss_print_no;
>
> - case BPSTAT_WHAT_LAST:
> - /* Not a real code, but listed here to shut up gcc -Wall. */
> + if (what.stepping_over_breakpoint)
> + ecs->event_thread->stepping_over_breakpoint = 1;
>
> - case BPSTAT_WHAT_KEEP_CHECKING:
> + switch (what.perform)
> + {
> + case pe_check_more:
> + /* Still need to check other stuff, at least the case
> + where we are stepping and step out of the right range. */
> break;
> - }
> + case pe_stop_end_range:
> + print_stop_reason (END_STEPPING_RANGE, 0);
> + /* FALLTHRU */
> + case pe_stop:
> + /* We are about to nuke the step_resume_breakpointt via the
> + cleanup chain, so no need to worry about it here. */
> + stop_stepping (ecs);
> + return;
> + case pe_going:
> + keep_going (ecs);
> + return;
> + }
> }
>
> /* We come here if we hit a breakpoint but should not
> @@ -4345,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"
>
--
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-15 15:08 ` Pedro Alves
@ 2010-06-15 21:54 ` Jan Kratochvil
2010-06-16 19:13 ` Pedro Alves
2010-06-16 20:40 ` [patch 3/3] bpstat_what removal [rediff] Tom Tromey
0 siblings, 2 replies; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-15 21:54 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
On Tue, 15 Jun 2010 17:08:19 +0200, Pedro Alves wrote:
> Do you think it would be hard to split this into smaller pieces?
Yes, I will do - just later. IMO we should probably more talk about its goal
than about specifics of the implementation.
> I've tried to review this a couple
> of times already, but it looks tricky and easy to miss something.
I wrote some notes for it before, attached them for illustration. I believe
one should not follow them when reviewing the patch - for obvious reasons.
> - It should be possible to fix PR 9436 without
We can pretend PR 9436 does not exist. I noted a wish of removing bpstat_what
already on #gdb 2008-12-31T22:51:10Z I believe not aware of this PR that time.
I believe the wish lasts longer than I have expressed it on #gdb.
> In fact, it's not clear to me yet why is the new interface better.
The interface was better (=completely removed) in the original post:
[patch 3/3] bpstat_what removal
http://sourceware.org/ml/gdb-patches/2010-05/msg00050.html
Currently the interface was created with the goal of not introducing anything
new. All its element have both their names and functionality exactly matching
the already existing (and currently required to stay in place) GDB data
structures. They had to copy them to comply with the separation requirements:
Re: [patch 3/3] bpstat_what removal
http://sourceware.org/ml/gdb-patches/2010-05/msg00186.html
"names and functionality exactly matching" - in the case of elements like
`ss_print_no' one can single-key jump on its definition to see
`ecs->event_thread->stop_step value 1.', naming the element
ecs_event_thread_stop_step_value_1 would be probably worse but I can change it
to the longer form if anyone would prefer it that way.
Contrary to it what exactly does BPSTAT_WHAT_STEP_RESUME and how to combine it
with other events is unclear to me.
/* Clear step resume breakpoint, and keep checking. */
Moreover I find this patch only as a first step for further simplifications.
For instance at least ecs->event_thread->stop_step and stop_print_frame should
definitely be changed and also simplified. This is not a part of this patch
(and I admit I currently do not a plan to do such next part soon).
Nullifying the layer makes it easier for futher simplifications downwards.
> To recap, IMO, the current problem with bpstat_main_action is that a few of
> its values aren't really independent and mutually exclusive
> with the others -- BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT.
What if a new breakpoint type wants to stop? What if a new breakpoint type
does not want to stop? And how they combine if they happend together with
other events? While there exists answer for it I do not know offhand. I know
offhand with my implementation. I should post a patch introducing new
breakpoint types in both variants of the bpstat_what implementation.
> How about fixing the PR that way, and adding the new testcase without all
> the revamping? That'd be surely a step in the right direction.
The revamping was the goal. I only found the PR to have some advocacy for its
acceptance but to be honest I would prefer to forget about the PR now at all.
> - I feel that even getting rid of the table bpstat_what_main_action
> table
Yes, it has to be done.
> could be done without changing the interface between breakpoint.c
> and infrun.c (removing bpstat_main_action), and other way around too.
Changing the interface was also the goal - the goal of simplifying GDB.
So far I thought the patches should have the goal of making the patched code
the best we can (in this case to make it more simple). If we track the goal
of a minimal patchset to reach some functionality we can completely drop this
patch as its primary goal is no new functionality but just a code
simplification (plus simplification of future code extensions).
> That is, it feels like we could tackle these changes independently.
> Not sure though.
Probably it can although I find such patch more difficult than this one.
I just mapped the effect of the current code and wrote a new code behaving
(hopefully) the same.
> Let me know what you think, and if you disagree, I'll try harder at
> reviewing this.
I believe we should try to find the shared goal of such patch first.
Thanks for reply!
Jan
------------------------------------------------------------------------------
bs->breakpoint_at == NULL no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_none no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_watchpoint && !stop no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_hardware_watchpoint && !stop no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_read_watchpoint && !stop no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_access_watchpoint && !stop no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_catchpoint && !stop no_effect kc BPSTAT_WHAT_KEEP_CHECKING
bp_watchpoint && stop && !print wp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_hardware_watchpoint && stop && !print wp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_read_watchpoint && stop && !print wp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_access_watchpoint && stop && !print wp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_breakpoint && stop && !print bp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_hardware_breakpoint && stop && !print bp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_until && stop && !print bp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_finish && stop && !print bp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_catchpoint && stop && !print bp_silent ss BPSTAT_WHAT_STOP_SILENT
bp_call_dummy bp_silent ss BPSTAT_WHAT_STOP_SILENT STOP_STACK_DUMMY
bp_std_terminate bp_silent ss BPSTAT_WHAT_STOP_SILENT STOP_STD_TERMINATE
bp_watchpoint && stop && print wp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_hardware_watchpoint && stop && print wp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_read_watchpoint && stop && print wp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_access_watchpoint && stop && print wp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_breakpoint && stop && print bp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_hardware_breakpoint && stop && print bp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_until && stop && print bp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_finish && stop && print bp_noisy sn BPSTAT_WHAT_STOP_NOISY
bp_catchpoint && stop && print bp_noisy sn BPSTAT_WHAT_STOP_NOISY
bs->breakpoint_at->owner == NULL bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_breakpoint && !stop bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_hardware_breakpoint && !stop bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_until && !stop bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_finish && !stop bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_step_resume && !stop bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_watchpoint_scope bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_thread_event bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_overlay_event bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_longjmp_master bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_std_terminate_master bp_nostop sgl BPSTAT_WHAT_SINGLE
bp_longjmp long_jump slr BPSTAT_WHAT_SET_LONGJMP_RESUME
bp_longjmp_resume long_resume clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
bp_step_resume && stop step_resume sr BPSTAT_WHAT_STEP_RESUME
bp_shlib_event shlib shl BPSTAT_WHAT_CHECK_SHLIBS
bp_jit_event jit_event jit BPSTAT_WHAT_CHECK_JIT
bp_tracepoint internal_error ("bpstat_what: tracepoint encountered")
bp_fast_tracepoint internal_error ("bpstat_what: tracepoint encountered")
err:
long_jump && clr
long_resume && sgl
long_resume && slr
long_resume && clr
#define kc BPSTAT_WHAT_KEEP_CHECKING
break;
#define ss BPSTAT_WHAT_STOP_SILENT
stop_print_frame = 0;
stop_stepping (ecs);
return;
#define sn BPSTAT_WHAT_STOP_NOISY
stop_print_frame = 1;
stop_stepping (ecs);
return;
#define sgl BPSTAT_WHAT_SINGLE
ecs->event_thread->stepping_over_breakpoint = 1;
break;
#define slr BPSTAT_WHAT_SET_LONGJMP_RESUME
special
ecs->event_thread->stepping_over_breakpoint = 1;
keep_going (ecs);
return;
#define clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
special
ecs->event_thread->stop_step = 1;
stop_stepping (ecs);
return;
#define sr BPSTAT_WHAT_STEP_RESUME
special
if (special2)
ecs->event_thread->stepping_over_breakpoint = 1;
keep_going (ecs);
return;
else
break;
#define shl BPSTAT_WHAT_CHECK_SHLIBS
special
if (stop_on_solib_events || is (STOP_STACK_DUMMY) || is (STOP_STD_TERMINATE))
stop_stepping (ecs);
return;
else
ecs->event_thread->stepping_over_breakpoint = 1;
break;
#define jit BPSTAT_WHAT_CHECK_JIT
special
ecs->event_thread->stepping_over_breakpoint = 1;
break;
BPSTAT_WHAT_LAST
#define err BPSTAT_WHAT_STOP_NOISY
default = force keep-going | check reasons why to stop? | always stop
default = stepping_over_breakpoint no | yes
ecs->event_thread->stop_step default=0 | 1 | 0
stop_print_frame default == 1 | 0 | 1
< BPSTAT_WHAT_KEEP_CHECKING: Check reasons why to stop otherwise keep-going.
< BPSTAT_WHAT_SINGLE: Check reasons why to stop, otherwise force stepping over breakpoint.
< STEPPING_KEEP_GOING: Force stepping over breakpoint, suppressing checking reasons why to stop.
< STOP_STEP: Always stop (not printing the source line).
< STOP_STEPPING: Stronger than STOP_STEP by printing the source line (stop_step == 0).
< BPSTAT_WHAT_STOP_SILENT: Always stop (stop_print_frame == 0).
< BPSTAT_WHAT_STOP_NOISY: Stronger than BPSTAT_WHAT_STOP_SILENT by stop_print_frame == 1.
------------------------------------------------------------------------------
STOP_STEPPING
stop_stepping (ecs);
return;
STOP_STEP
ecs->event_thread->stop_step = 1;
stop_stepping (ecs);
return;
STEPPING_KEEP_GOING
ecs->event_thread->stepping_over_breakpoint = 1;
keep_going (ecs);
return;
#define kc BPSTAT_WHAT_KEEP_CHECKING
break;
#define ss BPSTAT_WHAT_STOP_SILENT
stop_print_frame = 0;
stop_stepping (ecs);
return;
#define sn BPSTAT_WHAT_STOP_NOISY
stop_print_frame = 1;
stop_stepping (ecs);
return;
#define sgl BPSTAT_WHAT_SINGLE
ecs->event_thread->stepping_over_breakpoint = 1;
break;
------------------------------------------------------------------------------
#define slr BPSTAT_WHAT_SET_LONGJMP_RESUME
special
STEPPING_KEEP_GOING
#define clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
special
STOP_STEP
#define sr BPSTAT_WHAT_STEP_RESUME
special
if (special2)
STEPPING_KEEP_GOING
else
BPSTAT_WHAT_KEEP_CHECKING
#define shl BPSTAT_WHAT_CHECK_SHLIBS
special
if (stop_on_solib_events || is (STOP_STACK_DUMMY) || is (STOP_STD_TERMINATE))
STOP_STEPPING
else
BPSTAT_WHAT_SINGLE
#define jit BPSTAT_WHAT_CHECK_JIT
special
BPSTAT_WHAT_SINGLE
KEEP_CHECKING
STOP_SILENT
STOP_NOISY
SINGLE
SET_LONGJMP_RESUME
CLEAR_LONGJMP_RESUME
STEP_RESUME
CHECK_SHLIBS
CHECK_JIT
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-15 21:54 ` Jan Kratochvil
@ 2010-06-16 19:13 ` Pedro Alves
2010-06-18 10:41 ` Jan Kratochvil
2010-06-24 14:44 ` [patch 3/3] bpstat_what removal Jan Kratochvil
2010-06-16 20:40 ` [patch 3/3] bpstat_what removal [rediff] Tom Tromey
1 sibling, 2 replies; 21+ messages in thread
From: Pedro Alves @ 2010-06-16 19:13 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Stan Shebs
On Tuesday 15 June 2010 22:54:02, Jan Kratochvil wrote:
> On Tue, 15 Jun 2010 17:08:19 +0200, Pedro Alves wrote:
> > Do you think it would be hard to split this into smaller pieces?
>
> Yes, I will do - just later. IMO we should probably more talk about its goal
> than about specifics of the implementation.
I'll just do it now.
> > I've tried to review this a couple
> > of times already, but it looks tricky and easy to miss something.
>
> I wrote some notes for it before, attached them for illustration. I believe
> one should not follow them when reviewing the patch - for obvious reasons.
>
>
> > - It should be possible to fix PR 9436 without
>
> We can pretend PR 9436 does not exist.
I disagree. Fixing this exact design problem will make it much
clearer what needs to be done next.
> I noted a wish of removing bpstat_what
> already on #gdb 2008-12-31T22:51:10Z I believe not aware of this PR that time.
> I believe the wish lasts longer than I have expressed it on #gdb.
The comment mentioning that the table is not really needed is there
since 5.0. That's like 10 years ago. :-)
> > In fact, it's not clear to me yet why is the new interface better.
>
> The interface was better (=completely removed) in the original post:
> [patch 3/3] bpstat_what removal
> http://sourceware.org/ml/gdb-patches/2010-05/msg00050.html
Nope, that was certainly much worse, for the reason Stan already
explained in that thread.
> Currently the interface was created with the goal of not introducing anything
> new. All its element have both their names and functionality exactly matching
> the already existing (and currently required to stay in place) GDB data
> structures. They had to copy them to comply with the separation requirements:
> Re: [patch 3/3] bpstat_what removal
> http://sourceware.org/ml/gdb-patches/2010-05/msg00186.html
I don't see how that relates, sorry.
> Contrary to it what exactly does BPSTAT_WHAT_STEP_RESUME and how to combine it
> with other events is unclear to me.
> /* Clear step resume breakpoint, and keep checking. */
Yes, there's an issue with step-resume's priority over other events.
step-resume takes priority over all other actions, even stops for other
breakpoints. It usually just works (breakpoint on top of step-resume
breakpoint, for instance), because when you hit the step-resume, the step
range _also_ ends, so we stop anyway. I can see that the (infrun.c)
optimization around:
/* We are at the start of a different line. So stop. Note that
we don't stop if we step into the middle of a different line.
That is said to make things like for (;;) statements work
better. */
can make us miss a breakpoint in some rare cases. Acknowledging this
doesn't mean we can't tackle one issue at a time.
> > To recap, IMO, the current problem with bpstat_main_action is that a few of
> > its values aren't really independent and mutually exclusive
> > with the others -- BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT.
>
> What if a new breakpoint type wants to stop?
BPSTAT_WHAT_STOP_SILENT / BPSTAT_WHAT_STOP_NOISY.
> What if a new breakpoint type
> does not want to stop?
BPSTAT_WHAT_SINGLE
> And how they combine if they happend together with
> other events?
The highest priority of the above wins. If you're going to stop,
you'll never want to set longjmp or step resume breakpoints, or
start a step over. You're just going to stop, period.
(My feeling is that we should completely move the infrun-specific
breakpoints handling to infrun (longjmp, longjmp-resume, step-resume),
though it isn't clear _how_ is best. I hope that by eliminating
the table first, we can think of better interfaces afterwards.
We may even end up with something like yours, I'm not rejecting
it upfront, to be clear.)
> > How about fixing the PR that way, and adding the new testcase without all
> > the revamping? That'd be surely a step in the right direction.
>
> The revamping was the goal. I only found the PR to have some advocacy for its
> acceptance but to be honest I would prefer to forget about the PR now at all.
I beg to differ :-)
>
>
> > - I feel that even getting rid of the table bpstat_what_main_action
> > table
>
> Yes, it has to be done.
It doesn't _have_ to. There's nothing wrong with it, if you look at
it as a state machine (and once you remove the states that should never
have been put there in the first place). But yeah, there's a lot of
unnecessary indirection. I don't find it hard to read, but you do
have to think a bit about it to understand it the first time;
afterwards, it's like riding a bike. Basically, removing it means
moving from a data driven table to a code/switch driven table; a
different dressing, but it's mostly the same underneath. :-)
> > could be done without changing the interface between breakpoint.c
> > and infrun.c (removing bpstat_main_action), and other way around too.
>
> Changing the interface was also the goal - the goal of simplifying GDB.
>
> So far I thought the patches should have the goal of making the patched code
> the best we can (in this case to make it more simple). If we track the goal
> of a minimal patchset to reach some functionality we can completely drop this
> patch as its primary goal is no new functionality but just a code
> simplification (plus simplification of future code extensions).
Well, I agree with all that, but it just doesn't look simpler
to me at first look. :-) That's my main worry. I'll explain further below.
If we split this in smaller pieces, it should make second looks
easier, hopefuly.
> > That is, it feels like we could tackle these changes independently.
> > Not sure though.
>
> Probably it can although I find such patch more difficult than this one.
I'll prove you wrong. :-) I've taken your patch, and started hacking on
it, fixing a few things that didn't look exactly right, and the redundancy
in the perform's and the other flags, then, I renamed back the perform
actions to the current/old BPSTAT_WHAT_ names, and voila. I just had
to reorder the bpstat_what_main_action enum list to match the priorities.
I'd still prefer splitting the getting rid of BPSTAT_WHAT_CHECK_SHLIBS
and BPSTAT_WHAT_CHECK_JIT to its own patch, and, bpstat_what is probably
not exactly the ideal place to handle these breakpoints (it feels like
bpstop_what shouldn't have side effects), though that's no biggie and
can be fixed at any other time.
You'll notice that this bpstat_what version is even a bit more simplified
than yours, because it centralizes BPSTAT_WHAT_SINGLE handling (it's really
a property of whether there's a breakpoint at PC when we keep going,
so it could even be eliminated if we're okay with adding a
breakpoint_inserted_here call somewhere else; if it stays, it's really
a property of the breakpoint location, not the breakpoint, it seems.),
and, it starts off as BPSTAT_WHAT_KEEP_CHECKING, and so all cases that
would explicitly set no-action disappear.
Let me know what do you think.
--
Pedro Alves
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/breakpoint.c 2010-06-16 20:02:08.000000000 +0100
@@ -4115,7 +4115,7 @@ bpstat_stop_status (struct address_space
bs->stop = 0;
else
bpstat_check_breakpoint_conditions (bs, ptid);
-
+
if (bs->stop)
{
++(b->hit_count);
@@ -4187,235 +4187,133 @@ bpstat_stop_status (struct address_space
return root_bs->next;
}
-\f
-/* 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,
-
- /* We hit the longjmp_resume breakpoint. */
- long_resume,
+static void
+handle_jit_event (void)
+{
+ struct frame_info *frame;
+ struct gdbarch *gdbarch;
- /* We hit the step_resume breakpoint. */
- step_resume,
+ /* Switch terminal for any messages produced by
+ breakpoint_re_set. */
+ target_terminal_ours_for_output ();
- /* We hit the shared library event breakpoint. */
- shlib_event,
+ frame = get_current_frame ();
+ gdbarch = get_frame_arch (frame);
- /* We hit the jit event breakpoint. */
- jit_event,
+ jit_event_handler (gdbarch);
- /* This is just used to count how many enums there are. */
- class_last
- };
+ target_terminal_inferior ();
+}
- /* 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). */
+/* Prepare WHAT final decision for infrun. */
- 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
- switch (bs->breakpoint_at->owner->type)
{
- case bp_none:
- continue;
+ enum bp_loc_type loc_type = bs->breakpoint_at->loc_type;
+
+ /* If we end up proceeding, we'll need to step over this
+ breakpoint. */
+ if (loc_type == bp_loc_software_breakpoint
+ || loc_type == bp_loc_hardware_breakpoint)
+ this_action = BPSTAT_WHAT_SINGLE;
+ if (bs->breakpoint_at->owner == NULL)
+ bptype = bp_none;
+ else
+ bptype = bs->breakpoint_at->owner->type;
+ }
+
+ switch (bptype)
+ {
+ case bp_none:
+ break;
case bp_breakpoint:
case bp_hardware_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:
case bp_hardware_watchpoint:
case bp_read_watchpoint:
case bp_access_watchpoint:
+ case bp_catchpoint:
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;
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)
- {
- bs_class = step_resume;
- }
- else
- /* It is for the wrong frame. */
- bs_class = bp_nostop;
+ this_action = BPSTAT_WHAT_STEP_RESUME;
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;
break;
- case bp_catchpoint:
- if (bs->stop)
- {
- if (bs->print)
- bs_class = bp_noisy;
- else
- bs_class = bp_silent;
- }
- else
- /* There was a catchpoint, but we're not stopping.
- This requires no further action. */
- bs_class = no_effect;
+ 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)
+ /* FIXME, noisy or not? */
+ this_action = BPSTAT_WHAT_STOP_NOISY;
+ break;
+ case bp_jit_event:
+ jit_event = 1;
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;
+ /* Make sure the action is stop (silent or noisy), so
+ infrun.c pops the dummy frame. */
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;
+ /* Make sure the action is stop (silent or noisy), so
+ infrun.c pops the dummy frame. */
retval.call_dummy = STOP_STD_TERMINATE;
+ this_action = BPSTAT_WHAT_STOP_SILENT;
break;
case bp_tracepoint:
case bp_fast_tracepoint:
@@ -4424,11 +4322,37 @@ 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)
+ {
+ /* 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 ();
}
- retval.main_action = current_action;
+
+ if (jit_event)
+ {
+ handle_jit_event ();
+ }
+
return retval;
}
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h 2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/breakpoint.h 2010-06-16 20:02:08.000000000 +0100
@@ -563,7 +563,20 @@ extern bpstat bpstat_stop_status (struct
CORE_ADDR pc, ptid_t ptid);
\f
/* 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
Index: src/gdb/inferior.h
===================================================================
--- src.orig/gdb/inferior.h 2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/inferior.h 2010-06-16 20:02:07.000000000 +0100
@@ -232,6 +232,8 @@ extern char *construct_inferior_argument
/* From infrun.c */
+extern int stop_on_solib_events;
+
extern void start_remote (int from_tty);
extern void normal_stop (void);
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2010-06-16 20:02:02.000000000 +0100
+++ src/gdb/infrun.c 2010-06-16 20:02:08.000000000 +0100
@@ -294,7 +294,7 @@ static struct symbol *step_start_functio
/* 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)
@@ -4045,6 +4045,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:
@@ -4079,7 +4085,7 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
return;
case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME:
- if (debug_infrun)
+ if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
@@ -4092,16 +4098,17 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
return;
case BPSTAT_WHAT_SINGLE:
- if (debug_infrun)
+ if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_SINGLE\n");
ecs->event_thread->stepping_over_breakpoint = 1;
- /* Still need to check other stuff, at least the case
- where we are stepping and step out of the right range. */
+ /* Still need to check other stuff, at least the case where we
+ are stepping and step out of the right range. */
break;
case BPSTAT_WHAT_STOP_NOISY:
- if (debug_infrun)
+ if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
+
stop_print_frame = 1;
/* We are about to nuke the step_resume_breakpointt via the
@@ -4111,18 +4118,18 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
return;
case BPSTAT_WHAT_STOP_SILENT:
- if (debug_infrun)
+ if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
stop_print_frame = 0;
- /* We are about to nuke the step_resume_breakpoin via the
+ /* We are about to nuke the step_resume_breakpoint via the
cleanup chain, so no need to worry about it here. */
stop_stepping (ecs);
return;
case BPSTAT_WHAT_STEP_RESUME:
- if (debug_infrun)
+ if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STEP_RESUME\n");
delete_step_resume_breakpoint (ecs->event_thread);
@@ -4149,66 +4156,6 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
}
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;
}
@@ -4345,6 +4292,7 @@ infrun: not switching back to stepped th
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.
Index: src/gdb/testsuite/gdb.base/nostdlib.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/nostdlib.c 2010-06-16 20:02:07.000000000 +0100
@@ -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)
+{
+}
Index: src/gdb/testsuite/gdb.base/nostdlib.exp
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/nostdlib.exp 2010-06-16 20:02:07.000000000 +0100
@@ -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"
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-15 21:54 ` Jan Kratochvil
2010-06-16 19:13 ` Pedro Alves
@ 2010-06-16 20:40 ` Tom Tromey
2010-06-23 14:42 ` Jan Kratochvil
1 sibling, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2010-06-16 20:40 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches, Stan Shebs
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Pedro> Do you think it would be hard to split this into smaller pieces?
Jan> Yes, I will do - just later. IMO we should probably more talk
Jan> about its goal than about specifics of the implementation.
Yeah. I read this thread and I had some trouble following it.
I hope this note doesn't further confuse things.
Pedro> - It should be possible to fix PR 9436 without
Jan> We can pretend PR 9436 does not exist.
One thing I would like to understand better is the long-term design.
It seems to me that right now we have a few problems. I would like to
understand (1) whether and how your patch fixes them, and (2) what our
(by which I guess I mostly mean Pedro's :-) ideal design would look
like.
My understanding of the problems:
* We don't nicely handle multiple breakpoints at one location.
(AFAIK not touched by your change.)
* bpstat_what is difficult to understand & modify, particularly the table.
(You fixed this.)
* It is too hard to add new kinds of breakpoints. See ifunc or
next-over-throw.
(Maybe partially fixed?)
For multiple breakpoints ... to me it seems like we want to have some
way to notify *all* the breakpoints at a given spot, with some kind of
decision making coming later about how to proceed.
A case that has come up a couple times is having a way for Python to add
new invisible breakpoints, with Python code attached.
E.g., suppose you wanted to track malloc+free. You would want to put
breakpoints on these that don't interfere with stepping, but you would
also want to make sure that the Python handler is run every time the
breakpoint is hit -- even if the user also does "break malloc".
So here you'd like some sorting: have the internal breakpoint actions
trigger first, collect their responses ("continue the current
operation") and then the user breakpoints.
I suppose we'd have to look at a lot more cases to see if there is some
sensible generic approach.
Pedro> To recap, IMO, the current problem with bpstat_main_action is
Pedro> that a few of its values aren't really independent and mutually
Pedro> exclusive with the others -- BPSTAT_WHAT_CHECK_SHLIBS and
Pedro> BPSTAT_WHAT_CHECK_JIT.
Jan> What if a new breakpoint type wants to stop? What if a new
Jan> breakpoint type does not want to stop? And how they combine if
Jan> they happend together with other events? While there exists answer
Jan> for it I do not know offhand. I know offhand with my
Jan> implementation. I should post a patch introducing new breakpoint
Jan> types in both variants of the bpstat_what implementation.
Are you saying that the bpstat_main_action actually encodes several
things which should actually be independent? I think that is consistent
with what I see in your patch, I just wanted to be sure.
Pedro> - I feel that even getting rid of the table bpstat_what_main_action
Pedro> table
Jan> Yes, it has to be done.
Yes.
Tom
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-16 19:13 ` Pedro Alves
@ 2010-06-18 10:41 ` Jan Kratochvil
2010-06-18 11:42 ` Pedro Alves
2010-06-24 14:44 ` [patch 3/3] bpstat_what removal Jan Kratochvil
1 sibling, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-18 10:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
On Wed, 16 Jun 2010 21:13:30 +0200, Pedro Alves wrote:
> On Tuesday 15 June 2010 22:54:02, Jan Kratochvil wrote:
> > I noted a wish of removing bpstat_what
> > already on #gdb 2008-12-31T22:51:10Z I believe not aware of this PR that time.
> > I believe the wish lasts longer than I have expressed it on #gdb.
>
> The comment mentioning that the table is not really needed is there
> since 5.0. That's like 10 years ago. :-)
My plan was to remove the whole needless bpstat_what interface but that has
been rejected after my first post. Anyway it is offtopic now.
> > Currently the interface was created with the goal of not introducing anything
> > new. All its element have both their names and functionality exactly matching
> > the already existing (and currently required to stay in place) GDB data
> > structures. They had to copy them to comply with the separation requirements:
> > Re: [patch 3/3] bpstat_what removal
> > http://sourceware.org/ml/gdb-patches/2010-05/msg00186.html
>
> I don't see how that relates, sorry.
I do not understand offhand what does exactly mean BPSTAT_WHAT_SINGLE. While
in my proposed interface I find `this.stepping_over_breakpoint = 1;' is much
easier to understand it will probably set
`ecs->event_thread->stepping_over_breakpoint = 1;'.
But I understand a code readability improvement can be postponed into
different future patches.
> > Contrary to it what exactly does BPSTAT_WHAT_STEP_RESUME and how to combine it
> > with other events is unclear to me.
> > /* Clear step resume breakpoint, and keep checking. */
>
> Yes, there's an issue with step-resume's priority over other events.
> step-resume takes priority over all other actions, even stops for other
> breakpoints. It usually just works (breakpoint on top of step-resume
> breakpoint, for instance), because when you hit the step-resume, the step
> range _also_ ends, so we stop anyway. I can see that the (infrun.c)
> optimization around:
>
> /* We are at the start of a different line. So stop. Note that
> we don't stop if we step into the middle of a different line.
> That is said to make things like for (;;) statements work
> better. */
>
> can make us miss a breakpoint in some rare cases. Acknowledging this
> doesn't mean we can't tackle one issue at a time.
==> 1.c <==
extern int f (void);
int
main (void)
{
int i;
i = f ();
return 0;
}
==> 2.c <==
int
f (void)
{
return 1;
}
gcc -Wall -c -o 2.o 2.c; gcc -o 1 1.c 2.o -Wall -g
# 2.o has no DWARF!
gdb -nx -ex 'set breakpoint always-inserted on' -ex 'b *0x400481' -ex start -ex 'set debug infrun 1' -ex step -ex disass -ex 'p/x $pc' 1
8 i = f ();
0x000000000040047c <+8>: callq 0x40048c <f>
0x0000000000400481 <+13>: mov %eax,-0x4(%rbp)
9 return 0;
0x0000000000400484 <+16>: mov $0x0,%eax
infrun: prepare_to_wait
infrun: target_wait (-1, status) =
infrun: 1622 [process 1622],
infrun: status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x400481
infrun: BPSTAT_WHAT_STEP_RESUME
infrun: stepping inside range [0x40047c-0x400484]
infrun: resume (step=1, signal=0), trap_expected=0
infrun: prepare_to_wait
infrun: target_wait (-1, status) =
infrun: 1622 [process 1622],
infrun: status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x400481
infrun: BPSTAT_WHAT_STOP_NOISY
infrun: stop_stepping
That is it would be more correct to stop using BPSTAT_WHAT_STOP_NOISY
immediately without the last resume() call. If we stop at 0x400481 and
a breakpoint is placed there we can stop and print it. OTOH if the breakpoint
is there left placed in the inferior GDB will stop again on it anyway so it
has no user visible effect.
My patch does:
infrun: prepare_to_wait
infrun: target_wait (-1, status) =
infrun: 4037 [process 4037],
infrun: status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x400481
infrun: bp_breakpoint: print_frame pf_yes
infrun: bp_breakpoint: perform pe_stop
infrun: bp_step_resume: perform pe_check_more
infrun: bp_step_resume: bp_step_resume_on_stop yes
infrun: summary: print_frame pf_yes
infrun: summary: stop_step ss_default (ss_print_yes (stop_step 0))
infrun: summary: perform pe_stop
infrun: summary: stepping_over_breakpoint no
infrun: summary: bp_longjmp no
infrun: summary: bp_longjmp_resume no
infrun: summary: bp_step_resume_on_stop yes
infrun: stop_stepping
And therefore it does not invoke additional needless inferior resume there.
I was trying yesterday to find a user-visible reproducer (not just a `set
debug' output defect) but I have not found one soon enough and I have to move
on other work so just posting this mail. I still believe in some cases it may
be user visible or will be with the planned new gnu-ifunc, python and other
breakpoint types.
> > What if a new breakpoint type does not want to stop?
>
> BPSTAT_WHAT_SINGLE
I do not find this easy to find out.
> > And how they combine if they happend together with
> > other events?
>
> The highest priority of the above wins.
You describe your patch here. Based on it I made an ordered list:
#define kc BPSTAT_WHAT_KEEP_CHECKING
break;
#define sgl BPSTAT_WHAT_SINGLE
ecs->event_thread->stepping_over_breakpoint = 1;
break;
#define slr BPSTAT_WHAT_SET_LONGJMP_RESUME
special
ecs->event_thread->stepping_over_breakpoint = 1;
keep_going (ecs);
return;
#define clr BPSTAT_WHAT_CLEAR_LONGJMP_RESUME
special
ecs->event_thread->stop_step = 1;
stop_stepping (ecs);
return;
#define ss BPSTAT_WHAT_STOP_SILENT
stop_print_frame = 0;
stop_stepping (ecs);
return;
#define sn BPSTAT_WHAT_STOP_NOISY
stop_print_frame = 1;
stop_stepping (ecs);
return;
#define sr BPSTAT_WHAT_STEP_RESUME
special
if (special2)
ecs->event_thread->stepping_over_breakpoint = 1;
keep_going (ecs);
return;
else
break;
We do not want "highest priority of the above wins" but we want "higher
priority is a superset of actions of any lower priority". This has been
achieved in my patch according to the `set debug'-class reproducer above.
> If you're going to stop, you'll never want to set longjmp or step resume
> breakpoints, or start a step over. You're just going to stop, period.
But former BPSTAT_WHAT_STEP_RESUME and BPSTAT_WHAT_SET_LONGJMP_RESUME
(therefore those using `keep_going (ecs); return;' make a mess there as they
cancel lower priorities wanting to stop.
> (My feeling is that we should completely move the infrun-specific
> breakpoints handling to infrun (longjmp, longjmp-resume, step-resume),
> though it isn't clear _how_ is best. I hope that by eliminating
> the table first, we can think of better interfaces afterwards.
> We may even end up with something like yours, I'm not rejecting
> it upfront, to be clear.)
In general when I forget about my patch I find your solution definitely as an
improvement over current FSF GDB HEAD state. I still would like to review it
more, just hand waving it is correct in this mail.
> > > - I feel that even getting rid of the table bpstat_what_main_action
> > > table
> >
> > Yes, it has to be done.
>
> It doesn't _have_ to. There's nothing wrong with it, if you look at
> it as a state machine (and once you remove the states that should never
> have been put there in the first place).
To make it correct any superset of actions of any element of the enum set
would have to be represented by an element of that enum set. The enum set
actions would have to be closed for the operation of unification.
> I don't find it hard to read, but you do
> have to think a bit about it to understand it the first time;
Yes, I am mostly used to BPSTAT_WHAT_* now after spending many days with it.
This only proves it must be removed one day to get on par with the
contribution rate of Linux kernel.
> Well, I agree with all that, but it just doesn't look simpler
> to me at first look. :-) That's my main worry.
I believe nullifying this layer will make it easier to simplify other parts of
the code this code controls. That is to simplify code currently controlled by
`ecs->event_thread->stop_step', `stop_print_frame',
ecs->event_thread->stepping_over_breakpoint' (this one is the only one
possibly clear enough) and others. But I have not even tried to simplify this
part so I may have false predictions on the goals of my bpstat_what rework.
Primarily this future simplification goal was heavily hit by the
re-introduction of bpstat_what interface as requested after my first post.
> > Probably it can although I find such patch more difficult than this one.
>
> I'll prove you wrong. :-)
Your patch has a regression against my patch therefore I do not find it
proven. I was going to try some BPSTAT_WHAT_* patch while keeping it
regression free but when thinking about it now it probably would not be
possible anyway.
Your patch probably (I would have to verify this part first) does not have
a regression against FSF GDB HEAD and therefore it is a good step.
> You'll notice that this bpstat_what version is even a bit more simplified
> than yours, because it centralizes BPSTAT_WHAT_SINGLE handling (it's really
> a property of whether there's a breakpoint at PC when we keep going,
This violates the goal of my patch to make its reviewing easier by not
changing the behavior in any way for the cases only a single event happens.
I would prefer to remove it from this your patch.
> so it could even be eliminated if we're okay with adding a
> breakpoint_inserted_here call somewhere else; if it stays, it's really
> a property of the breakpoint location, not the breakpoint, it seems.),
This seems as a nice simplifications but it is outside of (my intended) scope
of this patch.
Thanks,
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-18 10:41 ` Jan Kratochvil
@ 2010-06-18 11:42 ` Pedro Alves
2010-06-18 14:09 ` Jan Kratochvil
0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2010-06-18 11:42 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Stan Shebs
On Friday 18 June 2010 11:41:29, Jan Kratochvil wrote:
>
> I do not understand offhand what does exactly mean BPSTAT_WHAT_SINGLE.
> While
> in my proposed interface I find `this.stepping_over_breakpoint = 1;' is much
> easier to understand it will probably set
> `ecs->event_thread->stepping_over_breakpoint = 1;'.
That change will need to be well thought out. That's a behaviour change,
and there are tricky code paths in infrun that want to rehit a breakpoint,
as the one you've shown. Let me reiterate --- I'm not against your patch;
I found it hard to review because a change like that (and others) has subtle
handle_inferior_event behaviour changes, that I'd prefer having that isolated
from the bpstat_what internals rework to not use the table. (I don't think
these behaviour changes and how they were correct and desirable were mentioned
or explained in either the patch descriptions, or in the code itself.)
> That is it would be more correct to stop using BPSTAT_WHAT_STOP_NOISY
> immediately without the last resume() call. If we stop at 0x400481 and
> a breakpoint is placed there we can stop and print it. OTOH if the breakpoint
> is there left placed in the inferior GDB will stop again on it anyway so it
> has no user visible effect.
Yeah, I realised that shortly after my last post. There are code paths
in handle_inferior_event that _want_ to resume until a breakpoint is
rehit, related to (nested) signals. Not sure they apply in this case,
I/We'll need to dig further. (another reason for wanting to have that change
separated).
> My patch does:
(...)
> But former BPSTAT_WHAT_STEP_RESUME and BPSTAT_WHAT_SET_LONGJMP_RESUME
> (therefore those using `keep_going (ecs); return;' make a mess there as they
> cancel lower priorities wanting to stop.
Well, not cancel, but postpone. I agree that this can be fixed/simplified,
and is yet another instance of "moving solib out of BPSTAT_WHAT_... makes
it easier to see what we need to do". I have trouble thinking how
you'd have a simultaneous BPSTAT_WHAT_STEP_RESUME along with an solib
or jit event, but, let's put thread event breakpoints and other kind
of event breakpoints we'll come up with, or even gdb side tracepoints
in the mix. All these breakpoints have the property that you do want to
avoid considering rehits introduced by these "spurious" resumes as separate
hits. Easier to think about if you consider tracepoints (you'd have double,
or more collects for the same tracepoint hit), and that does indeed suggest
something needs to change.
> In general when I forget about my patch I find your solution definitely as an
> improvement over current FSF GDB HEAD state. I still would like to review it
> more, just hand waving it is correct in this mail.
To be clear, my (well, our) patch was an attempt at splitting your patch
in two (well, three, the solib-event patch split didn't happen
yet) --- one that gets the table out of the way, and another that reworks
the bpstat_what interface. The latter would conceivably be somewhat like
the interdiff between your and my patch.
> > > Probably it can although I find such patch more difficult than this one.
> >
> > I'll prove you wrong. :-)
>
> Your patch has a regression against my patch therefore I do not find it
> proven.
?? What I was "proving" was that splitting from the larger patch a patch that
just removes the bpstat table without changing gdb's behaviour in any way,
was both possible, and actually simple, not that is was superior to your
patch. The goal was getting the table our of the way, which is supposedly
a non-behaviour (almost mechanical actually) change, so we can concentrate
on the interface and infrun issues.
> > You'll notice that this bpstat_what version is even a bit more simplified
> > than yours, because it centralizes BPSTAT_WHAT_SINGLE handling (it's really
> > a property of whether there's a breakpoint at PC when we keep going,
>
> This violates the goal of my patch to make its reviewing easier by not
> changing the behavior in any way for the cases only a single event happens.
(while making it hard to review because it changes the behavior when
multiple breakpoints happen :-) )
> I would prefer to remove it from this your patch.
That was supposedly not changing gdb's behaviour in _any_
way, either when single or multiple simultaneous events happen,
but I'll agree to split that bit out. Baby steps!
--
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-18 11:42 ` Pedro Alves
@ 2010-06-18 14:09 ` Jan Kratochvil
2010-06-18 14:35 ` Pedro Alves
0 siblings, 1 reply; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-18 14:09 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
On Fri, 18 Jun 2010 13:42:30 +0200, Pedro Alves wrote:
> There are code paths in handle_inferior_event that _want_ to resume until
> a breakpoint is rehit, related to (nested) signals. Not sure they apply in
> this case, I/We'll need to dig further. (another reason for wanting to have
> that change separated).
That is very surprising to me, so far I have considered such re-hits as
clearly a bug in handle_inferior_event, not considering other code may expect
it. I agree now my patch went too far and it should be split (like you did).
> > But former BPSTAT_WHAT_STEP_RESUME and BPSTAT_WHAT_SET_LONGJMP_RESUME
> > (therefore those using `keep_going (ecs); return;' make a mess there as they
> > cancel lower priorities wanting to stop.
>
> Well, not cancel, but postpone.
Without of possibility getting some event lost? (watchpoints come to my mind)
Not important now as I see there is an approved plan to remove it anyway.
> I have trouble thinking how you'd have a simultaneous
> BPSTAT_WHAT_STEP_RESUME along with an solib or jit event, but,
There is missing and planned "new invisible breakpoints, with Python code
attached" stated in:
Tom Tromey: Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00376.html
which can happen for (after re-hits start to be prevented):
call_func_without_debuginfo ();
line_with_python_invisible_breakpoint;
> let's put thread event breakpoints and other kind of event breakpoints we'll
> come up with, or even gdb side tracepoints in the mix.
I assume you mean the case I have described now above.
> All these breakpoints have the property that you do want to
> avoid considering rehits introduced by these "spurious" resumes as separate
> hits. Easier to think about if you consider tracepoints (you'd have double,
> or more collects for the same tracepoint hit), and that does indeed suggest
> something needs to change.
Great there is approval of this intention of re-hits removal.
> The goal was getting the table our of the way, which is supposedly
> a non-behaviour (almost mechanical actually) change, so we can concentrate
> on the interface and infrun issues.
OK, true the table itself is currently the most obvious pain.
> > This violates the goal of my patch to make its reviewing easier by not
> > changing the behavior in any way for the cases only a single event happens.
>
> (while making it hard to review because it changes the behavior when
> multiple breakpoints happen :-) )
... as the (IMO) "obviously broken behavior" is expected by the current code.
Thanks,
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-18 14:09 ` Jan Kratochvil
@ 2010-06-18 14:35 ` Pedro Alves
0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2010-06-18 14:35 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Stan Shebs
On Friday 18 June 2010 15:09:36, Jan Kratochvil wrote:
> There is missing and planned "new invisible breakpoints, with Python code
> attached" stated in:
> Tom Tromey: Re: [patch 3/3] bpstat_what removal [rediff]
> http://sourceware.org/ml/gdb-patches/2010-06/msg00376.html
>
> which can happen for (after re-hits start to be prevented):
> call_func_without_debuginfo ();
> line_with_python_invisible_breakpoint;
>
>
> > let's put thread event breakpoints and other kind of event breakpoints we'll
> > come up with, or even gdb side tracepoints in the mix.
>
> I assume you mean the case I have described now above.
Yes, those too. By gdb side tracepoints, I mean tracepoints that are
all handled by gdb core, with regular software breakpoint locations,
instead of relying on the target backend to implement them. Conceptually,
I think those python breakpoints, gdb side tracepoints, solib-, jit- and
thread- event breakpoints are all pretty much the same in infrun's perpective.
When they're hit, some callback is called, which does its own thing, and
returns one of "I cause a stop", or "I don't cause a stop, step over me and
let the inferior carry on doing what it was doing",
or "I don't cause a stop, and I'm now gone, so no need to step over me.".
Those python breakpoints sounds a lot like tracepoints to me, and goes
in line with what we've discussed before that "tracepoint" as more of an
attribute feeling an breakpoint-kind feeling.
Actually user breakpoints are pretty much the same too, if you consider that
the callback in question something like bpstat_check_breakpoint_conditions.
Anyways, there's clearly a distinction between infrun specific breakpoints,
and all other kinds of breakpoints.
--
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal [rediff]
2010-06-16 20:40 ` [patch 3/3] bpstat_what removal [rediff] Tom Tromey
@ 2010-06-23 14:42 ` Jan Kratochvil
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-23 14:42 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches, Stan Shebs
On Wed, 16 Jun 2010 22:39:44 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
[...]
> One thing I would like to understand better is the long-term design.
> It seems to me that right now we have a few problems. I would like to
> understand (1) whether and how your patch fixes them, and (2) what our
> (by which I guess I mostly mean Pedro's :-) ideal design would look
> like.
>
> My understanding of the problems:
>
> * We don't nicely handle multiple breakpoints at one location.
> (AFAIK not touched by your change.)
It gets fixed AFAIK. Or at least improved, not sure if the fix is complete.
It needs also a patch of this series:
Re: [patch 2/3] bpstat_print should consider all candidates
http://sourceware.org/ml/gdb-patches/2010-05/msg00368.html
As explained by Pedro currently some events are ignored but they get re-hit on
a vain attempt to resume at a breakpoint location and all the events get
executed properly. Described by Pedro's "Well, not cancel, but postpone." ...:
Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00404.html
> * bpstat_what is difficult to understand & modify, particularly the table.
> (You fixed this.)
The table part gets removed even in the Pedro's generally agreed
simplification:
Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00372.html
Other code readability issues are both not agreed upon and they are strongly
bound to the "re-hit" bug-to-bug compatibility described above.
> * It is too hard to add new kinds of breakpoints. See ifunc or
> next-over-throw.
> (Maybe partially fixed?)
It should become generally easy now even with the Pedro's generally agreed
simplification:
Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00372.html
The is the major change it has been design-approved the actions can get
executed in breakpoint.c instead of infrun.c which simplified it a lot.
> For multiple breakpoints ... to me it seems like we want to have some
> way to notify *all* the breakpoints at a given spot, with some kind of
> decision making coming later about how to proceed.
>
> A case that has come up a couple times is having a way for Python to add
> new invisible breakpoints, with Python code attached.
As the action can get executed from breakpoint.c now it can accordingly set
the bpstat_what() result making the "new invisible breakpoints" simple IMO.
> Pedro> To recap, IMO, the current problem with bpstat_main_action is
> Pedro> that a few of its values aren't really independent and mutually
> Pedro> exclusive with the others -- BPSTAT_WHAT_CHECK_SHLIBS and
> Pedro> BPSTAT_WHAT_CHECK_JIT.
>
> Jan> What if a new breakpoint type wants to stop? What if a new
> Jan> breakpoint type does not want to stop? And how they combine if
> Jan> they happend together with other events? While there exists answer
> Jan> for it I do not know offhand. I know offhand with my
> Jan> implementation. I should post a patch introducing new breakpoint
> Jan> types in both variants of the bpstat_what implementation.
>
> Are you saying that the bpstat_main_action actually encodes several
> things which should actually be independent? I think that is consistent
> with what I see in your patch, I just wanted to be sure.
There are three levels of independence:
No independence - current FSF GDB - everything is encoded as a single
decision. This is wrong, some functionality gets lost - PR 9436.
Partial independence - Pedro's simplification
Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00372.html
which makes BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT independent
thus no longer breaking PR 9436. But it still hides some information as shown
in my `set debug infrun' reproducer in
Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00403.html
unfortunately current GDB code is (reportedly - I do not know a reproducer)
relies on this "re-hit" bug as described by Pedro in:
Re: [patch 3/3] bpstat_what removal [rediff]
http://sourceware.org/ml/gdb-patches/2010-06/msg00404.html
Full independence - my original proposals
[patch 3/3] bpstat_what removal
http://sourceware.org/ml/gdb-patches/2010-05/msg00050.html
and with an interface
Re: [patch 3/3] bpstat_what removal
http://sourceware.org/ml/gdb-patches/2010-05/msg00370.html
which have been generally approved as a future goal but currently they
(a) seem to be a too large change to be approved in a single step and
(b) it may break some bug-to-bug compatibility relying on the "re-hit"s.
Thanks,
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-06-16 19:13 ` Pedro Alves
2010-06-18 10:41 ` Jan Kratochvil
@ 2010-06-24 14:44 ` Jan Kratochvil
2010-06-24 14:48 ` [patch 3.1/3] bpstat_what removal - addon gdb_assert Jan Kratochvil
2010-06-24 15:03 ` [patch 3/3] bpstat_what removal Pedro Alves
1 sibling, 2 replies; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-24 14:44 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
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;
}
-\f
-/* 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);
\f
/* 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"
^ permalink raw reply [flat|nested] 21+ messages in thread
* [patch 3.1/3] bpstat_what removal - addon gdb_assert
2010-06-24 14:44 ` [patch 3/3] bpstat_what removal Jan Kratochvil
@ 2010-06-24 14:48 ` Jan Kratochvil
2010-06-24 15:03 ` [patch 3/3] bpstat_what removal Pedro Alves
1 sibling, 0 replies; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-24 14:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
Hi Pedro,
On Thu, 24 Jun 2010 16:44:23 +0200, Jan Kratochvil wrote:
> attaching follow-up on your patch still using BPSTAT_WHAT_* symbols.
this is a gdb_assert followup with the goal of making the code both more clear
and more safe against future changes. I do not mind much about this patch.
No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.
Thanks,
Jan
2010-06-24 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (bpstat_what): Pre-set this_action to -1. Set
this_action to BPSTAT_WHAT_KEEP_CHECKING in cases it has been left
unchanged. New gdb_assert.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4234,7 +4234,7 @@ bpstat_what (bpstat bs)
{
/* 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 bpstat_what_main_action this_action = -1;
enum bptype bptype;
if (bs->breakpoint_at == NULL)
@@ -4251,6 +4251,7 @@ bpstat_what (bpstat bs)
switch (bptype)
{
case bp_none:
+ this_action = BPSTAT_WHAT_KEEP_CHECKING;
break;
case bp_breakpoint:
case bp_hardware_breakpoint:
@@ -4281,6 +4282,7 @@ bpstat_what (bpstat bs)
{
/* There was a watchpoint, but we're not stopping.
This requires no further action. */
+ this_action = BPSTAT_WHAT_KEEP_CHECKING;
}
break;
case bp_longjmp:
@@ -4317,6 +4319,7 @@ bpstat_what (bpstat bs)
{
/* There was a catchpoint, but we're not stopping.
This requires no further action. */
+ this_action = BPSTAT_WHAT_KEEP_CHECKING;
}
break;
case bp_shlib_event:
@@ -4359,6 +4362,7 @@ bpstat_what (bpstat bs)
_("bpstat_what: unhandled bptype %d"), (int) bptype);
}
+ gdb_assert (this_action != -1);
retval.main_action = max (retval.main_action, this_action);
}
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-06-24 14:44 ` [patch 3/3] bpstat_what removal Jan Kratochvil
2010-06-24 14:48 ` [patch 3.1/3] bpstat_what removal - addon gdb_assert Jan Kratochvil
@ 2010-06-24 15:03 ` Pedro Alves
2010-06-24 15:21 ` Jan Kratochvil
1 sibling, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2010-06-24 15:03 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Stan Shebs
On Thursday 24 June 2010 15:44:23, Jan Kratochvil wrote:
> 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.
Thanks! This is okay. Since this is still changing the shlib event
logic, remember to put the PR number in the changelog.
> No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.
--
Pedro Alves
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 3/3] bpstat_what removal
2010-06-24 15:03 ` [patch 3/3] bpstat_what removal Pedro Alves
@ 2010-06-24 15:21 ` Jan Kratochvil
0 siblings, 0 replies; 21+ messages in thread
From: Jan Kratochvil @ 2010-06-24 15:21 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Stan Shebs
On Thu, 24 Jun 2010 17:03:43 +0200, Pedro Alves wrote:
> Thanks! This is okay.
Checked-in:
http://sourceware.org/ml/gdb-cvs/2010-06/msg00160.html
> Since this is still changing the shlib event
> logic, remember to put the PR number in the changelog.
I would forget, put it there.
Thanks,
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2010-06-24 15:21 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-03 20:02 [patch 3/3] bpstat_what removal Jan Kratochvil
2010-05-04 14:10 ` Stan Shebs
2010-05-07 16:17 ` Jan Kratochvil
2010-05-07 16:26 ` Pedro Alves
2010-05-07 17:02 ` Jan Kratochvil
2010-05-07 17:17 ` Pedro Alves
2010-05-17 21:46 ` Jan Kratochvil
2010-06-12 17:02 ` [patch 3/3] bpstat_what removal [rediff] Jan Kratochvil
2010-06-15 15:08 ` Pedro Alves
2010-06-15 21:54 ` Jan Kratochvil
2010-06-16 19:13 ` Pedro Alves
2010-06-18 10:41 ` Jan Kratochvil
2010-06-18 11:42 ` Pedro Alves
2010-06-18 14:09 ` Jan Kratochvil
2010-06-18 14:35 ` Pedro Alves
2010-06-24 14:44 ` [patch 3/3] bpstat_what removal Jan Kratochvil
2010-06-24 14:48 ` [patch 3.1/3] bpstat_what removal - addon gdb_assert Jan Kratochvil
2010-06-24 15:03 ` [patch 3/3] bpstat_what removal Pedro Alves
2010-06-24 15:21 ` Jan Kratochvil
2010-06-16 20:40 ` [patch 3/3] bpstat_what removal [rediff] Tom Tromey
2010-06-23 14:42 ` Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).