From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12302 invoked by alias); 5 Aug 2013 14:12:00 -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 12277 invoked by uid 89); 5 Aug 2013 14:11:59 -0000 X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=AWL,BAYES_00,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; Mon, 05 Aug 2013 14:11:59 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r75EBpFx017841 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 5 Aug 2013 10:11:51 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r75EBn8Q007502; Mon, 5 Aug 2013 10:11:50 -0400 Message-ID: <51FFB2A5.8040005@redhat.com> Date: Mon, 05 Aug 2013 14:12: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: gdb-patches@sourceware.org Subject: Re: [PATCH] Only print entry values for arguments. References: <1375663450-5825-1-git-send-email-yao@codesourcery.com> In-Reply-To: <1375663450-5825-1-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00141.txt.bz2 On 08/05/2013 01:44 AM, Yao Qi wrote: > Hi, > When I think about how to handle entry values in my "skip unavailable" > patch, I played with "set print entry-values" to different values, and > use MI commands '-stack-list-{locals,arguments,variables}' to see the > changes on output. However, I find that "print entry-values" affects > the output of "-stack-list-locals", which is not expected. > > -gdb-set print entry-values only > ^done > -stack-list-locals --simple-values > ^done,locals=[{name="array",type="unsigned char [2]"},{name="i@entry",type="int",value=""}] > > -gdb-set print entry-values both > ^done > -stack-list-locals --simple-values > ^done,locals=[{name="array",type="unsigned char [2]"},{name="i",type="int",value=""},{name="i@entry",type="int",value=""}] > > "print entry-values" is the option about printing frame arguments, > which has nothing to do with locals. This patch is to check whether it > is an argument to decide to call read_frame_arg or read_frame_local. > read_frame_local is a new function which is to print local without > considering much on "print entry-values". I have to say that it is > odd to pass "struct frame_arg *" to function read_frame_local, but it > is required by caller and the following calls. Functions > list_args_or_locals and list_arg_or_local process both arguments and > locals by means of "struct frame_arg *", so I have to use "struct > frame_arg *" for function read_frame_local too. Note that "print > entry-values" is for arguments only, but is checked in both > list_args_or_locals and list_arg_or_local. > We need some changes here > to apply value of "print entry-values" to arguments only, but it is > not the goal of this patch. Yeah, or we could rename "struct frame_arg" to "struct frame_value" or some such, and update it's comments to better reflect reality. > 2013-08-05 Yao Qi > > * frame.h (read_frame_local): Declare. > * mi/mi-cmd-stack.c (list_args_or_locals): Call > read_frame_local. > * stack.c (read_frame_local): New. > > gdb/testsuite: > > 2013-08-05 Yao Qi > > * gdb.trace/mi-trace-unavailable.exp: Don't set > "print entry-values" to "no". > (test_trace_unavailable): Set various values to > "print entry-values" to test the output of > '-stack-list-locals' is not affected. > diff --git a/gdb/stack.c b/gdb/stack.c > index 510f20d..9e9ebc1 100644 > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -301,6 +301,33 @@ print_frame_arg (const struct frame_arg *arg) > annotate_arg_end (); > } > > +/* Read in inferior function local SYM at FRAM into ARGP. Caller is typo: FRAME. > + responsible for xfree of ARGP->ERROR. This function never throws an > + exception. */ > + > +void > +read_frame_local (struct symbol *sym, struct frame_info *frame, > + struct frame_arg *argp) > +{ > + volatile struct gdb_exception except; > + struct value *val = NULL; > + char *val_error = NULL; > + > + TRY_CATCH (except, RETURN_MASK_ERROR) > + { > + val = read_var_value (sym, frame); > + } > + if (val == NULL) > + { > + val_error = alloca (strlen (except.message) + 1); > + strcpy (val_error, except.message); > + } > + > + argp->sym = sym; > + argp->val = val; > + argp->error = val_error ? xstrdup (val_error) : NULL; > +} > + I can see where this came from, but isn't all this the same as: argp->error = (val == NULL) ? xstrdup (except.message) : NULL; argp->val = val; argp->sym = sym; ? > @@ -89,6 +86,21 @@ proc test_trace_unavailable { data_source } { > ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \ > "-trace-find frame-number 0" > > + # Option of print entry-values shouldn't affect the output of > + # '-stack-list-locals'. # The "print entry-values" option shouldn't affect the # output of '-stack-list-locals'. would read better a little less odd to me. > -mi_gdb_test "-gdb-set print entry-values no" {\^done} \ > - "-gdb-set print entry-values no" ... > + mi_gdb_test "-gdb-set print entry-values no" {\^done} "" Was there a reason to make the message argument the empty string? I'd rather that bit was mentioned in the ChangeLog too: > 2013-08-05 Yao Qi > > * gdb.trace/mi-trace-unavailable.exp: Don't set > "print entry-values" to "no". > (test_trace_unavailable): Set various values to > "print entry-values" to test the output of > '-stack-list-locals' is not affected. (test_trace_unavailable): Set various values to "print entry-values" to test that the output of '-stack-list-locals' is not affected, and then set set "print entry-values" to "no". Otherwise it looks good to me. I wonder whether we should put this in 7.6.1 ? -- Pedro Alves