From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11058 invoked by alias); 18 Sep 2013 14:04:25 -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 11048 invoked by uid 89); 18 Sep 2013 14:04:24 -0000 Received: from mms1.broadcom.com (HELO mms1.broadcom.com) (216.31.210.17) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Sep 2013 14:04:24 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_05,KHOP_THREADED,RDNS_NONE autolearn=no version=3.3.2 X-HELO: mms1.broadcom.com Received: from [10.9.208.57] by mms1.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Wed, 18 Sep 2013 07:00:12 -0700 X-Server-Uuid: 06151B78-6688-425E-9DE2-57CB27892261 Received: from IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) by IRVEXCHCAS08.corp.ad.broadcom.com (10.9.208.57) with Microsoft SMTP Server (TLS) id 14.1.438.0; Wed, 18 Sep 2013 07:04:09 -0700 Received: from mail-irva-13.broadcom.com (10.10.10.20) by IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) with Microsoft SMTP Server id 14.1.438.0; Wed, 18 Sep 2013 07:04:10 -0700 Received: from [10.177.73.74] (unknown [10.177.73.74]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id 6D484246A3; Wed, 18 Sep 2013 07:04:09 -0700 (PDT) Message-ID: <5239B2D8.4030403@broadcom.com> Date: Wed, 18 Sep 2013 14:04:00 -0000 From: "Andrew Burgess" User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 MIME-Version: 1.0 To: "Pedro Alves" 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> <5237567C.8050406@redhat.com> In-Reply-To: <5237567C.8050406@redhat.com> Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2013-09/txt/msg00600.txt.bz2 On 16/09/2013 8:05 PM, Pedro Alves wrote: > 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? > Committed. Thanks for your time. Andrew