Re: MI non-stop mode spec

[Vladimir Prus <vladimir@codesourcery.com>]

On Sunday 23 March 2008 07:40:17 Nick Roberts wrote:
>  > >  > >  > Didn't you check in a patch to make *-varobjs be found to a
>  > >  > >  > thread?
>  > >  > >
>  > >  > > I submitted a patch earlier this year that stopped thinking that
>  > >  > > a variable object had gone out of scope if the thread changed
>  > >  > > but nothing happened.
>  > >  >
>  > >  > Can you resend the current version of that patch, and I'll take a
>  > >  > look.
>  > >
>  > > I used pid_to_thread_id (inferior_ptid) for the thread_id, just as
>  > > infrun.c does for *stopped.  I think that the thread_id problem for
>  > > single/multi-threaded programs should be fixed first (perhaps along
>  > > the lines Daniel suggested).
>  > 
>  > Dan's patch is in, however, it only affects Linux. I suspect in might
>  > be a long time till the problem is solved on all system. Until then,
>  > we probably should assume that if thread list is empty, then the 
>  > program is ST and never will become MT.
>  > 
>  > I happen to be working on variable objects for non-stop now, so I'll
>  > play with the last version of your patch that you have posted.
> 
> I've made some changes since then so here's my latest patch updated to current
> sources.  I found that after all I did need thread_id == -2 for global
> variables (used in mi-cmd-var which doesn't print the thread_id field for them
> since their value is the same in all threads).

I don't think we need it. A global variable has a NULL valid_block, so we can
use that. I'll work on adjusting your patch this way.

>   static struct value *
>   c_value_of_root (struct varobj **var_handle)
>   {
>     struct value *new_val = NULL;
>     struct varobj *var = *var_handle;
> !   struct frame_id saved_frame_id;
> !   struct cleanup *old_cleanups = NULL;
> !   int within_scope, thread_id;
> !   ptid_t ptid;
>   
>     /*  Only root variables can be updated... */
>     if (!is_root_p (var))
>       /* Not a root var */
>       return NULL;
>   
>     /* Determine whether the variable is still around. */
>     if (var->root->valid_block == NULL || var->root->use_selected_frame)
>       within_scope = 1;
>     else
>       {
> !       thread_id = var->root->thread_id;
> !       ptid = thread_id_to_pid (thread_id);
> !       if (thread_id == 0)
> !       /* Single-threaded application.  */
> !       within_scope = check_scope (var);
> !       else if (thread_id != -1 && target_thread_alive (ptid))
>         {
> ! 
> !         saved_frame_id = get_frame_id (get_selected_frame (NULL));
> !         old_cleanups = make_cleanup_restore_current_thread (inferior_ptid,
> !                                                             saved_frame_id);
> !         switch_to_thread (ptid);
> !         within_scope = check_scope (var);
> !       }
> !       else
> !       /* Mark it as dead.  */
> !       var->root->thread_id = -1;
>       }
>   
>     if (within_scope)
> *************** c_value_of_root (struct varobj **var_han
> *** 2186,2191 ****
> --- 2233,2240 ----
>         return new_val;
>       }
>   
> +   do_cleanups (old_cleanups);
> + 
>     return NULL;
>   }

I think the use of cleanups above is wrong. You basically have:

	struct cleanups *old_cleanups = NULL;
	if (whatever)
		old_cleanups = ...
	do_cleanups (old_cleanups);

so, if 'whatever' evaluates to false, all cleanups, including those set
in parent, will be executed.

That's what we get for using a language that does not have exceptions
and proper destructors. I'll fix this too.

- Volodya