From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 56073 invoked by alias); 8 Jul 2015 13:52:36 -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 56064 invoked by uid 89); 8 Jul 2015 13:52:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 08 Jul 2015 13:52:35 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id E9828AB7; Wed, 8 Jul 2015 13:52:33 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t68DqWmS016394; Wed, 8 Jul 2015 09:52:32 -0400 Message-ID: <559D2B1F.1080503@redhat.com> Date: Wed, 08 Jul 2015 13:52:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Patrick Palka CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH] tui: replace deprecated_register_changed_hook with observer References: <1436145432-6502-1-git-send-email-patrick@parcs.ath.cx> <559D0C63.3000200@redhat.com> <559D1C18.4070008@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-07/txt/msg00214.txt.bz2 On 07/08/2015 02:37 PM, Patrick Palka wrote: > On Wed, Jul 8, 2015 at 8:48 AM, Pedro Alves wrote: >> On 07/08/2015 01:30 PM, Patrick Palka wrote: >>> On Wed, Jul 8, 2015 at 7:41 AM, Pedro Alves wrote: >>>> On 07/06/2015 02:17 AM, Patrick Palka wrote: >>>>> This is a straightforward replacement of the TUI's use of the >>>>> aforementioned hook with the register_changed observer. Since this was >>>>> the only user of the hook, this patch also removes the hook. >>>>> >>>>> [ I am not sure if the changes to the function tui_register_changed are >>>>> correct. In particular, the inputted frame argument is now passed down >>>>> to tui_check_data_values instead of the frame returned by >>>>> get_selected_frame. The frame argument passed to each register_changed >>>>> observer corresponds to the VALUE_FRAME_ID of the register being >>>>> modified within a register assignment, e.g. the $rax in "print $rax = >>>>> FOO". When would the frame corresponding to the VALUE_FRAME_ID of a >>>>> register not be the currently selected frame? ] >>>>> >>>> >>>> Grepping for value_assign callers finds e.g., varobjs: >>>> >>>> varobj.c: val = value_assign (var->value, value); >>>> >>>> Adding an assertion like this: >>>> >>>> @@ -1169,6 +1169,7 @@ value_assign (struct value *toval, struct value *fromval) >>>> } >>>> } >>>> >>>> + gdb_assert (frame == get_selected_frame (NULL)); >>>> observer_notify_register_changed (frame, value_reg); >>>> if (deprecated_register_changed_hook) >>>> deprecated_register_changed_hook (-1); >>>> >>>> and playing with varobjs shows the assertion failing: >>>> >>>> (gdb) interpreter-exec mi "-var-create - * $rax" >>>> ^done,name="var1",numchild="0",value="6295640",type="int64_t",has_more="0" >>>> (gdb) up >>>> #1 0x000000000040082a in thread_function0 (arg=0x0) at threads.c:69 >>>> 69 usleep (1); /* Loop increment. */ >>>> (gdb) up >>>> #2 0x0000003616a07ee5 in start_thread (arg=0x7ffff7fc1700) at pthread_create.c:309 >>>> 309 THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd)); >>>> (gdb) interpreter-exec mi "-var-assign var1 1" >>>> ~"/home/pedro/gdb/mygit/build/../src/gdb/valops.c:1172: internal-error: value_assign: Assertion `frame == get_selected_frame (NULL)' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) " >>>> >>>> The TUI doesn't use MI, but there are probably other similar cases >>>> in the tree. E.g., I'd assume you can create a register Value with Python, >>>> and then assign to it when the selected frame is not >>>> the register's frame. >>> >>> Ah okay.. So it seems to me that if the frame argument != >>> get_selected_frame, then we should not update the register window at >>> all since the register window is supposed to show the register values >>> of the currently selected frame. >> >> Yes, I think so. >> >>> Or instead, just ignore the frame argument and always pass >>> get_selected_frame to tui_check_data_values, even if frame != >>> get_selected_frame. Seems to me that this is the safest option. >> >> That'd be a 1-1 with the current code. Though, I believe >> that results in spuriously clearing the highlight of >> previously changed registers (of the selected frame), because >> nothing will have changed. So seems like the other option >> actually fixes a bug. > > Is it actually the case that a register change made on one frame can > not show up on some other frame? Oh, you're right, good point. Registers can well be at the same physical location across frames. > > If I debug gdb with gdb, doing "start" followed by "step" a couple > dozen times, do "layout regs", then select the outermost frame and do > "print $rbx = 50", the regs window shows that $rbx has not changed on > the (selected) outermost frame but if i select the innermost frame, > $rbx has changed to 50. And the frame_id of the register $rbx was > indeed the (selected) outermost frame, yet the registers of the > selected frame did not change after the value assignment and the > registers of some other frame did. I don't know why this particular > example behaves this way, but it seems to illustrates that it's > possible that a register change made in one frame can affect the > register values of another frame. So I don't know if the "frame != > get_selected_frame ()" check is 100% correct. > Yeah. Sorry, somehow forgot this... Thanks, Pedro Alves