This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] tui: replace deprecated_register_changed_hook with observer
- From: Pedro Alves <palves at redhat dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 08 Jul 2015 14:52:31 +0100
- Subject: Re: [PATCH] tui: replace deprecated_register_changed_hook with observer
- Authentication-results: sourceware.org; auth=none
- References: <1436145432-6502-1-git-send-email-patrick at parcs dot ath dot cx> <559D0C63 dot 3000200 at redhat dot com> <CA+C-WL8xaUs2=e21O_Q1v6q4r2K092yGavXgg6AFBsw6kXeQTg at mail dot gmail dot com> <559D1C18 dot 4070008 at redhat dot com> <CA+C-WL_iEzo3oN55ow1B0XBcfJAnN+3ESUOcRMW-AckyvXmYKw at mail dot gmail dot com>
On 07/08/2015 02:37 PM, Patrick Palka wrote:
> On Wed, Jul 8, 2015 at 8:48 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 07/08/2015 01:30 PM, Patrick Palka wrote:
>>> On Wed, Jul 8, 2015 at 7:41 AM, Pedro Alves <palves@redhat.com> 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