From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9523 invoked by alias); 5 Sep 2013 16:35:51 -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 9509 invoked by uid 89); 5 Sep 2013 16:35:50 -0000 Received: from mms2.broadcom.com (HELO mms2.broadcom.com) (216.31.210.18) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Sep 2013 16:35:50 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RDNS_NONE autolearn=no version=3.3.2 X-HELO: mms2.broadcom.com Received: from [10.9.208.53] by mms2.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Thu, 05 Sep 2013 09:29:18 -0700 X-Server-Uuid: 4500596E-606A-40F9-852D-14843D8201B2 Received: from IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) by IRVEXCHCAS06.corp.ad.broadcom.com (10.9.208.53) with Microsoft SMTP Server (TLS) id 14.1.438.0; Thu, 5 Sep 2013 09:35:37 -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; Thu, 5 Sep 2013 09:35:37 -0700 Received: from [10.177.73.74] (unknown [10.177.73.74]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id D9F2C1A46; Thu, 5 Sep 2013 09:35:36 -0700 (PDT) Message-ID: <5228B2D8.7060604@broadcom.com> Date: Thu, 05 Sep 2013 16:35: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> In-Reply-To: <5228B15F.7060108@redhat.com> Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2013-09/txt/msg00178.txt.bz2 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. Andrew