From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7655 invoked by alias); 16 Sep 2013 19:05:38 -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 7646 invoked by uid 89); 16 Sep 2013 19:05:37 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Sep 2013 19:05:37 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.1 required=5.0 tests=AWL,BAYES_05,KHOP_THREADED,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8GJ5YDL029741 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 16 Sep 2013 15:05:35 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8GJ5XdC012476; Mon, 16 Sep 2013 15:05:33 -0400 Message-ID: <5237567C.8050406@redhat.com> Date: Mon, 16 Sep 2013 19:05: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: Andrew Burgess CC: gdb-patches@sourceware.org, Mark Kettenis Subject: Re: [PATCH] Print registers not saved in the frame as "", instead of "". References: <5200F55E.2050308@broadcom.com> <201308061318.r76DIMdd016369@glazunov.sibelius.xs4all.nl> <5200FECF.7030304@broadcom.com> <201308061541.r76FfYQN022875@glazunov.sibelius.xs4all.nl> <520142D9.4030304@redhat.com> <5208E3C8.7060107@broadcom.com> <5208E938.3080305@redhat.com> <201308122001.r7CK1862007934@glazunov.sibelius.xs4all.nl> <520E7255.7080206@redhat.com> <5211F25A.5070907@broadcom.com> <5228B15F.7060108@redhat.com> <5228B2D8.7060604@broadcom.com> In-Reply-To: <5228B2D8.7060604@broadcom.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-09/txt/msg00468.txt.bz2 On 09/05/2013 05:35 PM, Andrew Burgess wrote: > On 05/09/2013 5:29 PM, Pedro Alves wrote: >> Hi guys, >> >> Getting back to this, trying to make progress. >> >> On 08/19/2013 11:24 AM, Andrew Burgess wrote: >>> On 16/08/2013 7:41 PM, Pedro Alves wrote: >>>> On 08/12/2013 09:01 PM, Mark Kettenis wrote: >>>>>> Date: Mon, 12 Aug 2013 14:55:04 +0100 >>>>>> From: Pedro Alves >>>>>> >>>>>> On 08/12/2013 02:31 PM, Andrew Burgess wrote: >>>>>>> On 06/08/2013 7:39 PM, Pedro Alves wrote: >>>>>>>> On 08/06/2013 04:41 PM, Mark Kettenis wrote: >>>>>>>>>> Date: Tue, 6 Aug 2013 14:49:03 +0100 >>>>>>>>>> From: "Andrew Burgess" >>>>>>>> >>>>>>>>>> 3. My understanding was that values lost due to the ABI of a call site >>>>>>>>>> were recorded as optimized out. For evidence I would present >>>>>>>>>> dwarf2_frame_prev_register, and how DWARF2_FRAME_REG_UNDEFINED is handled. >>>>>>>>>> >>>>>>>>>> For these reasons I believe my patch should still be considered, what do >>>>>>>>>> you think? >>>>>>>>> >>>>>>>>> I think that registers are either available or unavailble. A register >>>>>>>>> being unavailble implies that a variable that is supposed to live in >>>>>>>>> such a register may have been optimized out. Whether GDB's pseudo >>>>>>>>> variables that respresent registers are considered unavailable or >>>>>>>>> optimized out in that case is arguable. >>>>>>>> >>>>>>>> I think improving consistency as in Andrew's patch is good. >>>>>>> >>>>>>> Given almost a week has passed with no further feedback I plan to >>>>>>> commit this patch tomorrow unless there's any further discussion to be had. >>>>>> >>>>>> TBC, note my opinion doesn't get to overrule Mark's. Consensus >>>>>> works much better, and Mark does have deep knowledge of all >>>>>> ABI/pseudo registers/etc. gdb things. >>>>>> That said, Mark, if you still disagree, please counter argue, >>>>>> otherwise, we'll just have to assume you do agree with the >>>>>> rationales and clarifications. >>>>> >>>>> Can't say I agree. It simply doesn't make sense for registers to be >>>>> "optimized out". I guess there are two reasons why GDB can't display >>>>> the contents of a register in a frame: >>>>> >>>>> 1. The register contents aren't made available by the debugging >>>>> interface, i.e. ptrace(2) or the remote stub doesn't tell us. >>>>> >>>>> 2. The register wasn't saved before calling another function. >>>>> >>>>> I guess after Andrew's chnages 1) would be shown as and >>>>> 2) would become . But in the latter case something >>>>> like would make more sense. >>>>> >>>>> That said, Pedro, you're pretty much the expert for this area of GDB. >>>>> So If you think Andrew should go ahead with this, feel free to ignore >>>>> me. >>>> >>>> This is a tough call. I do agree that "optimized out" for registers >>>> is a bit confusing. However, we already do print "" in >>>> other places, such as when printing expressions, and consistency >>>> is good. If we did add a distinction, I agree with Andrew that it should >>>> be done in a more systematic way. However, I'm not really sure we need >>>> much machinery. Wouldn't something like: >>>> >>>> void >>>> val_print_optimized_out (const struct value *val, struct ui_file *stream) >>>> { >>>> if (value_lval_const (val) == lval_register) >>>> fprintf_filtered (stream, _("")); >>>> else >>>> fprintf_filtered (stream, _("")); >>>> } >>>> >>>> work? What could be the register value cases that would print >>>> "not saved" that we'd still want to print "optimized out" ? >>> >>> The only case I can immediately think of where this would cause a >>> problem would be for computed locations, (lval_computed). The easy >>> answer would be (in that case) the blame the compiler - why say the >>> location is in a register if that register is volatile - but sadly I see >>> this way too often. >> >> Hmm, OK, but then lval_computed values with that change won't >> ever show "", due to the lval_register check. IOW, >> we'd have to do something else in addition to lval_computed values >> to make them print something other than the current . >> >> However, I've come to think there's a really simple rule to >> follow here -- We should only ever print for values >> that represent machine/pseudo registers. IOW, $pc, $rax, etc. >> If the debug info happens to describe a variable as being located >> in some optimized out register, we should still print >> . The previous version of the patch failed that: >> >> (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for test int_param_single_reg_loc >> bt >> #0 0x000000000040058f in breakpt () >> -#1 0x00000000004005a2 in int_param_single_reg_loc (operand0=, operand1=0xdeadbe00deadbe01, operand2=) >> +#1 0x00000000004005a2 in int_param_single_reg_loc (operand0=, operand1=0xdeadbe00deadbe01, operand2=) >> #2 0x0000000000400577 in main () >> >> It didn't really make a lot of sense. This new version doesn't have >> that change anymore. >> >> That simple rule suggests that whatever the internal representation, >> we should be easily able to have a single central point where to tag >> such values. In fact, I think that already exists in value_of_register. >> >>> However, exchanging what I see as the current larger inconsistency, for >>> this much smaller one seems like a good deal to me, especially if it >>> gets this patch unblocked... >> >> Alright, what do you (all) think of of this (supposedly finished) patch >> on top of yours (Andrew's) then? > > > Looks good to me. Thanks for this. Thanks. Could you apply your patch then? -- Pedro Alves