From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27707 invoked by alias); 1 Aug 2013 12:33:19 -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 27698 invoked by uid 89); 1 Aug 2013 12:33:19 -0000 X-Spam-SWARE-Status: No, score=-6.2 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RDNS_NONE,SPF_HELO_PASS,SPF_PASS autolearn=no version=3.3.1 Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 01 Aug 2013 12:33:18 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r71CX6ca020859 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 1 Aug 2013 08:33:06 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r71CX49I027021; Thu, 1 Aug 2013 08:33:05 -0400 Message-ID: <51FA557F.5@redhat.com> Date: Thu, 01 Aug 2013 12:33:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: Eli Zaretskii , gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] Add options to skip unavailable locals References: <1372816106-15942-1-git-send-email-yao@codesourcery.com> <1372816106-15942-3-git-send-email-yao@codesourcery.com> <83vc4ra1ps.fsf@gnu.org> <51EC8CCE.7020007@codesourcery.com> In-Reply-To: <51EC8CCE.7020007@codesourcery.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00019.txt.bz2 On 07/22/2013 02:37 AM, Yao Qi wrote: > 2013-07-22 Pedro Alves > Yao Qi > > * mi/mi-cmd-stack.c (list_args_or_locals): Adjust prototype. > (parse_no_frames_option): Remove. > (mi_cmd_stack_list_locals): Handle --skip-unavailable. > (mi_cmd_stack_list_args): Adjust. > (mi_cmd_stack_list_variables): Handle --skip-unavailable. > (list_args_or_locals): New parameter 'skip_unavailable'. > Handle it. > * valprint.c (scalar_type_p): Rename to ... > (val_print_scalar_type_p): ... this. Make extern. > (val_print, value_check_printable): Adjust. > * valprint.h (val_print_scalar_type_p): Declare. > * value.c (value_entirely_unavailable): New function. > * value.h (value_entirely_unavailable): Declare. > > * NEWS: Mention the new option "--skip-unavailable" to these > MI commands. Hmm, this "these" reads out of nowhere. "Which these?" > > gdb/doc: > > 2013-07-22 Pedro Alves > Yao Qi > > * gdb.texinfo (GDB/MI Stack Manipulation)<-stack-list-locals>: Space before "<". > Document new --skip-unavailable option. > <-stack-list-variables>: Document new --skip-unavailable option. > > +If the @code{--skip-unavailable} option is specified, local variables > +that are not available are not listed. Partially available locals > +variables are still displayed, however. s/available locals variables/available local variables/ > +If the @code{--skip-unavailable} option is specified, local variables > +and arguments that are not collected are not listed. This still says "that are not collected". I think it should say "that that are not available", like the other bits (and similarly to the adjustment done in other patches, IIRC). > @@ -295,18 +293,39 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc) > struct ui_out *uiout = current_uiout; > int raw_arg = 0; > enum py_bt_status result = PY_BT_ERROR; > + int skip_unavailable = 0; > + int oind = 0; > > - if (argc > 0) > - raw_arg = parse_no_frames_option (argv[0]); > + /* We can't use mi_getopt here, because the number of options is not > + determined. */ Hmm. Isn't that easy to fix though? We'd just need an mi_getopt variant that doesn't error out when it sees an unknown option, but instead returns that option's position (similarly to getopt). It's then the caller's responsibility to parse the rest of the option string. > + for (oind = 0; oind < argc; oind++) > + { > + int found = 0; > > - if (argc < 1 || (argc > 3 && ! raw_arg) || (argc == 2 && ! raw_arg)) > + if (strcmp (argv[oind], "--no-frame-filters") == 0) > + { > + raw_arg = oind + 1; > + found = 1; > + } > + else if (strcmp (argv[oind], "--skip-unavailable") == 0) > + { > + skip_unavailable = 1; > + found = 1; > + } > + > + if (!found) > + break; > + } > + > + if (argc - oind != 1 && argc - oind != 3) > error (_("-stack-list-arguments: Usage: " \ > - "[--no-frame-filters] PRINT_VALUES [FRAME_LOW FRAME_HIGH]")); > + "[--no-frame-filters] [--skip-unavailable] " > + "PRINT_VALUES [FRAME_LOW FRAME_HIGH]")); > > - if (argc >= 3) > + if (argc - oind == 3) > { > - frame_low = atoi (argv[1 + raw_arg]); > - frame_high = atoi (argv[2 + raw_arg]); > + frame_low = atoi (argv[1 + oind]); > + frame_high = atoi (argv[2 + oind]); > } > else > { > @@ -316,7 +335,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc) > frame_high = -1; > } > > @@ -597,7 +626,6 @@ list_args_or_locals (enum what_to_list what, enum print_values values, > if (print_me) > { > struct symbol *sym2; > - struct frame_arg arg, entryarg; > > if (SYMBOL_IS_ARGUMENT (sym)) > sym2 = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), > @@ -607,33 +635,56 @@ list_args_or_locals (enum what_to_list what, enum print_values values, > sym2 = sym; > gdb_assert (sym2 != NULL); > > - memset (&arg, 0, sizeof (arg)); > - arg.sym = sym2; > - arg.entry_kind = print_entry_values_no; > - memset (&entryarg, 0, sizeof (entryarg)); > - entryarg.sym = sym2; > - entryarg.entry_kind = print_entry_values_no; > - > - switch (values) > + /* Need to read the value before being able to determine > + whether its unavailable. */ > + if (values == PRINT_ALL_VALUES > + || values == PRINT_SIMPLE_VALUES > + || skip_unavailable) > + val = read_var_value (sym2, fi); > + > + if (skip_unavailable > + && (value_entirely_unavailable (val) > + /* A scalar object that does not have all bits > + available is also considered unavailable, > + because all bits contribute to its > + representation. */ > + || (val_print_scalar_type_p (value_type (val)) > + && !value_bytes_available (val, > + value_embedded_offset (val), > + TYPE_LENGTH (value_type (val)))))) > + ; > + else I don't think this has been updated right for entry values. With entry values, we now have _two_ values to account for. I think we need to do this once for each of the regular arg and the entry arg? > { > - case PRINT_SIMPLE_VALUES: > - type = check_typedef (sym2->type); > - if (TYPE_CODE (type) != TYPE_CODE_ARRAY > - && TYPE_CODE (type) != TYPE_CODE_STRUCT > - && TYPE_CODE (type) != TYPE_CODE_UNION) > + struct frame_arg arg, entryarg; > + > + memset (&arg, 0, sizeof (arg)); > + arg.sym = sym2; > + arg.entry_kind = print_entry_values_no; > + memset (&entryarg, 0, sizeof (entryarg)); > + entryarg.sym = sym2; > + entryarg.entry_kind = print_entry_values_no; > + > + switch (values) > { > - case PRINT_ALL_VALUES: > - read_frame_arg (sym2, fi, &arg, &entryarg); > + case PRINT_SIMPLE_VALUES: > + type = check_typedef (sym2->type); > + if (TYPE_CODE (type) != TYPE_CODE_ARRAY > + && TYPE_CODE (type) != TYPE_CODE_STRUCT > + && TYPE_CODE (type) != TYPE_CODE_UNION) > + { > + case PRINT_ALL_VALUES: > + read_frame_arg (sym2, fi, &arg, &entryarg); > + } > + break; > } > - break; > - } > > - if (arg.entry_kind != print_entry_values_only) > - list_arg_or_local (&arg, what, values); > - if (entryarg.entry_kind != print_entry_values_no) > - list_arg_or_local (&entryarg, what, values); > - xfree (arg.error); > - xfree (entryarg.error); > + if (arg.entry_kind != print_entry_values_only) > + list_arg_or_local (&arg, what, values); > + if (entryarg.entry_kind != print_entry_values_no) > + list_arg_or_local (&entryarg, what, values); > + xfree (arg.error); > + xfree (entryarg.error); > + } > } > } > -- Pedro Alves