From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104909 invoked by alias); 18 May 2018 01:56:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 104811 invoked by uid 89); 18 May 2018 01:55:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=UD:test.c, testc, sk:get_cur, test.c X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 18 May 2018 01:55:50 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 827E51E512; Thu, 17 May 2018 21:55:48 -0400 (EDT) Subject: Re: [RFC 2/5] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND To: Philippe Waroquiers , gdb-patches@sourceware.org References: <20180505192804.12731-1-philippe.waroquiers@skynet.be> <20180505192804.12731-3-philippe.waroquiers@skynet.be> From: Simon Marchi Message-ID: <558abe97-4bae-dc11-a987-2adb7afa0f41@simark.ca> Date: Fri, 18 May 2018 01:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180505192804.12731-3-philippe.waroquiers@skynet.be> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-05/txt/msg00396.txt.bz2 On 2018-05-05 03:28 PM, Philippe Waroquiers wrote: > Also implement the command 'faas COMMAND', a shortcut for > 'frame apply all -s COMMAND' > --- > gdb/stack.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 214 insertions(+), 25 deletions(-) > > diff --git a/gdb/stack.c b/gdb/stack.c > index ecf1ee8379..61e86ab18b 100644 > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -777,7 +777,7 @@ do_gdb_disassembly (struct gdbarch *gdbarch, > > SRC_LINE: Print only source line. > LOCATION: Print only location. > - LOC_AND_SRC: Print location and source line. > + SRC_AND_LOC: Print location and source line. Can you push this as a separate, obvious patch? > > Used in "where" output, and to emit breakpoint or step > messages. */ > @@ -1687,6 +1687,39 @@ info_frame_command (const char *addr_exp, int from_tty) > } > } > > +/* COUNT specifies the nr of outermost frames we are interested in. nr -> number. But actually I think the description is clear enough with only the second sentence: /* Return the starting frame needed to handle the COUNT outermost frames. */ > + trailing_outermost_frame returns the starting frame > + needed to handle COUNT outermost frames. */ > + > +static struct frame_info* Space before *. > +trailing_outermost_frame (int count) > +{ > + struct frame_info *current; > + struct frame_info *trailing; > + > + trailing = get_current_frame (); > + > + gdb_assert (count > 0); > + > + current = trailing; > + while (current && count--) current != nullptr > + { > + QUIT; > + current = get_prev_frame (current); > + } > + > + /* Will stop when CURRENT reaches the top of the stack. > + TRAILING will be COUNT below it. */ > + while (current) current != nullptr > + { > + QUIT; > + trailing = get_prev_frame (trailing); > + current = get_prev_frame (current); > + } > + > + return trailing; > +} > + > /* Print briefly all stack frames or just the innermost COUNT_EXP > frames. */ > > @@ -1696,7 +1729,6 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags, > { > struct frame_info *fi; > int count; > - int i; > int py_start = 0, py_end = 0; > enum ext_lang_bt_status result = EXT_LANG_BT_ERROR; > > @@ -1756,30 +1788,13 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags, > > if (count_exp != NULL && count < 0) > { > - struct frame_info *current; > - > - count = -count; > - > - current = trailing; > - while (current && count--) > - { > - QUIT; > - current = get_prev_frame (current); > - } > - > - /* Will stop when CURRENT reaches the top of the stack. > - TRAILING will be COUNT below it. */ > - while (current) > - { > - QUIT; > - trailing = get_prev_frame (trailing); > - current = get_prev_frame (current); > - } > - > + trailing = trailing_outermost_frame (-count); > count = -1; > } > + else > + trailing = get_current_frame (); We now do trailing = get_current_frame (); both before the if and in the else. We just need one, I don't mind which one. > > - for (i = 0, fi = trailing; fi && count--; i++, fi = get_prev_frame (fi)) > + for (fi = trailing; fi && count--; fi = get_prev_frame (fi)) Removing the i variable seems to be something you could push as an obvious patch. > { > QUIT; > > @@ -2506,9 +2521,148 @@ func_command (const char *arg, int from_tty) > select_and_print_frame (frame); > } > > +/* Apply a GDB command to a all stack frames, or innermost COUNT frames. > + With a negative COUNT, apply command on outermost -COUNT frames. > + > + frame apply 3 info frame Apply 'info frame' to frames 0, 1, 2 > + frame apply -3 info frame Apply 'info frame' to outermost 3 frames. > + frame apply all x/i $pc Apply 'x/i $pc' cmd to all frames. > + frame apply all -s p local_var_no_idea_in_which_frame > + If a frame has a local variable called > + local_var_no_idea_in_which_frame, print frame > + and value of local_var_no_idea_in_which_frame. > + frame apply all -sqq p local_var_no_idea_in_which_frame > + Same as before, but only print the variable value. */ > + > +/* Apply a GDB command to COUNT stack frames, starting at trailing. > + COUNT -1 means all frames starting at trailing. WHICH_COMMAND is used trailing -> TRAILING > + for error messages. */ > +static void > +frame_apply_command_count (const char* which_command, > + const char *cmd, int from_tty, > + struct frame_info *trailing, int count) > +{ > + int print_what_v = 2; /* Corresponding to LOCATION. */ > + enum print_what print_what[5] = > + { > + LOC_AND_ADDRESS, /* Should never be used, this is verbosity 0. */ > + SRC_LINE, > + LOCATION, > + LOC_AND_ADDRESS, > + SRC_AND_LOC > + }; > + bool cont; > + bool silent; > + struct frame_info *fi; > + > + if (cmd != NULL) > + check_for_flags_vqcs (which_command, &cmd, > + &print_what_v, 4, > + &cont, &silent); > + > + if (cmd == NULL || *cmd == '\000') '\0' ? > + error (_("Please specify a command to apply on the selected frames")); > + > + for (fi = trailing; fi && count--; fi = get_prev_frame (fi)) > + { > + struct frame_id frame_id = get_frame_id (fi); > + > + QUIT; > + > + select_frame (fi); > + TRY > + { > + std::string cmd_result = execute_command_to_string (cmd, from_tty); > + if (!silent || cmd_result.length() > 0) > + { > + if (print_what_v > 0) > + print_stack_frame (fi, 1, print_what[print_what_v], 0); > + printf_filtered ("%s", cmd_result.c_str ()); > + } > + } > + CATCH (ex, RETURN_MASK_ERROR) > + { > + if (!silent) > + { > + if (print_what_v > 0) > + print_stack_frame (fi, 1, print_what [print_what_v], 0); > + if (cont) > + printf_filtered ("%s\n", ex.message); > + else > + throw_exception (ex); > + } > + } > + END_CATCH; > + > + /* execute_command_to_string might invalidate FI. */ > + fi = frame_find_by_id (frame_id); > + if (fi == NULL) > + { > + trailing = NULL; > + warning (_("Unable to restore previously selected frame.")); > + break; > + } > + } > +} > + > +static void > +frame_apply_all_command (const char *cmd, int from_tty) > +{ > + struct frame_info *fi; > + struct frame_info *trailing = NULL; > + int count; > + int i; These variables are unused. > + > + if (!target_has_stack) > + error (_("No stack.")); > + > + frame_apply_command_count ("frame apply all", cmd, from_tty, > + get_current_frame(), INT_MAX); Space after get_current_frame. > +} > + > +/* Implementation of the "frame apply" command. */ > + > +static void > +frame_apply_command (const char* cmd, int from_tty) > +{ > + int count; > + struct frame_info *trailing; > + > + if (!target_has_stack) > + error (_("No stack.")); > + > + count = get_number_trailer (&cmd, 0); > + > + if (count < 0) > + { > + trailing = trailing_outermost_frame (-count); > + count = -1; > + } > + else > + trailing = get_current_frame (); > + > + frame_apply_command_count ("frame apply", cmd, from_tty, > + trailing, count); > +} While testing this, I intuitively expected that this command would work the same way as thread apply: take a frame number of frame number range. I had to think for a second when this did not do as I expected: (gdb) bt #0 break_here () at test.c:6 #1 0x000055555555461a in main (argc=1, argv=0x7fffffffde78) at test.c:12 (gdb) frame apply 0 print 123 (gdb) frame apply 1 print 123 #0 break_here () at test.c:6 $6 = 123 So I'm wondering whether it would make more sense to have both thread apply and frame apply work in a similar way, to avoid confusing users. > + > +/* Implementation of the "faas" command. */ > + > +static void > +faas_command (const char *cmd, int from_tty) > +{ > + std::string expanded = std::string ("frame apply all -s ") + std::string (cmd); > + execute_command (expanded.c_str (), from_tty); > +} > + > + > +/* Commands with a prefix of `frame'. */ > +struct cmd_list_element *frame_cmd_list = NULL; > + > void > _initialize_stack (void) > { > + static struct cmd_list_element *frame_apply_list = NULL; > + > add_com ("return", class_stack, return_command, _("\ > Make selected stack frame return to its caller.\n\ > Control remains in the debugger, but when you continue\n\ > @@ -2531,14 +2685,49 @@ An argument says how many frames down to go.")); > Same as the `down' command, but does not print anything.\n\ > This is useful in command scripts.")); > > - add_com ("frame", class_stack, frame_command, _("\ > + add_prefix_cmd ("frame", class_stack, frame_command, _("\ > Select and print a stack frame.\nWith no argument, \ > print the selected stack frame. (See also \"info frame\").\n\ > An argument specifies the frame to select.\n\ > -It can be a stack frame number or the address of the frame.")); > +It can be a stack frame number or the address of the frame."), > + &frame_cmd_list, "frame ", 1, &cmdlist); > > add_com_alias ("f", "frame", class_stack, 1); > > +#define FRAME_APPLY_FLAGS_HELP "\ > +FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\ > + c(continue), s(silent).\n\ > +Verbosity (default 2) controls what to print for a frame:\n\ > + 0 : no frame info is printed\n\ > + 1 : source line\n\ > + 2 : location\n\ > + 3 : location and address\n\ > + 4 : source line and location\n\ > +By default, if a COMMAND raises an error, frame apply is aborted.\n\ > +Flag c indicates to print the error and continue.\n\ > +Flag s indicates to silently ignore a COMMAND that raises an error\n\ > +or produces no output." I would prefer if this was a const char [] rather than a macro. Thanks, Simon