From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94734 invoked by alias); 8 Jul 2015 13:37:54 -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 94722 invoked by uid 89); 8 Jul 2015 13:37:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-ob0-f171.google.com Received: from mail-ob0-f171.google.com (HELO mail-ob0-f171.google.com) (209.85.214.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 08 Jul 2015 13:37:52 +0000 Received: by obbop1 with SMTP id op1so150534452obb.2 for ; Wed, 08 Jul 2015 06:37:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=NIEvRpIwNm1c7B/+KcuTGS6M+p3gxBWphKWvv5sN2XA=; b=XDsGI2FcPADm+PDsCdDGMw+Y6aBJgsEivpUFC9XKB/dFEr7E4xErR6jFya47VqoEfv wNvfOKXXuVFwUzGigHsHmdU5prycCBvxd7Tyi15x/OMI0KyXc6EoZ4zp43sCXndVrwyH 6vwyvZhfFnTHDwTKAUeFwPyKHd6yAbH7jCcFJrw4kHGuEcWK4V6XWtuxRePLGCJ1Zljt k6dSal1IK+Bvl0yTTOiVCwrktNGm1ykZArfiBafI7cCW9dbGDSNVNQpzf4oKuJTunflq SSiYpEe4Fw2EkgF9LNowZYokVWfeNyngr3KLXA65e3wjBHSHQ5Jb/h39OWQlc/uLaDpS NS1w== X-Gm-Message-State: ALoCoQnkvk08RwePZbDMNHIxkToN9kWzURgma4bdD0yTr4nLLbqB2/1xg9erF2H9dpsEXbqlmlOy X-Received: by 10.60.63.20 with SMTP id c20mr9597858oes.22.1436362670585; Wed, 08 Jul 2015 06:37:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.96.167 with HTTP; Wed, 8 Jul 2015 06:37:30 -0700 (PDT) In-Reply-To: <559D1C18.4070008@redhat.com> References: <1436145432-6502-1-git-send-email-patrick@parcs.ath.cx> <559D0C63.3000200@redhat.com> <559D1C18.4070008@redhat.com> From: Patrick Palka Date: Wed, 08 Jul 2015 13:37:00 -0000 Message-ID: Subject: Re: [PATCH] tui: replace deprecated_register_changed_hook with observer To: Pedro Alves Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-07/txt/msg00209.txt.bz2 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 d= own >>>> to tui_check_data_values instead of the frame returned by >>>> get_selected_frame. The frame argument passed to each register_chan= ged >>>> observer corresponds to the VALUE_FRAME_ID of the register being >>>> modified within a register assignment, e.g. the $rax in "print $rax = =3D >>>> 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 =3D 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 =3D=3D 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=3D"var1",numchild=3D"0",value=3D"6295640",type=3D"int64_t",= has_more=3D"0" >>> (gdb) up >>> #1 0x000000000040082a in thread_function0 (arg=3D0x0) at threads.c:69 >>> 69 usleep (1); /* Loop increment. */ >>> (gdb) up >>> #2 0x0000003616a07ee5 in start_thread (arg=3D0x7ffff7fc1700) at pthre= ad_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 =3D=3D 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 Pyt= hon, >>> 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 !=3D >> 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 !=3D >> 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? 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 =3D 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 !=3D get_selected_frame ()" check is 100% correct.