Re: [PATCH] tui: replace deprecated_register_changed_hook with observer

[Patrick Palka <patrick at parcs dot ath dot cx>]

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?

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.