From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28313 invoked by alias); 24 Jan 2008 22:19:32 -0000 Received: (qmail 28298 invoked by uid 22791); 24 Jan 2008 22:19:31 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 24 Jan 2008 22:19:02 +0000 Received: from kahikatea.snap.net.nz (232.60.255.123.dynamic.snap.net.nz [123.255.60.232]) by viper.snap.net.nz (Postfix) with ESMTP id E55903DA5D3; Fri, 25 Jan 2008 11:18:53 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id DCFE48FC6D; Fri, 25 Jan 2008 11:18:46 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18329.3782.282262.175798@kahikatea.snap.net.nz> Date: Thu, 24 Jan 2008 22:24:00 -0000 To: Vladimir Prus , gdb-patches@sources.redhat.com Subject: Re: [PATCH] Variable objects in multi-threaded programs In-Reply-To: <18328.16293.286454.235065@kahikatea.snap.net.nz> References: <18328.5277.85018.374650@kahikatea.snap.net.nz> <18328.16293.286454.235065@kahikatea.snap.net.nz> X-Mailer: VM 7.19 under Emacs 23.0.50.36 X-IsSubscribed: yes 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 X-SW-Source: 2008-01/txt/msg00596.txt.bz2 > > I'm somewhat concerned that this patch makes gdb traverse stacks of all > > threads -- it can take quite some time. Would a better solution be > > to store thread id inside varobj? > > Yes I think it would. As an element of struct varobj_root? > > Perhaps this should also included as a field in the output of -var-create, > the frontend could organise the display of watch expressions by thread. If > we use the GDB thread id, this would be consistent with infrun.c Here's a revised patch along those lines. The diffs for gdbthread.h, thread.c are as before, that of varobj.c is new and there are now ones for mi-cmd-var.c and varobj.h. -- Nick http://www.inet.net.nz/~nickrob 2008-01-25 Nick Roberts * thread.c (make_cleanup_restore_current_thread) (do_restore_current_thread_cleanup): Don't make static (struct current_thread_cleanup): Move to gdbthread.h * gdbthread.h: Declare above functions as externs here. * varobj.c (struct varobj_root): New component thread_id. (varobj_create): Set it to -2 for global variables. (varobj_get_thread_id, check_scope): New functions. (c_value_of_root): Use it to iterate over threads. * varobj.h (varobj_get_thread_id): New extern. * mi/mi-cmd-var.c (print_varobj): Add thread-id field. *** varobj.c.~1.99.~ 2008-01-04 10:24:29.000000000 +1300 --- varobj.c 2008-01-25 10:50:06.000000000 +1300 *************** *** 24,30 **** --- 24,32 ---- #include "language.h" #include "wrapper.h" #include "gdbcmd.h" + #include "gdbthread.h" #include "block.h" + #include "inferior.h" #include "gdb_assert.h" #include "gdb_string.h" *************** struct varobj_root *** 65,70 **** --- 67,75 ---- /* The frame for this expression */ struct frame_id frame; + /* GDB thread id. */ + int thread_id; + /* If 1, "update" always recomputes the frame & valid block using the currently selected frame. */ int use_selected_frame; *************** varobj_create (char *objname, *** 492,497 **** --- 497,507 ---- var->format = variable_default_display (var); var->root->valid_block = innermost_block; + if (innermost_block) + var->root->thread_id = pid_to_thread_id (inferior_ptid); + else + /* Give global variables a thread_id of -2. */ + var->root->thread_id = -2; expr_len = strlen (expression); var->name = savestring (expression, expr_len); /* For a root var, the name and the expr are the same. */ *************** varobj_get_display_format (struct varobj *** 686,691 **** --- 696,707 ---- return var->format; } + int + varobj_get_thread_id (struct varobj *var) + { + return var->root->thread_id; + } + void varobj_set_frozen (struct varobj *var, int frozen) { *************** c_path_expr_of_child (struct varobj *chi *** 2153,2189 **** return child->path_expr; } static struct value * c_value_of_root (struct varobj **var_handle) { struct value *new_val = NULL; struct varobj *var = *var_handle; ! struct frame_info *fi; ! int within_scope; /* 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 { ! fi = frame_find_by_id (var->root->frame); ! within_scope = fi != NULL; ! /* FIXME: select_frame could fail */ ! if (fi) { ! CORE_ADDR pc = get_frame_pc (fi); ! if (pc < BLOCK_START (var->root->valid_block) || ! pc >= BLOCK_END (var->root->valid_block)) ! within_scope = 0; ! else ! select_frame (fi); ! } } if (within_scope) --- 2169,2229 ---- return child->path_expr; } + static void + check_scope (struct varobj *var, int* scope) + { + struct frame_info *fi; + + fi = frame_find_by_id (var->root->frame); + *scope = fi != NULL; + + /* FIXME: select_frame could fail */ + if (fi) + { + CORE_ADDR pc = get_frame_pc (fi); + if (pc < BLOCK_START (var->root->valid_block) || + pc >= BLOCK_END (var->root->valid_block)) + *scope = 0; + else + select_frame (fi); + } + } + 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) ! check_scope (var, &within_scope); ! 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); ! check_scope (var, &within_scope); ! } ! else ! var->root->thread_id = -1; } if (within_scope) *************** c_value_of_root (struct varobj **var_han *** 2194,2199 **** --- 2234,2241 ---- return new_val; } + do_cleanups (old_cleanups); + return NULL; } *** thread.c.~1.59.~ 2008-01-24 09:47:31.000000000 +1300 --- thread.c 2008-01-24 15:38:41.000000000 +1300 *************** static void info_threads_command (char * *** 60,67 **** static void thread_apply_command (char *, int); static void restore_current_thread (ptid_t); static void prune_threads (void); - static struct cleanup *make_cleanup_restore_current_thread (ptid_t, - struct frame_id); void delete_step_resume_breakpoint (void *arg) --- 60,65 ---- *************** restore_selected_frame (struct frame_id *** 496,508 **** } } ! struct current_thread_cleanup ! { ! ptid_t inferior_ptid; ! struct frame_id selected_frame_id; ! }; ! ! static void do_restore_current_thread_cleanup (void *arg) { struct current_thread_cleanup *old = arg; --- 494,500 ---- } } ! void do_restore_current_thread_cleanup (void *arg) { struct current_thread_cleanup *old = arg; *************** do_restore_current_thread_cleanup (void *** 511,517 **** xfree (old); } ! static struct cleanup * make_cleanup_restore_current_thread (ptid_t inferior_ptid, struct frame_id a_frame_id) { --- 503,509 ---- xfree (old); } ! struct cleanup * make_cleanup_restore_current_thread (ptid_t inferior_ptid, struct frame_id a_frame_id) { *** gdbthread.h.~1.19.~ 2008-01-24 09:47:30.000000000 +1300 --- gdbthread.h 2008-01-24 15:37:19.000000000 +1300 *************** extern void load_infrun_state (ptid_t pt *** 143,148 **** --- 143,159 ---- /* Switch from one thread to another. */ extern void switch_to_thread (ptid_t ptid); + struct current_thread_cleanup + { + ptid_t inferior_ptid; + struct frame_id selected_frame_id; + }; + + extern void do_restore_current_thread_cleanup (void *arg); + + extern struct cleanup * make_cleanup_restore_current_thread + (ptid_t inferior_ptid, struct frame_id a_frame_id); + /* Commands with a prefix of `thread'. */ extern struct cmd_list_element *thread_cmd_list; *** varobj.h.~1.14.~ 2008-01-04 10:24:30.000000000 +1300 --- varobj.h 2008-01-24 22:09:17.000000000 +1300 *************** extern enum varobj_display_formats varob *** 85,90 **** --- 85,92 ---- extern enum varobj_display_formats varobj_get_display_format ( struct varobj *var); + extern int varobj_get_thread_id (struct varobj *var); + extern void varobj_set_frozen (struct varobj *var, int frozen); extern int varobj_get_frozen (struct varobj *var); *** mi-cmd-var.c.~1.44.~ 2008-01-23 16:19:39.000000000 +1300 --- mi-cmd-var.c 2008-01-25 00:43:22.000000000 +1300 *************** print_varobj (struct varobj *var, enum p *** 50,55 **** --- 50,56 ---- { struct type *gdb_type; char *type; + int thread_id; ui_out_field_string (uiout, "name", varobj_get_objname (var)); if (print_expression) *************** print_varobj (struct varobj *var, enum p *** 66,71 **** --- 67,76 ---- xfree (type); } + thread_id = varobj_get_thread_id (var); + if (thread_id != -2) + ui_out_field_int (uiout, "thread-id", thread_id); + if (varobj_get_frozen (var)) ui_out_field_int (uiout, "frozen", 1); }