This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFC]: pending break support
- From: Daniel Jacobowitz <drow at mvista dot com>
- To: "J. Johnston" <jjohnstn at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com, Eli Zaretskii <eliz at elta dot co dot il>
- Date: Thu, 4 Dec 2003 23:56:32 -0500
- Subject: Re: [RFC]: pending break support
- References: <3FCD3DC5.8000308@redhat.com>
I was the last person with my fingers in this code, so I'll review
this.
On Tue, Dec 02, 2003 at 08:35:01PM -0500, J. Johnston wrote:
> This is the next stage for the pending breakpoint support. I have put
> together a real patch run through the testsuite with a new test case to
> verify the pending breakpoint support.
>
> I consider the following default behaviors to be controlled at a later date
> with settings.
>
> 1. For now, once a pending breakpoint is resolved, it is deleted and
> replaced
> with whatever breakpoint(s) would naturally occur at that point in
> time for
> the given input. In most cases, it will be first found location after
> a
> shared library load. It is possible for the symbol to exist in
> multiple
> shared libraries if the pending breakpoint is disabled, then enabled
> again
> after multiple shared libraries have been loaded. Whatever the current
> breakpoint behavior for such a case will be followed (i.e. the
> breakpoint
> is simply issued at a later time).
This is fine. What to do with other shared objects containing the
symbol should be orthogonal to pending-ness. I.e. if you want both
libfoo.so:func and libbar.so:func, you probably want app:func too if
there is one. This is a separate problem for the future.
> 2. For now, a pending breakpoint queries the user as to what they want to
> do.
This is also fine - another interface issue for the future.
> Break condition and command support exists mostly as normal, however,
> parsing of conditions does not occur until the breakpoint location is
> resolved. I have tried to make sure that the original state existing at
> the point of the break command is restored when the breakpoint is resolved
> (e.g. language, input_radix).
This sounds like a great solution for the moment, while we're only
generating one breakpoint. This moment may last quite a while.
> To avoid a problem with restarting programs where pending breakpoints are
> yet to be resolved, I have set up a "silent_if_not_found" flag for
> decode_line_1. It simply causes an exception to be thrown rather than
> calling error() if the source file or function cannot be found. This flag
> is turned on by the code that checks for pending breakpoint resolution.
> Without this code, you would see a score of error messages for every failed
> attempt at resolving a pending breakpoint when shared libraries are loaded.
You'll need to run this past Elena as the linespec maintainer. It
looks plausible (though ugly) to me. It would be nice if we could
easily suppress all error messages using one of the catch* functions
but it doesn't look like we can at the moment.
> Please let me know if it ok to commit?
Specific comments on the patch below. The general overview is OK but
details need work. Also, I think this patch is too big to go in as one
piece. Could you do it as at least three:
- linespec changes
- Convert current enable_state checks to a function (see below)
- the rest
? It won't be much work to separate them, and it'll be easier to
proofread for everybody. Plus the earlier ones can go in while we work
on details of the last one.
> 2003-12-02 Jeff Johnston <jjohnstn@redhat.com>
>
> * breakpoint.c (condition_command): Only parse the expression if not
> a
> pending breakpoint.
> (insert_bp_location): Check for not pending when checking for an
> enabled breakpoint.
> (insert_breakpoints): Ditto.
> (remove_breakpoint): Ditto.
> (breakpoint_here_p): Ditto.
> (breakpoint_thread_match): Ditto.
> (bpstat_stop_status): Ditto.
> (bpstat_should_stop): Ditto.
> (bpstat_have_active_hw_watchpoints): Ditto.
> (check_duplicates): Ditto.
> (disable_breakpoints_in_shlibs): Ditto.
> (hw_watchpoint_used_count): Ditto.
> (delete_breakpoint): Ditto.
> (print_one_breakpoint): Add pending support.
> (mention): Ditto.
> (set_raw_breakpoint): Initialize pending flag to 0.
> (captured_parse_breakpoint_args): New structure.
> (resolve_pending_breakpoint): New function.
> (do_captured_parse_breakpoint): Ditto.
> (re_enable_breakpoints_in_shlibs): Try and resolve any existing
> pending breakpoints that are enabled.
> (solib_load_unload_1): Add additional parameter to decode_line_1
> call.
> (break_at_finish_command_1): Ditto.
> (until_break_command): Ditto.
> (handle_gnu_v3_exceptions): Ditto.
> (decode_line_spec_1): Ditto.
> (breakpoint_re_set_one): Ditto. Also check for pending flag.
> (parse_breakpoint_sals): Add new silent_flag parameter and pass this
> parameter on to decode_line_1.
> (do_captured_breakpoint): Account for new parameter when calling
> parse_breakpoint_sals.
> (break_command_1): Indirectly call parse_breakpoint_sals via
> catch_errors. Support pending breakpoint if file or function not
> found.
> (do_enable_breakpoint): Check for enabling a disabled pending
> breakpoint
> in which case try and resolve the pending breakpoint.
> * breakpoint.h (struct breakpoint): Add new from_tty, flag, and
> pending
> fields.
> * linespec.c (symtab_from_filename): Add new silent_if_not_found
> parameter. Do not call error() for missing file.
> (decode_variable): Ditto. Do not call error() for missing function.
> (decode_line_1): Add new silent_if_not_found parameter. Pass new
> parameter to symtab_from_filename() and decode_variable().
> * linespec.h (decode_line_1): Add new silent_if_not_found parameter.
> * symtab.c (decode_line_spec): Change call to decode_line_1 to add
> default 0 last parameter.
> * tracepoint.c (trace_command, scope_info): Ditto.
> * cli/cli-cmds.c (edit_command, list_command): Change call to
> decode_line_1 to add default 0 last parameter.
>
> doc/ChangeLog:
>
> 2003-12-03 Jeff Johnston <jjohnstn@redhat.com>
>
> * gdb.texinfo: Add documentation for pending breakpoint support.
>
> testsuite/ChangeLog:
>
> * lib/gdb.exp (gdb_breakpoint): Add support for pending breakpoint
> query.
> * gdb.base/pending.exp: New test.
> * gdb.base/pending.c: New file.
> * gdb.base/pendshr1.c: Ditto.
> * gdb.base/langs.exp: Fix to ignore pending breakpoint message.
> * gdb.base/shlib-call.exp: Use gdb_breakpoint for shr1 breakpoint.
>
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 breakpoint.c
> --- breakpoint.c 17 Nov 2003 00:55:49 -0000 1.145
> +++ breakpoint.c 3 Dec 2003 01:21:49 -0000
> @@ -119,6 +119,8 @@ static void condition_command (char *, i
>
> static int get_number_trailer (char **, int);
>
> +static int do_captured_parse_breakpoint (void *);
> +
> void set_breakpoint_count (int);
>
> typedef enum
> @@ -563,9 +565,12 @@ condition_command (char *arg, int from_t
> /* I don't know if it matters whether this is the string the user
> typed in or the decompiled expression. */
> b->cond_string = savestring (arg, strlen (arg));
> - b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
> - if (*arg)
> - error ("Junk at end of expression");
> + if (!b->pending)
> + {
> + b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
> + if (*arg)
> + error ("Junk at end of expression");
> + }
> }
> breakpoints_changed ();
> breakpoint_modify_event (b->number);
> @@ -757,7 +762,7 @@ insert_bp_location (struct bp_location *
>
> /* Permanent breakpoints cannot be inserted or removed. Disabled
> breakpoints should not be inserted. */
> - if (bpt->owner->enable_state != bp_enabled)
> + if (bpt->owner->enable_state != bp_enabled || bpt->owner->pending)
> return 0;
>
> if (bpt->inserted || bpt->duplicate)
Since you need to touch most of these checks anyway, how about a
functon for the test? I think it should be:
static int
breakpoint_enabled (struct breakpoint *b)
{
return b->enable_state == bp_enabled && !b->pending;
}
This one becomes:
if (!breakpoint_enabled (bpt->owner))
It's not clear what to do with permanent breakpoints (I don't think
that should be an enable state, long term!) so I chose the version with
minimal textual changes. If you'll do this in a separate patch you can
do it without the && !b->pending, and add that in the third patch,
making this a standalone cleanup.
> @@ -1107,7 +1112,7 @@ insert_breakpoints (void)
> {
> /* Permanent breakpoints cannot be inserted or removed. Disabled
> breakpoints should not be inserted. */
> - if (b->owner->enable_state != bp_enabled)
> + if (b->owner->enable_state != bp_enabled || b->owner->pending)
> continue;
>
> /* FIXME drow/2003-10-07: This code should be pushed elsewhere when
> @@ -1458,6 +1463,7 @@ remove_breakpoint (struct bp_location *b
> }
> else if (b->loc_type == bp_loc_hardware_watchpoint
> && b->owner->enable_state == bp_enabled
> + && !b->owner->pending
> && !b->duplicate)
> {
> struct value *v;
> @@ -1514,6 +1520,7 @@ remove_breakpoint (struct bp_location *b
> b->owner->type == bp_catch_vfork ||
> b->owner->type == bp_catch_exec)
> && b->owner->enable_state == bp_enabled
> + && !b->owner->pending
> && !b->duplicate)
> {
> val = -1;
> @@ -1539,6 +1546,7 @@ remove_breakpoint (struct bp_location *b
> else if ((b->owner->type == bp_catch_catch ||
> b->owner->type == bp_catch_throw)
> && b->owner->enable_state == bp_enabled
> + && !b->owner->pending
> && !b->duplicate)
> {
>
> @@ -1550,6 +1558,7 @@ remove_breakpoint (struct bp_location *b
> else if (ep_is_exception_catchpoint (b->owner)
> && b->inserted /* sometimes previous insert doesn't happen */
> && b->owner->enable_state == bp_enabled
> + && !b->owner->pending
> && !b->duplicate)
> {
>
> @@ -1675,7 +1684,8 @@ breakpoint_here_p (CORE_ADDR pc)
> && bpt->loc_type != bp_loc_hardware_breakpoint)
> continue;
>
> - if ((bpt->owner->enable_state == bp_enabled
> + if (((bpt->owner->enable_state == bp_enabled
> + && !bpt->owner->pending)
> || bpt->owner->enable_state == bp_permanent)
> && bpt->address == pc) /* bp is enabled and matches pc */
> {
> @@ -1772,7 +1782,8 @@ breakpoint_thread_match (CORE_ADDR pc, p
> && bpt->loc_type != bp_loc_hardware_breakpoint)
> continue;
>
> - if ((bpt->owner->enable_state == bp_enabled
> + if (((bpt->owner->enable_state == bp_enabled
> + && !bpt->owner->pending)
> || bpt->owner->enable_state == bp_permanent)
> && bpt->address == pc
> && (bpt->owner->thread == -1 || bpt->owner->thread == thread))
> @@ -2576,7 +2587,8 @@ bpstat_stop_status (CORE_ADDR *pc, int n
> {
> if (b->enable_state == bp_disabled
> || b->enable_state == bp_shlib_disabled
> - || b->enable_state == bp_call_disabled)
> + || b->enable_state == bp_call_disabled
> + || b->pending)
> continue;
>
> if (b->type != bp_watchpoint
This would be
if (!breakpoint_enabled (b) && !b->enable_state == bp_permanent)
continue;
since there are only five enable states and that's what it's really
asking for.
> @@ -3179,7 +3191,7 @@ bpstat_should_step (void)
> {
> struct breakpoint *b;
> ALL_BREAKPOINTS (b)
> - if (b->enable_state == bp_enabled && b->type == bp_watchpoint)
> + if (b->enable_state == bp_enabled && !b->pending && b->type == bp_watchpoint)
> return 1;
> return 0;
> }
> @@ -3190,7 +3202,7 @@ bpstat_have_active_hw_watchpoints (void)
> {
> struct bp_location *bpt;
> ALL_BP_LOCATIONS (bpt)
> - if ((bpt->owner->enable_state == bp_enabled)
> + if ((bpt->owner->enable_state == bp_enabled && !bpt->owner->pending)
> && bpt->inserted
> && bpt->loc_type == bp_loc_hardware_watchpoint)
> return 1;
> @@ -3458,7 +3470,15 @@ print_one_breakpoint (struct breakpoint
> if (addressprint)
> {
> annotate_field (4);
> - ui_out_field_core_addr (uiout, "addr", b->loc->address);
> + if (b->pending)
> + {
> + if (TARGET_ADDR_BIT <= 32)
> + ui_out_field_string (uiout, "addr", "<PENDING> ");
> + else
> + ui_out_field_string (uiout, "addr", "<PENDING> ");
> + }
> + else
> + ui_out_field_core_addr (uiout, "addr", b->loc->address);
> }
> annotate_field (5);
> *last_addr = b->loc->address;
I'm curious what effect this will have on MI consumers. Also, the
resulting MI output will be somewhat gruesome with the embedded spaces.
Maybe:
ui_out_field_string (uiout, "addr", "<PENDING>");
ui_out_text (uiout, "addr", " ");
but I'm not sure that works without testing it.
The other possible alternative is not outputting addr at all for
MI-like uiouts. May be better to do this (more flexible).
> @@ -3477,6 +3497,10 @@ print_one_breakpoint (struct breakpoint
> ui_out_text (uiout, ":");
> ui_out_field_int (uiout, "line", b->line_number);
> }
> + else if (b->pending)
> + {
> + ui_out_field_string (uiout, "pending", b->addr_string);
> + }
> else
> {
> print_address_symbolic (b->loc->address, stb->stream, demangle, "");
Same questions as above.
> @@ -3513,7 +3537,15 @@ print_one_breakpoint (struct breakpoint
> ui_out_field_stream (uiout, "cond", stb);
> ui_out_text (uiout, "\n");
> }
> -
> +
> + if (b->pending && b->cond_string)
> + {
> + annotate_field (7);
> + ui_out_text (uiout, "\tpending stop only if ");
> + ui_out_field_string (uiout, "cond", b->cond_string);
> + ui_out_text (uiout, "\n");
> + }
> +
> if (b->thread != -1)
> {
> /* FIXME should make an annotation for this */
That seems redundant. We've already said that the breakpoint is
pending. Why not have:
else if (b->pending && b->cond_string)
{
annotate_field (7);
ui_out_text (uiout, "\tstop only if ");
ui_out_field_string (uiout, "cond", b->cond_string);
ui_out_text (uiout, "\n");
}
> @@ -3744,14 +3776,14 @@ describe_other_breakpoints (CORE_ADDR pc
>
> ALL_BREAKPOINTS (b)
> if (b->loc->address == pc) /* address match / overlay match */
> - if (!overlay_debugging || b->loc->section == section)
> + if (!b->pending && (!overlay_debugging || b->loc->section == section))
> others++;
Sure, I guess this is OK. That use of b->loc->address is broken already
- many things do not use the address field, you have to check the
breakpoint type first. But it's mostly harmless and will be cleaned up
later.
> if (others > 0)
> {
> printf_filtered ("Note: breakpoint%s ", (others > 1) ? "s" : "");
> ALL_BREAKPOINTS (b)
> if (b->loc->address == pc) /* address match / overlay match */
> - if (!overlay_debugging || b->loc->section == section)
> + if (!b->pending && (!overlay_debugging || b->loc->section == section))
> {
> others--;
> printf_filtered ("%d%s%s ",
> @@ -3840,6 +3872,7 @@ check_duplicates (struct breakpoint *bpt
> ALL_BP_LOCATIONS (b)
> if (b->owner->enable_state != bp_disabled
> && b->owner->enable_state != bp_shlib_disabled
> + && !b->owner->pending
> && b->owner->enable_state != bp_call_disabled
> && b->address == address /* address / overlay match */
> && (!overlay_debugging || b->section == section)
> @@ -3874,6 +3907,7 @@ check_duplicates (struct breakpoint *bpt
> {
> if (b->owner->enable_state != bp_disabled
> && b->owner->enable_state != bp_shlib_disabled
> + && !b->owner->pending
> && b->owner->enable_state != bp_call_disabled
> && b->address == address /* address / overlay match */
> && (!overlay_debugging || b->section == section)
> @@ -4050,6 +4084,7 @@ set_raw_breakpoint (struct symtab_and_li
> b->forked_inferior_pid = 0;
> b->exec_pathname = NULL;
> b->ops = NULL;
> + b->pending = 0;
>
> /* Add this breakpoint to the end of the chain
> so that a list of breakpoints will come out in order
> @@ -4269,6 +4304,7 @@ disable_breakpoints_in_shlibs (int silen
> if (((b->type == bp_breakpoint) ||
> (b->type == bp_hardware_breakpoint)) &&
> b->enable_state == bp_enabled &&
> + !b->pending &&
> !b->loc->duplicate &&
> PC_SOLIB (b->loc->address))
> {
> @@ -4288,23 +4324,123 @@ disable_breakpoints_in_shlibs (int silen
> }
> }
>
> +struct captured_parse_breakpoint_args
> + {
> + char **arg_p;
> + struct symtabs_and_lines *sals_p;
> + char ***addr_string_p;
> + int silent_flag;
> + };
A description of fields would be nice.
> +
> +/* Try and resolve a pending breakpoint. */
> +static struct breakpoint *
> +resolve_pending_breakpoint (struct breakpoint *b)
> +{
> + /* Try and reparse the breakpoint in case the shared library
> + is now loaded. */
> + struct symtabs_and_lines sals;
> + struct symtab_and_line pending_sal;
> + /* Pointers in arg to the start, and one past the end, of the
> + condition. */
Huh? I've got no idea what this comment refers to.
> + char **cond_string = (char **) NULL;
> + char *copy_arg = b->addr_string;
> + char **addr_string;
> + struct captured_parse_breakpoint_args parse_args;
> + int rc;
> + struct ui_file *old_gdb_stderr;
> +
> + sals.sals = NULL;
> + sals.nelts = 0;
> + addr_string = NULL;
> +
> + parse_args.arg_p = ©_arg;
> + parse_args.sals_p = &sals;
> + parse_args.addr_string_p = &addr_string;
> + parse_args.silent_flag = 1;
> +
> + old_gdb_stderr = gdb_stderr;
> + gdb_stderr = ui_file_new ();
> +
> + rc = catch_errors (do_captured_parse_breakpoint, &parse_args,
> + NULL, RETURN_MASK_ALL);
> +
> + ui_file_delete (gdb_stderr);
> + gdb_stderr = old_gdb_stderr;
> +
> + if (rc == GDB_RC_OK)
> + {
> + enum language old_language = current_language->la_language;
> + int old_input_radix = input_radix;
> + char *arg;
> + struct breakpoint *b1;
> +
> + printf_filtered ("Pending breakpoint \"%s\" resolved\n", b->addr_string);
> +
> + /* Set language, input-radix, then reissue breakpoint command. Following the
> + command, restore the language and input-radix. */
> + set_language (b->language);
> + input_radix = b->input_radix;
This is gross, but acceptable. However, you need cleanups. Anywhere
in GDB that you massage evil global state to make a function call,
you'll need a cleanup chain to restore the state; if the breakpoint
fails to insert the error() call below will leave the language
incorrect.
> + break_command_1 (b->addr_string, b->flag, b->from_tty);
> + b1 = breakpoint_chain;
> + while (b1->next)
> + b1 = b1->next;
> + /* If there is condition specified, it should be copied over. */
> + if (b->cond_string)
> + {
> + arg = b->cond_string;
> + b1->cond_string = savestring (arg, strlen (arg));
> + b1->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
> + if (*arg)
> + error ("Junk at end of expression");
> + }
> + /* If there are commands associated with the breakpoint, they should be copied too. */
> + if (b->commands)
> + {
> + b1->commands = copy_command_lines (b->commands);
> + }
This, on the other hand, is not OK. First of all you're wasting work -
you parse the breakpoint location twice. Then you go grubbing around
in the breakpoint chain looking for it, which assumes it will be added
at the end of the chain - it will, but that sort of detail shouldn't be
exposed. And break_command_1 can currently create multiple
breakpoints, so you'll get that wrong too.
decode_line_1 can end up prompting the user; you should have a testcase
for that and make sure it does something sane. Easiest way is probably
to put it in gdb.cp using an overloaded function. All set breakpoints
should get the condition and commands.
Can you arrange for most of this function to happen inside
break_command_1, possibly by giving it a pending breakpoint as an
optional argument? Other ideas?
> +
> + set_language (old_language);
> + input_radix = old_input_radix;
> + return b1; /* Pending breakpoint resolved. */
> + }
> +
> + /* Otherwise, we didn't successfully resolve pending breakpoint. */
> + return NULL;
> +}
> +
> /* Try to reenable any breakpoints in shared libraries. */
> void
> re_enable_breakpoints_in_shlibs (void)
> {
> struct breakpoint *b;
> + struct breakpoint *del_b = NULL;
>
> ALL_BREAKPOINTS (b)
> + {
> + if (del_b)
> + {
> + delete_breakpoint (del_b);
> + del_b = NULL;
> + }
No, use ALL_BREAKPOINTS_SAFE and you can get rid of del_b.
> if (b->enable_state == bp_shlib_disabled)
> - {
> - char buf[1], *lib;
> + {
> + char buf[1], *lib;
> +
> + /* Do not reenable the breakpoint if the shared library
> + is still not mapped in. */
> + lib = PC_SOLIB (b->loc->address);
> + if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
> + b->enable_state = bp_enabled;
> + }
> + else if (b->pending && (b->enable_state == bp_enabled))
> + {
> + if (resolve_pending_breakpoint (b) != NULL)
> + del_b = b; /* Mark pending breakpoint for deletion. */
> + }
> + }
>
> - /* Do not reenable the breakpoint if the shared library
> - is still not mapped in. */
> - lib = PC_SOLIB (b->loc->address);
> - if (lib != NULL && target_read_memory (b->loc->address, buf, 1) == 0)
> - b->enable_state = bp_enabled;
> - }
> + if (del_b)
> + delete_breakpoint (del_b);
> }
>
> #endif
> @@ -4323,7 +4459,7 @@ solib_load_unload_1 (char *hookname, int
> int thread = -1; /* All threads. */
>
> /* Set a breakpoint on the specified hook. */
> - sals = decode_line_1 (&hookname, 1, (struct symtab *) NULL, 0, &canonical);
> + sals = decode_line_1 (&hookname, 1, (struct symtab *) NULL, 0, &canonical, 0);
> addr_end = hookname;
>
> if (sals.nelts == 0)
> @@ -4491,7 +4627,7 @@ hw_watchpoint_used_count (enum bptype ty
> *other_type_used = 0;
> ALL_BREAKPOINTS (b)
> {
> - if (b->enable_state == bp_enabled)
> + if (b->enable_state == bp_enabled && !b->pending)
> {
> if (b->type == type)
> i++;
> @@ -4717,14 +4853,21 @@ mention (struct breakpoint *b)
>
> if (say_where)
> {
> - if (addressprint || b->source_file == NULL)
> + if (b->pending)
> {
> - printf_filtered (" at ");
> - print_address_numeric (b->loc->address, 1, gdb_stdout);
> + printf_filtered (" (%s) pending.", b->addr_string);
> + }
> + else
> + {
> + if (addressprint || b->source_file == NULL)
> + {
> + printf_filtered (" at ");
> + print_address_numeric (b->loc->address, 1, gdb_stdout);
> + }
> + if (b->source_file)
> + printf_filtered (": file %s, line %d.",
> + b->source_file, b->line_number);
> }
> - if (b->source_file)
> - printf_filtered (": file %s, line %d.",
> - b->source_file, b->line_number);
> }
> do_cleanups (old_chain);
> if (ui_out_is_mi_like_p (uiout))
> @@ -4800,7 +4943,8 @@ create_breakpoints (struct symtabs_and_l
> static void
> parse_breakpoint_sals (char **address,
> struct symtabs_and_lines *sals,
> - char ***addr_string)
> + char ***addr_string,
> + int silent_flag)
> {
> char *addr_start = *address;
> *addr_string = NULL;
> @@ -4841,9 +4985,9 @@ parse_breakpoint_sals (char **address,
> || ((strchr ("+-", (*address)[0]) != NULL)
> && ((*address)[1] != '['))))
> *sals = decode_line_1 (address, 1, default_breakpoint_symtab,
> - default_breakpoint_line, addr_string);
> + default_breakpoint_line, addr_string, silent_flag);
> else
> - *sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0, addr_string);
> + *sals = decode_line_1 (address, 1, (struct symtab *) NULL, 0, addr_string, silent_flag);
> }
> /* For any SAL that didn't have a canonical string, fill one in. */
> if (sals->nelts > 0 && *addr_string == NULL)
> @@ -4897,6 +5041,16 @@ breakpoint_sals_to_pc (struct symtabs_an
> }
> }
>
> +static int
> +do_captured_parse_breakpoint (void *data)
> +{
> + struct captured_parse_breakpoint_args *args = data;
> +
> + parse_breakpoint_sals (args->arg_p, args->sals_p, args->addr_string_p, args->silent_flag);
> +
> + return GDB_RC_OK;
> +}
> +
> /* Set a breakpoint according to ARG (function, linenum or *address)
> flag: first bit : 0 non-temporary, 1 temporary.
> second bit : 0 normal breakpoint, 1 hardware breakpoint. */
> @@ -4907,14 +5061,18 @@ break_command_1 (char *arg, int flag, in
> int tempflag, hardwareflag;
> struct symtabs_and_lines sals;
> struct expression **cond = 0;
> + struct symtab_and_line pending_sal;
> /* Pointers in arg to the start, and one past the end, of the
> condition. */
> char **cond_string = (char **) NULL;
> + char *copy_arg;
> char *addr_start = arg;
> char **addr_string;
> struct cleanup *old_chain;
> struct cleanup *breakpoint_chain = NULL;
> - int i;
> + struct captured_parse_breakpoint_args parse_args;
> + int i, rc;
> + int pending = 0;
> int thread = -1;
> int ignore_count = 0;
>
> @@ -4924,19 +5082,51 @@ break_command_1 (char *arg, int flag, in
> sals.sals = NULL;
> sals.nelts = 0;
> addr_string = NULL;
> - parse_breakpoint_sals (&arg, &sals, &addr_string);
>
> - if (!sals.nelts)
> + parse_args.arg_p = &arg;
> + parse_args.sals_p = &sals;
> + parse_args.addr_string_p = &addr_string;
> + parse_args.silent_flag = 0;
> +
> + rc = catch_errors (do_captured_parse_breakpoint, &parse_args,
> + NULL, RETURN_MASK_ALL);
> +
> + if (rc != GDB_RC_OK)
> + {
> + char *msg = error_last_message ();
> + /* Check for "Function xxx not defined" and "No source file xxx ..." messages. If
> + these are matched, allow breakpoint to be set pending on shared library loads. */
> + if (!strncmp (msg, "Function ", sizeof ("Function ") - 1) ||
> + !strncmp (msg, "No source file", sizeof ("No source file") - 1))
No, find another way to do this. Just because GDB isn't properly
translated is no reason to make it even worse, and I don't want to
change the wording on an error message and discover that it breaks a
feature. You may need to get a proper error code out of linespec -
hoist the existing error calls up a level and provide an entry point
which returns success/failure/reason.
> + {
> + if (!query ("Make breakpoint pending on future shared library load? "))
> + return;
I'm still not really happy with this wording. What does that mean?
Plus, it'll look really out of place if we don't have shared library
support.
> + copy_arg = (char *)xmalloc (strlen (addr_start));
> + strcpy (copy_arg, addr_start);
The cast is not necessary nowadays. There are also a number of
functions for doing this, which don't suffer from the off-by-one error
above :) Try xstrdup instead.
> + addr_string = ©_arg;
Is addr_string guaranteed to still be NULL at this point, or are we
leaking?
> + sals.nelts = 1;
> + sals.sals = &pending_sal;
> + pending_sal.pc = 0;
> + pending = 1;
> + }
> + else
> + return;
> + }
> + else if (!sals.nelts)
> return;
>
> +
> /* Create a chain of things that always need to be cleaned up. */
> old_chain = make_cleanup (null_cleanup, 0);
>
> - /* Make sure that all storage allocated to SALS gets freed. */
> - make_cleanup (xfree, sals.sals);
> -
> - /* Cleanup the addr_string array but not its contents. */
> - make_cleanup (xfree, addr_string);
> + if (!pending)
> + {
> + /* Make sure that all storage allocated to SALS gets freed. */
> + make_cleanup (xfree, sals.sals);
> +
> + /* Cleanup the addr_string array but not its contents. */
> + make_cleanup (xfree, addr_string);
> + }
>
> /* Allocate space for all the cond expressions. */
> cond = xcalloc (sals.nelts, sizeof (struct expression *));
> @@ -4963,62 +5153,87 @@ break_command_1 (char *arg, int flag, in
>
> /* Resolve all line numbers to PC's and verify that the addresses
> are ok for the target. */
> - breakpoint_sals_to_pc (&sals, addr_start);
> + if (!pending)
> + breakpoint_sals_to_pc (&sals, addr_start);
>
> /* Verify that condition can be parsed, before setting any
> breakpoints. Allocate a separate condition expression for each
> breakpoint. */
> thread = -1; /* No specific thread yet */
> - for (i = 0; i < sals.nelts; i++)
> + if (!pending)
> {
> - char *tok = arg;
> - while (tok && *tok)
> + for (i = 0; i < sals.nelts; i++)
> {
> - char *end_tok;
> - int toklen;
> - char *cond_start = NULL;
> - char *cond_end = NULL;
> - while (*tok == ' ' || *tok == '\t')
> - tok++;
> -
> - end_tok = tok;
> -
> - while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
> - end_tok++;
> -
> - toklen = end_tok - tok;
> -
> - if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
> + char *tok = arg;
> + while (tok && *tok)
> {
> - tok = cond_start = end_tok + 1;
> - cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
> - make_cleanup (xfree, cond[i]);
> - cond_end = tok;
> - cond_string[i] = savestring (cond_start, cond_end - cond_start);
> - make_cleanup (xfree, cond_string[i]);
> - }
> - else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
> - {
> - char *tmptok;
> -
> - tok = end_tok + 1;
> - tmptok = tok;
> - thread = strtol (tok, &tok, 0);
> - if (tok == tmptok)
> - error ("Junk after thread keyword.");
> - if (!valid_thread_id (thread))
> - error ("Unknown thread %d\n", thread);
> + char *end_tok;
> + int toklen;
> + char *cond_start = NULL;
> + char *cond_end = NULL;
> + while (*tok == ' ' || *tok == '\t')
> + tok++;
> +
> + end_tok = tok;
> +
> + while (*end_tok != ' ' && *end_tok != '\t' && *end_tok != '\000')
> + end_tok++;
> +
> + toklen = end_tok - tok;
> +
> + if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
> + {
> + tok = cond_start = end_tok + 1;
> + cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
> + make_cleanup (xfree, cond[i]);
> + cond_end = tok;
> + cond_string[i] = savestring (cond_start, cond_end - cond_start);
> + make_cleanup (xfree, cond_string[i]);
> + }
> + else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
> + {
> + char *tmptok;
> +
> + tok = end_tok + 1;
> + tmptok = tok;
> + thread = strtol (tok, &tok, 0);
> + if (tok == tmptok)
> + error ("Junk after thread keyword.");
> + if (!valid_thread_id (thread))
> + error ("Unknown thread %d\n", thread);
> + }
> + else
> + error ("Junk at end of arguments.");
> }
> - else
> - error ("Junk at end of arguments.");
> }
> + create_breakpoints (sals, addr_string, cond, cond_string,
> + hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
> + tempflag ? disp_del : disp_donttouch,
> + thread, ignore_count, from_tty);
> }
> + else
> + {
> + struct symtab_and_line sal;
> + struct breakpoint *b;
>
> - create_breakpoints (sals, addr_string, cond, cond_string,
> - hardwareflag ? bp_hardware_breakpoint : bp_breakpoint,
> - tempflag ? disp_del : disp_donttouch,
> - thread, ignore_count, from_tty);
> + sal.symtab = NULL;
> + sal.pc = 0;
>
> + b = set_raw_breakpoint (sal, hardwareflag ? bp_hardware_breakpoint : bp_breakpoint);
> + set_breakpoint_count (breakpoint_count + 1);
> + b->number = breakpoint_count;
> + b->cond = *cond;
> + b->thread = thread;
> + b->addr_string = *addr_string;
> + b->cond_string = *cond_string;
> + b->ignore_count = ignore_count;
> + b->pending = 1;
> + b->disposition = tempflag ? disp_del : disp_donttouch;
> + b->from_tty = from_tty;
> + b->flag = flag;
> + mention (b);
> + }
> +
> if (sals.nelts > 1)
> {
> warning ("Multiple breakpoints were set.");
> @@ -5065,7 +5280,7 @@ do_captured_breakpoint (void *data)
> sals.nelts = 0;
> address_end = args->address;
> addr_string = NULL;
> - parse_breakpoint_sals (&address_end, &sals, &addr_string);
> + parse_breakpoint_sals (&address_end, &sals, &addr_string, 0);
>
> if (!sals.nelts)
> return GDB_RC_NONE;
> @@ -5289,7 +5504,7 @@ break_at_finish_command_1 (char *arg, in
>
> beg_addr_string = addr_string;
> sals = decode_line_1 (&addr_string, 1, (struct symtab *) NULL, 0,
> - (char ***) NULL);
> + (char ***) NULL, 0);
>
> xfree (beg_addr_string);
> old_chain = make_cleanup (xfree, sals.sals);
> @@ -5806,10 +6021,10 @@ until_break_command (char *arg, int from
>
> if (default_breakpoint_valid)
> sals = decode_line_1 (&arg, 1, default_breakpoint_symtab,
> - default_breakpoint_line, (char ***) NULL);
> + default_breakpoint_line, (char ***) NULL, 0);
> else
> sals = decode_line_1 (&arg, 1, (struct symtab *) NULL,
> - 0, (char ***) NULL);
> + 0, (char ***) NULL, 0);
>
> if (sals.nelts != 1)
> error ("Couldn't get information on specified line.");
> @@ -6269,7 +6484,7 @@ handle_gnu_v3_exceptions (int tempflag,
> trigger_func_name = xstrdup ("__cxa_throw");
>
> nameptr = trigger_func_name;
> - sals = decode_line_1 (&nameptr, 1, NULL, 0, NULL);
> + sals = decode_line_1 (&nameptr, 1, NULL, 0, NULL, 0);
> if (sals.nelts == 0)
> {
> xfree (trigger_func_name);
> @@ -6764,6 +6979,7 @@ delete_breakpoint (struct breakpoint *bp
> && !b->loc->duplicate
> && b->enable_state != bp_disabled
> && b->enable_state != bp_shlib_disabled
> + && !b->pending
> && b->enable_state != bp_call_disabled)
> {
> int val;
> @@ -6969,6 +7185,10 @@ breakpoint_re_set_one (void *bint)
> shlib_disabled breakpoint though. There's a fair chance we
> can't re-set it if the shared library it's in hasn't been
> loaded yet. */
> +
> + if (b->pending)
> + break;
> +
> save_enable = b->enable_state;
> if (b->enable_state != bp_shlib_disabled)
> b->enable_state = bp_disabled;
> @@ -6976,7 +7196,7 @@ breakpoint_re_set_one (void *bint)
> set_language (b->language);
> input_radix = b->input_radix;
> s = b->addr_string;
> - sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL);
> + sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL, 0);
> for (i = 0; i < sals.nelts; i++)
> {
> resolve_sal_pc (&sals.sals[i]);
> @@ -7063,7 +7283,7 @@ breakpoint_re_set_one (void *bint)
> value_free (b->val);
> b->val = evaluate_expression (b->exp);
> release_value (b->val);
> - if (VALUE_LAZY (b->val) && b->enable_state == bp_enabled)
> + if (VALUE_LAZY (b->val) && b->enable_state == bp_enabled && !b->pending)
> value_fetch_lazy (b->val);
>
> if (b->cond_string != NULL)
> @@ -7073,7 +7293,7 @@ breakpoint_re_set_one (void *bint)
> xfree (b->cond);
> b->cond = parse_exp_1 (&s, (struct block *) 0, 0);
> }
> - if (b->enable_state == bp_enabled)
> + if (b->enable_state == bp_enabled && !b->pending)
> mention (b);
> value_free_to_mark (mark);
> break;
> @@ -7364,70 +7584,92 @@ do_enable_breakpoint (struct breakpoint
> error ("Hardware breakpoints used exceeds limit.");
> }
>
> - if (bpt->enable_state != bp_permanent)
> - bpt->enable_state = bp_enabled;
> - bpt->disposition = disposition;
> - check_duplicates (bpt);
> - breakpoints_changed ();
> -
> - if (bpt->type == bp_watchpoint ||
> - bpt->type == bp_hardware_watchpoint ||
> - bpt->type == bp_read_watchpoint ||
> - bpt->type == bp_access_watchpoint)
> - {
> - if (bpt->exp_valid_block != NULL)
> - {
> - struct frame_info *fr =
> - fr = frame_find_by_id (bpt->watchpoint_frame);
> - if (fr == NULL)
> + if (bpt->pending)
> + {
> + if (bpt->enable_state != bp_enabled)
> + {
> + /* When enabling a pending breakpoint, we need to check if the breakpoint
> + is resolvable since shared libraries could have been loaded
> + after the breakpoint was disabled. */
> + struct breakpoint *new_bp;
> + breakpoints_changed ();
Sure this is necessary? If we resolve the breakpoint successfully,
this will get called from set_raw_breakpoint.
> + if ((new_bp = resolve_pending_breakpoint (bpt)) != NULL)
Might as well not use assignment-in-if-statement in new code.
> {
> - printf_filtered ("\
> -Cannot enable watchpoint %d because the block in which its expression\n\
> -is valid is not currently in scope.\n", bpt->number);
> - bpt->enable_state = bp_disabled;
> + delete_breakpoint (bpt);
> return;
> }
> -
> - save_selected_frame = deprecated_selected_frame;
> - save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
> - select_frame (fr);
> + bpt->enable_state = bp_enabled;
> + bpt->disposition = disposition;
> }
> -
> - value_free (bpt->val);
> - mark = value_mark ();
> - bpt->val = evaluate_expression (bpt->exp);
> - release_value (bpt->val);
> - if (VALUE_LAZY (bpt->val))
> - value_fetch_lazy (bpt->val);
> -
> - if (bpt->type == bp_hardware_watchpoint ||
> - bpt->type == bp_read_watchpoint ||
> + }
> + else /* Not a pending breakpoint. */
> + {
> + if (bpt->enable_state != bp_permanent)
> + bpt->enable_state = bp_enabled;
> + bpt->disposition = disposition;
> + check_duplicates (bpt);
> + breakpoints_changed ();
> +
> + if (bpt->type == bp_watchpoint ||
> + bpt->type == bp_hardware_watchpoint ||
> + bpt->type == bp_read_watchpoint ||
> bpt->type == bp_access_watchpoint)
> {
> - int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
> - int mem_cnt = can_use_hardware_watchpoint (bpt->val);
> -
> - /* Hack around 'unused var' error for some targets here */
> - (void) mem_cnt, i;
> - target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
> - bpt->type, i + mem_cnt, other_type_used);
> - /* we can consider of type is bp_hardware_watchpoint, convert to
> - bp_watchpoint in the following condition */
> - if (target_resources_ok < 0)
> + if (bpt->exp_valid_block != NULL)
> + {
> + struct frame_info *fr =
> + fr = frame_find_by_id (bpt->watchpoint_frame);
> + if (fr == NULL)
> + {
> + printf_filtered ("\
> +Cannot enable watchpoint %d because the block in which its expression\n\
> +is valid is not currently in scope.\n", bpt->number);
> + bpt->enable_state = bp_disabled;
> + return;
> + }
> +
> + save_selected_frame = deprecated_selected_frame;
> + save_selected_frame_level = frame_relative_level (deprecated_selected_frame);
> + select_frame (fr);
> + }
> +
> + value_free (bpt->val);
> + mark = value_mark ();
> + bpt->val = evaluate_expression (bpt->exp);
> + release_value (bpt->val);
> + if (VALUE_LAZY (bpt->val))
> + value_fetch_lazy (bpt->val);
> +
> + if (bpt->type == bp_hardware_watchpoint ||
> + bpt->type == bp_read_watchpoint ||
> + bpt->type == bp_access_watchpoint)
> {
> - printf_filtered ("\
> + int i = hw_watchpoint_used_count (bpt->type, &other_type_used);
> + int mem_cnt = can_use_hardware_watchpoint (bpt->val);
> +
> + /* Hack around 'unused var' error for some targets here */
> + (void) mem_cnt, i;
> + target_resources_ok = TARGET_CAN_USE_HARDWARE_WATCHPOINT (
> + bpt->type, i + mem_cnt, other_type_used);
> + /* we can consider of type is bp_hardware_watchpoint, convert to
> + bp_watchpoint in the following condition */
> + if (target_resources_ok < 0)
> + {
> + printf_filtered ("\
> Cannot enable watchpoint %d because target watch resources\n\
> have been allocated for other watchpoints.\n", bpt->number);
> - bpt->enable_state = bp_disabled;
> - value_free_to_mark (mark);
> - return;
> + bpt->enable_state = bp_disabled;
> + value_free_to_mark (mark);
> + return;
> + }
> }
> +
> + if (save_selected_frame_level >= 0)
> + select_frame (save_selected_frame);
> + value_free_to_mark (mark);
> }
> -
> - if (save_selected_frame_level >= 0)
> - select_frame (save_selected_frame);
> - value_free_to_mark (mark);
> }
> +
> if (modify_breakpoint_hook)
> modify_breakpoint_hook (bpt);
> breakpoint_modify_event (bpt->number);
> @@ -7512,10 +7754,10 @@ decode_line_spec_1 (char *string, int fu
> sals = decode_line_1 (&string, funfirstline,
> default_breakpoint_symtab,
> default_breakpoint_line,
> - (char ***) NULL);
> + (char ***) NULL, 0);
> else
> sals = decode_line_1 (&string, funfirstline,
> - (struct symtab *) NULL, 0, (char ***) NULL);
> + (struct symtab *) NULL, 0, (char ***) NULL, 0);
> if (*string)
> error ("Junk at end of line specification: %s", string);
> return sals;
> Index: breakpoint.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.h,v
> retrieving revision 1.26
> diff -u -p -r1.26 breakpoint.h
> --- breakpoint.h 6 Nov 2003 18:24:55 -0000 1.26
> +++ breakpoint.h 3 Dec 2003 01:21:50 -0000
> @@ -158,11 +158,14 @@ enum enable_state
> automatically enabled and reset when the call
> "lands" (either completes, or stops at another
> eventpoint). */
> - bp_permanent /* There is a breakpoint instruction hard-wired into
> + bp_permanent, /* There is a breakpoint instruction hard-wired into
> the target's code. Don't try to write another
> breakpoint instruction on top of it, or restore
> its value. Step over it using the architecture's
> SKIP_INSN macro. */
> + bp_shlib_pending, /* The eventpoint could not be set as an shlib has
> + not yet been loaded the first time. When the
> + shlib is loaded, it will be reissued. */
> };
>
>
> @@ -385,6 +388,15 @@ struct breakpoint
>
> /* Methods associated with this breakpoint. */
> struct breakpoint_ops *ops;
> +
> + /* Initial from_tty value. */
> + int from_tty;
> +
> + /* Initial flag value. */
> + int flag;
> +
> + /* Is breakpoint pending on shlib loads? */
> + int pending;
> };
>
> /* The following stuff is an abstract data type "bpstat" ("breakpoint
> Index: linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 linespec.c
> --- linespec.c 20 Oct 2003 14:38:42 -0000 1.52
> +++ linespec.c 3 Dec 2003 01:21:52 -0000
> @@ -100,7 +100,8 @@ static struct symtabs_and_lines decode_l
> int, int, char ***);
>
> static struct symtab *symtab_from_filename (char **argptr,
> - char *p, int is_quote_enclosed);
> + char *p, int is_quote_enclosed,
> + int silent_if_not_found);
>
> static struct
> symtabs_and_lines decode_all_digits (char **argptr,
> @@ -119,7 +120,8 @@ static struct symtabs_and_lines decode_d
> static struct symtabs_and_lines decode_variable (char *copy,
> int funfirstline,
> char ***canonical,
> - struct symtab *file_symtab);
> + struct symtab *file_symtab,
> + int silent_if_not_found);
>
> static struct
> symtabs_and_lines symbol_found (int funfirstline,
> @@ -637,7 +639,11 @@ decode_line_2 (struct symbol *sym_arr[],
>
> Note that it is possible to return zero for the symtab
> if no file is validly specified. Callers must check that.
> - Also, the line number returned may be invalid. */
> + Also, the line number returned may be invalid.
> +
> + If the silent_if_not_found flag is set, then no error message is issued for
> + an unknown function or file. This is used when searching for locations in
> + shared libraries that may not be loaded yet. */
>
> /* We allow single quotes in various places. This is a hideous
> kludge, which exists because the completer can't yet deal with the
GNU coding style says that, when you're talking about the value of a
variable in a comment, this should be SILENT_IF_NOT_FOUND.
If SILENT_IF_NOT_FOUND is set, then no error message is issued for
an unknown function or file. This is used when searching for
locations in shared libraries that may not be loaded yet. */
> @@ -646,7 +652,7 @@ decode_line_2 (struct symbol *sym_arr[],
>
> struct symtabs_and_lines
> decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
> - int default_line, char ***canonical)
> + int default_line, char ***canonical, int silent_if_not_found)
> {
> char *p;
> char *q;
> @@ -722,7 +728,8 @@ decode_line_1 (char **argptr, int funfir
> /* No, the first part is a filename; set s to be that file's
> symtab. Also, move argptr past the filename. */
>
> - file_symtab = symtab_from_filename (argptr, p, is_quote_enclosed);
> + file_symtab = symtab_from_filename (argptr, p, is_quote_enclosed,
> + silent_if_not_found);
> }
> #if 0
> /* No one really seems to know why this was added. It certainly
> @@ -827,7 +834,8 @@ decode_line_1 (char **argptr, int funfir
> /* Look up that token as a variable.
> If file specified, use that file's per-file block to start with. */
>
> - return decode_variable (copy, funfirstline, canonical, file_symtab);
> + return decode_variable (copy, funfirstline, canonical,
> + file_symtab, silent_if_not_found);
> }
>
>
> @@ -1422,10 +1430,13 @@ collect_methods (char *copy, struct type
>
>
> /* Return the symtab associated to the filename given by the substring
> - of *ARGPTR ending at P, and advance ARGPTR past that filename. */
> + of *ARGPTR ending at P, and advance ARGPTR past that filename. The
> + silent_if_not_found flag is used to specify that if the filename is
> + not found, do not issue an error message. */
>
> static struct symtab *
> -symtab_from_filename (char **argptr, char *p, int is_quote_enclosed)
> +symtab_from_filename (char **argptr, char *p,
> + int is_quote_enclosed, int silent_if_not_found)
> {
> char *p1;
> char *copy;
> @@ -1450,6 +1461,8 @@ symtab_from_filename (char **argptr, cha
> {
> if (!have_full_symbols () && !have_partial_symbols ())
> error ("No symbol table is loaded. Use the \"file\" command.");
> + if (silent_if_not_found)
> + throw_exception (RETURN_ERROR);
> error ("No source file named %s.", copy);
> }
>
> @@ -1626,11 +1639,13 @@ decode_dollar (char *copy, int funfirstl
>
>
> /* Decode a linespec that's a variable. If FILE_SYMTAB is non-NULL,
> - look in that symtab's static variables first. */
> + look in that symtab's static variables first. The silent_if_not_found
> + flag is used to specify that no error message should be issued if the
> + function is not found. */
>
> static struct symtabs_and_lines
> decode_variable (char *copy, int funfirstline, char ***canonical,
> - struct symtab *file_symtab)
> + struct symtab *file_symtab, int silent_if_not_found)
> {
> struct symbol *sym;
> /* The symtab that SYM was found in. */
> @@ -1658,6 +1673,9 @@ decode_variable (char *copy, int funfirs
> !have_partial_symbols () && !have_minimal_symbols ())
> error ("No symbol table is loaded. Use the \"file\" command.");
>
> + if (silent_if_not_found)
> + throw_exception (RETURN_ERROR);
> +
> error ("Function \"%s\" not defined.", copy);
> }
>
> Index: linespec.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 linespec.h
> --- linespec.h 12 Apr 2003 17:41:25 -0000 1.3
> +++ linespec.h 3 Dec 2003 01:21:52 -0000
> @@ -24,6 +24,7 @@ struct symtab;
> extern struct symtabs_and_lines
> decode_line_1 (char **argptr, int funfirstline,
> struct symtab *default_symtab, int default_line,
> - char ***canonical);
> + char ***canonical,
> + int silent_if_not_found);
>
> #endif /* defined (LINESPEC_H) */
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.123
> diff -u -p -r1.123 symtab.c
> --- symtab.c 22 Nov 2003 16:01:03 -0000 1.123
> +++ symtab.c 3 Dec 2003 01:21:53 -0000
> @@ -3868,7 +3868,7 @@ decode_line_spec (char *string, int funf
>
> sals = decode_line_1 (&string, funfirstline,
> cursal.symtab, cursal.line,
> - (char ***) NULL);
> + (char ***) NULL, 0);
>
> if (*string)
> error ("Junk at end of line specification: %s", string);
> Index: tracepoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/tracepoint.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 tracepoint.c
> --- tracepoint.c 2 Oct 2003 20:28:30 -0000 1.54
> +++ tracepoint.c 3 Dec 2003 01:21:53 -0000
> @@ -392,7 +392,7 @@ trace_command (char *arg, int from_tty)
> printf_filtered ("TRACE %s\n", arg);
>
> addr_start = arg;
> - sals = decode_line_1 (&arg, 1, (struct symtab *) NULL, 0, &canonical);
> + sals = decode_line_1 (&arg, 1, (struct symtab *) NULL, 0, &canonical, 0);
> addr_end = arg;
> if (!sals.nelts)
> return; /* ??? Presumably decode_line_1 has already warned? */
> @@ -2341,7 +2341,7 @@ scope_info (char *args, int from_tty)
> if (args == 0 || *args == 0)
> error ("requires an argument (function, line or *addr) to define a scope");
>
> - sals = decode_line_1 (&args, 1, NULL, 0, &canonical);
> + sals = decode_line_1 (&args, 1, NULL, 0, &canonical, 0);
> if (sals.nelts == 0)
> return; /* presumably decode_line_1 has already warned */
>
> Index: cli/cli-cmds.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 cli-cmds.c
> --- cli/cli-cmds.c 8 Nov 2003 00:13:03 -0000 1.36
> +++ cli/cli-cmds.c 3 Dec 2003 01:21:54 -0000
> @@ -557,7 +557,7 @@ edit_command (char *arg, int from_tty)
> /* Now should only be one argument -- decode it in SAL */
>
> arg1 = arg;
> - sals = decode_line_1 (&arg1, 0, 0, 0, 0);
> + sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
>
> if (! sals.nelts) return; /* C++ */
> if (sals.nelts > 1) {
> @@ -681,7 +681,7 @@ list_command (char *arg, int from_tty)
> dummy_beg = 1;
> else
> {
> - sals = decode_line_1 (&arg1, 0, 0, 0, 0);
> + sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
>
> if (!sals.nelts)
> return; /* C++ */
> @@ -714,9 +714,9 @@ list_command (char *arg, int from_tty)
> else
> {
> if (dummy_beg)
> - sals_end = decode_line_1 (&arg1, 0, 0, 0, 0);
> + sals_end = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
> else
> - sals_end = decode_line_1 (&arg1, 0, sal.symtab, sal.line, 0);
> + sals_end = decode_line_1 (&arg1, 0, sal.symtab, sal.line, 0, 0);
> if (sals_end.nelts == 0)
> return;
> if (sals_end.nelts > 1)
You will need to fix Insight builds also if you change the signature of
decode_line_1.
> ? penddoc.patch
> Index: gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.185
> diff -u -r1.185 gdb.texinfo
> --- gdb.texinfo 28 Oct 2003 22:04:47 -0000 1.185
> +++ gdb.texinfo 3 Dec 2003 01:23:13 -0000
> @@ -2600,16 +2600,23 @@
> Enabled breakpoints are marked with @samp{y}. @samp{n} marks breakpoints
> that are not enabled.
> @item Address
> -Where the breakpoint is in your program, as a memory address.
> +Where the breakpoint is in your program, as a memory address. If the
> +breakpoint is pending on a future load of a shared library, the address
> +will be listed as <PENDING>.
> @item What
> Where the breakpoint is in the source for your program, as a file and
> -line number.
> +line number. For a pending breakpoint, the original string passed to
> +the breakpoint command will be listed as it cannot be resolved until
> +the appropriate shared library is loaded in the future.
> @end table
>
> @noindent
> If a breakpoint is conditional, @code{info break} shows the condition on
> the line following the affected breakpoint; breakpoint commands, if any,
> -are listed after that.
> +are listed after that. A pending breakpoint is allowed to have a condition
> +specified for it. The condition is not parsed for validity until a shared
> +library is loaded that allows the pending breakpoint to resolve to a
> +valid location.
>
> @noindent
> @code{info break} with a breakpoint
> @@ -2631,6 +2638,23 @@
> your program. There is nothing silly or meaningless about this. When
> the breakpoints are conditional, this is even useful
> (@pxref{Conditions, ,Break conditions}).
> +
> +@cindex pending breakpoints
> +If a specified breakpoint location cannot be found, you will be prompted
> +as to whether you want to make the breakpoint pending on a future shared
> +library load. This is useful for setting breakpoints at the start of your
> +@value{GDBN} session for locations that you know will be dynamically loaded
> +later by the program being debugged.
> +You may specify
> +a condition for a pending breakpoint, however, the
> +condition will only be parsed for correctness after the breakpoint location
> +is resolved by a future shared library load. A pending breakpoint can be
> +enabled or disabled. A disabled pending breakpoint will not be resolved
> +on subsequent shared library loads. Enabling a disabled pending breakpoint
> +will cause @value{GDBN} to attempt to resolve the breakpoint in the event
> +that the required shared library has already been loaded.
> +Once a shared library load resolves a pending breakpoint location, the
> +pending breakpoint is removed and a real breakpoint is created.
>
> @cindex negative breakpoint numbers
> @cindex internal @value{GDBN} breakpoints
> Index: gdb.base/langs.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/langs.exp,v
> retrieving revision 1.2
> diff -u -p -r1.2 langs.exp
> --- gdb.base/langs.exp 23 Jun 2003 22:42:50 -0000 1.2
> +++ gdb.base/langs.exp 3 Dec 2003 01:22:27 -0000
> @@ -65,8 +65,23 @@ gdb_start
> gdb_reinitialize_dir $srcdir/$subdir
> gdb_load $binfile
>
> -gdb_test "b langs0" "Function \"langs0\" not defined\.|Breakpoint .* (deferred).*" \
> - "break on nonexistent function in langs.exp"
> +send_gdb "b langs0\n"
> +gdb_expect {
Use gdb_test_multiple, please.
> + -re "Function \"langs0\" not defined\.$" {
> + pass "break on nonexistent function in langs.exp"
> + }
> + -re "Function \"langs0\" not defined\..*Make breakpoint pending.*" {
> + send_gdb "n\n"
> + pass "break on nonexistent function in langs.exp"
> + }
> + -re "Breakpoint .* (deferred).*" {
> + pass "break on nonexistent function in langs.exp"
> + }
Might deferred be a better name for this concept anyway?
> + -re ".*$gdb_prompt.*" {
> + fail "break on nonexistent function in langs.exp"
> + }
> + timeout { fail "break on nonexistent function in langs.exp" }
> +}
>
> if {$hp_aCC_compiler} {
> set isfixed 1
> Index: gdb.base/shlib-call.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/shlib-call.exp,v
> retrieving revision 1.7
> diff -u -p -r1.7 shlib-call.exp
> --- gdb.base/shlib-call.exp 28 Sep 2002 01:12:04 -0000 1.7
> +++ gdb.base/shlib-call.exp 3 Dec 2003 01:22:27 -0000
> @@ -291,7 +291,7 @@ send_gdb "set width 0\n" ; gdb_expect -r
>
> # PR's 16495, 18213
> # test that we can re-set breakpoints in shared libraries
> -gdb_test "break shr1" "Breakpoint 1.*" "set bp in shared library"
> +gdb_breakpoint "shr1"
>
> # FIXME: should not send "run" explicitly. Non-portable.
>
> Index: lib/gdb.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
> retrieving revision 1.41
> diff -u -p -r1.41 gdb.exp
> --- lib/gdb.exp 23 Nov 2003 01:09:19 -0000 1.41
> +++ lib/gdb.exp 3 Dec 2003 01:22:28 -0000
> @@ -269,6 +269,11 @@ proc gdb_breakpoint { function } {
> -re "Breakpoint \[0-9\]* at .*: file .*, line $decimal.\r\n$gdb_prompt $" {}
> -re "Breakpoint \[0-9\]*: file .*, line $decimal.\r\n$gdb_prompt $" {}
> -re "Breakpoint \[0-9\]* at .*$gdb_prompt $" {}
> + -re "Make breakpoint pending.*y or n. $" {
> + send_gdb "n\n";
> + fail "setting breakpoint at $function" ;
> + return 0
> + }
> -re "$gdb_prompt $" { fail "setting breakpoint at $function" ; return 0 }
> timeout { fail "setting breakpoint at $function (timeout)" ; return 0 }
> }
You've made the testsuite handle this concept, but you haven't added
tests for it. You'll need to do that.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer