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 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.

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.