This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA]: pending breakpoint support [1/3]
On Tue, Jan 27, 2004 at 03:41:04PM -0500, J. Johnston wrote:
> Anyway, I have a new and improved patch attached that
> addresses the issues below. I have included answers to questions below as
> well.
Thanks. I have just one question and a couple of cosmetic issues left.
I'm also still curious about pending breakpoints which resolve to
multiple functions, et cetera. We can revisit at a later date.
> >When reading the testsuite diffs I asked:
> > Might deferred be a better name for this concept anyway?
> >
> >Comments?
> >
>
> This is a case of "you can't please everybody all the time". I am
> certainly happy with it.
OK. I have some ideas for better wording of the message, but I can
take care of it later.
> Well, it only gets there on a "not-found" error from decode_line_1 so the
> onus is on decode_line_1 and it's children to do proper cleanups which is
> outside of this patch.
Gotcha.
About the new patch:
> + /* Flag value for pending breakpoint.
> + first bit : 0 non-temporary, 1 temporary.
> + second bit : 0 normal breakpoint, 1 hardware breakpoint. */
> + int flag;
OK, now I can see what flag is used for. This raises a new question.
Should temporary breakpoints be allowed to be pending?
Oh, I guess the answer is yes. tbreak my_dso_initialization; run. So
this is fine.
> +/* Try and resolve a pending breakpoint. */
> +static int
> +resolve_pending_breakpoint (struct breakpoint *b)
I love the new version of this function...
> + input_radix = b->input_radix;
> + rc = break_command_1 (b->addr_string, b->flag, b->from_tty, b);
> +
> + if (rc == GDB_RC_OK)
> + /* Pending breakpoint has been resolved. */
> + printf_filtered ("Resolves pending breakpoint \"%s\"\n", b->addr_string);
Spelling error, though.
> @@ -4779,6 +4888,26 @@ create_breakpoints (struct symtabs_and_l
> b->ignore_count = ignore_count;
> b->enable_state = bp_enabled;
> b->disposition = disposition;
> + /* If resolving a pending breakpoint, a check must be made to see if
> + the user has specified a new condition or commands for the
> + breakpoint. A new condition will override any condition that was
> + initially specified with the initial breakpoint command. */
> + if (pending_bp)
> + {
> + char *arg;
> + if (pending_bp->cond_string)
> + {
> + arg = pending_bp->cond_string;
> + 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 pending breakpoint condition expression");
> + }
> + /* If there are commands associated with the breakpoint, they should
> + be copied too. */
> + if (pending_bp->commands)
> + b->commands = copy_command_lines (pending_bp->commands);
> + }
> mention (b);
> }
> }
Here's the one question.
The only way to get here with a PENDING_BP is from break_command_1 from
resolve_pending_breakpoint. So I don't think it is possible for there
to be a condition already set on B, which makes the comment about
"overriding" such a condition a little strange. Am I right, or is
there some other way to get a condition to here?
> -static void
> -break_command_1 (char *arg, int flag, int from_tty)
> + PENDING_BP is non-NULL when this function is being called to resolve
> + a pending breakpoint. */
> +
> +static int
> +break_command_1 (char *arg, int flag, int from_tty, struct breakpoint *pending_bp)
> {
> 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. */
Aha! That's where that baffling comment came from, it was already
present in break_command_1. Here we have a var named ARG, so it makes
more sense, but the variables cond_start and cond_end that it refers to
have walked down about a hundred lines.
> + 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 ();
> + if (resolve_pending_breakpoint (bpt) == GDB_RC_OK)
new_bp is no longer used, you can remove it.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer