From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32105 invoked by alias); 14 Feb 2011 11:46:20 -0000 Received: (qmail 32092 invoked by uid 22791); 14 Feb 2011 11:46:14 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_CP,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Feb 2011 11:46:04 +0000 Received: (qmail 28465 invoked from network); 14 Feb 2011 11:46:02 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Feb 2011 11:46:02 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [unavailable values part 1, 05/17] move traceframe memory reading fallback to read-only sections to GDB side Date: Mon, 14 Feb 2011 11:46:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.35-25-generic; KDE/4.6.0; x86_64; ; ) References: <201102071430.29773.pedro@codesourcery.com> In-Reply-To: <201102071430.29773.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201102141145.59206.pedro@codesourcery.com> 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: 2011-02/txt/msg00257.txt.bz2 Here's the version I committed. The difference to the previous version I had posted is that this one brings back a bit of code I was removing, like so: Index: src/gdb/tracepoint.c =================================================================== --- src.orig/gdb/tracepoint.c 2011-02-11 15:51:35.667503995 +0000 +++ src/gdb/tracepoint.c 2011-02-11 15:53:28.127504002 +0000 @@ -2931,11 +2931,20 @@ get_traceframe_number (void) return traceframe_number; } +/* Make the traceframe NUM be the current trace frame. Does nothing + if NUM is already current. */ + void set_current_traceframe (int num) { int newnum; + if (traceframe_number == num) + { + /* Nothing to do. */ + return; + } + newnum = target_trace_find (tfind_number, num, 0, 0, NULL); if (newnum != num) Because without that, "quit" would fail against targets that don't support tracepoints, while trying to select the current traceframe as -1 (that is, trying to make sure we're out of tfind mode). target_trace_find would throw an error. -- Pedro Alves 2011-02-14 Pedro Alves gdb/ * target.c (target_read_live_memory): New function. (memory_xfer_live_readonly_partial): New. (memory_xfer_partial): If reading from a traceframe, fallback to reading unavailable read-only memory from read-only regions of live target memory. * tracepoint.c (disconnect_tracing): Adjust. (set_current_traceframe): New, factored out from set_traceframe_number. (set_traceframe_number): Reimplement to only change the traceframe number on the GDB side. (do_restore_current_traceframe_cleanup): Adjust. (make_cleanup_restore_traceframe_number): New. (cur_traceframe_number): New global. (tfile_open): Set cur_traceframe_number to no traceframe. (set_tfile_traceframe): New function. (tfile_trace_find): If looking up a traceframe using any method other than by number, make sure the current tfile traceframe matches gdb's current traceframe. Update the current tfile traceframe if the lookup succeeded. (tfile_fetch_registers, tfile_xfer_partial) (tfile_get_trace_state_variable_value): Make sure the remote traceframe matches gdb's current traceframe. * remote.c (remote_traceframe_number): New global. (remote_open_1): Set it to -1. (set_remote_traceframe): New function. (remote_fetch_registers, remote_store_registers) (remote_xfer_memory, remote_xfer_partial) (remote_get_trace_state_variable_value): Make sure the remote traceframe matches gdb's current traceframe. (remote_trace_find): If looking up a traceframe using any method other than by number, make sure the current remote traceframe matches gdb's current traceframe. Update the current remote traceframe if the lookup succeeded. * infrun.c (fetch_inferior_event): Adjust. * tracepoint.h (set_current_traceframe): Declare. (get_traceframe_number, set_traceframe_number): Add describing comments. --- gdb/infrun.c | 2 gdb/remote.c | 43 ++++++++++++++++++ gdb/target.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/tracepoint.c | 61 ++++++++++++++++++++++++-- gdb/tracepoint.h | 19 +++++++- 5 files changed, 247 insertions(+), 7 deletions(-) Index: src/gdb/target.c =================================================================== --- src.orig/gdb/target.c 2011-02-14 10:49:24.000000000 +0000 +++ src/gdb/target.c 2011-02-14 10:49:26.551772004 +0000 @@ -1271,6 +1271,82 @@ target_section_by_addr (struct target_op return NULL; } +/* Read memory from the live target, even if currently inspecting a + traceframe. The return is the same as that of target_read. */ + +static LONGEST +target_read_live_memory (enum target_object object, + ULONGEST memaddr, gdb_byte *myaddr, LONGEST len) +{ + int ret; + struct cleanup *cleanup; + + /* Switch momentarily out of tfind mode so to access live memory. + Note that this must not clear global state, such as the frame + cache, which must still remain valid for the previous traceframe. + We may be _building_ the frame cache at this point. */ + cleanup = make_cleanup_restore_traceframe_number (); + set_traceframe_number (-1); + + ret = target_read (current_target.beneath, object, NULL, + myaddr, memaddr, len); + + do_cleanups (cleanup); + return ret; +} + +/* Using the set of read-only target sections of OPS, read live + read-only memory. Note that the actual reads start from the + top-most target again. */ + +static LONGEST +memory_xfer_live_readonly_partial (struct target_ops *ops, + enum target_object object, + gdb_byte *readbuf, ULONGEST memaddr, + LONGEST len) +{ + struct target_section *secp; + struct target_section_table *table; + + secp = target_section_by_addr (ops, memaddr); + if (secp != NULL + && (bfd_get_section_flags (secp->bfd, secp->the_bfd_section) + & SEC_READONLY)) + { + struct target_section *p; + ULONGEST memend = memaddr + len; + + table = target_get_section_table (ops); + + for (p = table->sections; p < table->sections_end; p++) + { + if (memaddr >= p->addr) + { + if (memend <= p->endaddr) + { + /* Entire transfer is within this section. */ + return target_read_live_memory (object, memaddr, + readbuf, len); + } + else if (memaddr >= p->endaddr) + { + /* This section ends before the transfer starts. */ + continue; + } + else + { + /* This section overlaps the transfer. Just do half. */ + len = p->endaddr - memaddr; + return target_read_live_memory (object, memaddr, + readbuf, len); + } + } + } + } + + return 0; +} + /* Perform a partial memory transfer. For docs see target.h, to_xfer_partial. */ @@ -1329,6 +1405,59 @@ memory_xfer_partial (struct target_ops * } } + /* If reading unavailable memory in the context of traceframes, and + this address falls within a read-only section, fallback to + reading from live memory. */ + if (readbuf != NULL && get_traceframe_number () != -1) + { + VEC(mem_range_s) *available; + + /* If we fail to get the set of available memory, then the + target does not support querying traceframe info, and so we + attempt reading from the traceframe anyway (assuming the + target implements the old QTro packet then). */ + if (traceframe_available_memory (&available, memaddr, len)) + { + struct cleanup *old_chain; + + old_chain = make_cleanup (VEC_cleanup(mem_range_s), &available); + + if (VEC_empty (mem_range_s, available) + || VEC_index (mem_range_s, available, 0)->start != memaddr) + { + /* Don't read into the traceframe's available + memory. */ + if (!VEC_empty (mem_range_s, available)) + { + LONGEST oldlen = len; + + len = VEC_index (mem_range_s, available, 0)->start - memaddr; + gdb_assert (len <= oldlen); + } + + do_cleanups (old_chain); + + /* This goes through the topmost target again. */ + res = memory_xfer_live_readonly_partial (ops, object, + readbuf, memaddr, len); + if (res > 0) + return res; + + /* No use trying further, we know some memory starting + at MEMADDR isn't available. */ + return -1; + } + + /* Don't try to read more than how much is available, in + case the target implements the deprecated QTro packet to + cater for older GDBs (the target's knowledge of read-only + sections may be outdated by now). */ + len = VEC_index (mem_range_s, available, 0)->length; + + do_cleanups (old_chain); + } + } + /* Try GDB's internal data cache. */ region = lookup_mem_region (memaddr); /* region->hi == 0 means there's no upper bound. */ Index: src/gdb/tracepoint.c =================================================================== --- src.orig/gdb/tracepoint.c 2011-02-14 10:49:24.000000000 +0000 +++ src/gdb/tracepoint.c 2011-02-14 11:04:39.111772004 +0000 @@ -1927,7 +1927,7 @@ disconnect_tracing (int from_tty) confusing upon reconnection. Just use these calls instead of full tfind_1 behavior because we're in the middle of detaching, and there's no point to updating current stack frame etc. */ - set_traceframe_number (-1); + set_current_traceframe (-1); set_traceframe_context (NULL); } @@ -2935,7 +2935,7 @@ get_traceframe_number (void) if NUM is already current. */ void -set_traceframe_number (int num) +set_current_traceframe (int num) { int newnum; @@ -2959,6 +2959,15 @@ set_traceframe_number (int num) clear_traceframe_info (); } +/* Make the traceframe NUM be the current trace frame, and do nothing + more. */ + +void +set_traceframe_number (int num) +{ + traceframe_number = num; +} + /* A cleanup used when switching away and back from tfind mode. */ struct current_traceframe_cleanup @@ -2972,7 +2981,7 @@ do_restore_current_traceframe_cleanup (v { struct current_traceframe_cleanup *old = arg; - set_traceframe_number (old->traceframe_number); + set_current_traceframe (old->traceframe_number); } static void @@ -2995,6 +3004,12 @@ make_cleanup_restore_current_traceframe restore_current_traceframe_cleanup_dtor); } +struct cleanup * +make_cleanup_restore_traceframe_number (void) +{ + return make_cleanup_restore_integer (&traceframe_number); +} + /* Given a number and address, return an uploaded tracepoint with that number, creating if necessary. */ @@ -3247,6 +3262,7 @@ char *trace_filename; int trace_fd = -1; off_t trace_frames_offset; off_t cur_offset; +int cur_traceframe_number; int cur_data_size; int trace_regblock_size; @@ -3338,6 +3354,8 @@ tfile_open (char *filename, int from_tty ts->disconnected_tracing = 0; ts->circular_buffer = 0; + cur_traceframe_number = -1; + /* Read through a section of newline-terminated lines that define things like tracepoints. */ i = 0; @@ -3752,6 +3770,28 @@ tfile_get_traceframe_address (off_t tfra return addr; } +/* Make tfile's selected traceframe match GDB's selected + traceframe. */ + +static void +set_tfile_traceframe (void) +{ + int newnum; + + if (cur_traceframe_number == get_traceframe_number ()) + return; + + /* Avoid recursion, tfile_trace_find calls us again. */ + cur_traceframe_number = get_traceframe_number (); + + newnum = target_trace_find (tfind_number, + get_traceframe_number (), 0, 0, NULL); + + /* Should not happen. If it does, all bets are off. */ + if (newnum != get_traceframe_number ()) + warning (_("could not set tfile's traceframe")); +} + /* Given a type of search and some parameters, scan the collection of traceframes in the file looking for a match. When found, return both the traceframe and tracepoint number, otherwise -1 for @@ -3768,6 +3808,12 @@ tfile_trace_find (enum trace_find_type t off_t offset, tframe_offset; ULONGEST tfaddr; + /* Lookups other than by absolute frame number depend on the current + trace selected, so make sure it is correct on the tfile end + first. */ + if (type != tfind_number) + set_tfile_traceframe (); + lseek (trace_fd, trace_frames_offset, SEEK_SET); offset = trace_frames_offset; while (1) @@ -3820,6 +3866,7 @@ tfile_trace_find (enum trace_find_type t *tpp = tpnum; cur_offset = offset; cur_data_size = data_size; + cur_traceframe_number = tfnum; return tfnum; } /* Skip past the traceframe's data. */ @@ -3936,6 +3983,8 @@ tfile_fetch_registers (struct target_ops if (!trace_regblock_size) return; + set_tfile_traceframe (); + regs = alloca (trace_regblock_size); if (traceframe_find_block_type ('R', 0) >= 0) @@ -4019,7 +4068,9 @@ tfile_xfer_partial (struct target_ops *o if (readbuf == NULL) error (_("tfile_xfer_partial: trace file is read-only")); - if (traceframe_number != -1) + set_tfile_traceframe (); + + if (traceframe_number != -1) { int pos = 0; @@ -4102,6 +4153,8 @@ tfile_get_trace_state_variable_value (in { int pos; + set_tfile_traceframe (); + pos = 0; while ((pos = traceframe_find_block_type ('V', pos)) >= 0) { Index: src/gdb/remote.c =================================================================== --- src.orig/gdb/remote.c 2011-02-14 10:49:24.000000000 +0000 +++ src/gdb/remote.c 2011-02-14 10:49:26.601772004 +0000 @@ -1353,6 +1353,10 @@ static ptid_t any_thread_ptid; static ptid_t general_thread; static ptid_t continue_thread; +/* This the traceframe which we last selected on the remote system. + It will be -1 if no traceframe is selected. */ +static int remote_traceframe_number = -1; + /* Find out if the stub attached to PID (and hence GDB should offer to detach instead of killing it when bailing out). */ @@ -4002,6 +4006,7 @@ remote_open_1 (char *name, int from_tty, general_thread = not_sent_ptid; continue_thread = not_sent_ptid; + remote_traceframe_number = -1; /* Probe for ability to use "ThreadInfo" query, as required. */ use_threadinfo_query = 1; @@ -5770,6 +5775,28 @@ fetch_registers_using_g (struct regcache process_g_packet (regcache); } +/* Make the remote selected traceframe match GDB's selected + traceframe. */ + +static void +set_remote_traceframe (void) +{ + int newnum; + + if (remote_traceframe_number == get_traceframe_number ()) + return; + + /* Avoid recursion, remote_trace_find calls us again. */ + remote_traceframe_number = get_traceframe_number (); + + newnum = target_trace_find (tfind_number, + get_traceframe_number (), 0, 0, NULL); + + /* Should not happen. If it does, all bets are off. */ + if (newnum != get_traceframe_number ()) + warning (_("could not set remote traceframe")); +} + static void remote_fetch_registers (struct target_ops *ops, struct regcache *regcache, int regnum) @@ -5777,6 +5804,7 @@ remote_fetch_registers (struct target_op struct remote_arch_state *rsa = get_remote_arch_state (); int i; + set_remote_traceframe (); set_general_thread (inferior_ptid); if (regnum >= 0) @@ -5934,6 +5962,7 @@ remote_store_registers (struct target_op struct remote_arch_state *rsa = get_remote_arch_state (); int i; + set_remote_traceframe (); set_general_thread (inferior_ptid); if (regnum >= 0) @@ -6502,6 +6531,7 @@ remote_xfer_memory (CORE_ADDR mem_addr, { int res; + set_remote_traceframe (); set_general_thread (inferior_ptid); if (should_write) @@ -8084,6 +8114,7 @@ remote_xfer_partial (struct target_ops * char *p2; char query_type; + set_remote_traceframe (); set_general_thread (inferior_ptid); rs = get_remote_state (); @@ -9972,6 +10003,12 @@ remote_trace_find (enum trace_find_type char *p, *reply; int target_frameno = -1, target_tracept = -1; + /* Lookups other than by absolute frame number depend on the current + trace selected, so make sure it is correct on the remote end + first. */ + if (type != tfind_number) + set_remote_traceframe (); + p = rs->buf; strcpy (p, "QTFrame:"); p = strchr (p, '\0'); @@ -10009,6 +10046,8 @@ remote_trace_find (enum trace_find_type target_frameno = (int) strtol (p, &reply, 16); if (reply == p) error (_("Unable to parse trace frame number")); + /* Don't update our remote traceframe number cache on failure + to select a remote traceframe. */ if (target_frameno == -1) return -1; break; @@ -10029,6 +10068,8 @@ remote_trace_find (enum trace_find_type } if (tpp) *tpp = target_tracept; + + remote_traceframe_number = target_frameno; return target_frameno; } @@ -10039,6 +10080,8 @@ remote_get_trace_state_variable_value (i char *reply; ULONGEST uval; + set_remote_traceframe (); + sprintf (rs->buf, "qTV:%x", tsvnum); putpkt (rs->buf); reply = remote_get_noisy_reply (&target_buf, &target_buf_size); Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2011-02-14 10:46:42.000000000 +0000 +++ src/gdb/infrun.c 2011-02-14 10:49:26.611772005 +0000 @@ -2631,7 +2631,7 @@ fetch_inferior_event (void *client_data) if (non_stop) { make_cleanup_restore_current_traceframe (); - set_traceframe_number (-1); + set_current_traceframe (-1); } if (non_stop) Index: src/gdb/tracepoint.h =================================================================== --- src.orig/gdb/tracepoint.h 2011-02-14 10:49:24.000000000 +0000 +++ src/gdb/tracepoint.h 2011-02-14 10:49:26.611772005 +0000 @@ -192,9 +192,24 @@ extern void release_static_tracepoint_ma extern void (*deprecated_trace_find_hook) (char *arg, int from_tty); extern void (*deprecated_trace_start_stop_hook) (int start, int from_tty); -int get_traceframe_number (void); -void set_traceframe_number (int); +/* Returns the current traceframe number. */ +extern int get_traceframe_number (void); + +/* Make the traceframe NUM be the current GDB trace frame number, and + do nothing more. In particular, this does not flush the + register/frame caches or notify the target about the trace frame + change, so that is can be used when we need to momentarily access + live memory. Targets lazily switch their current traceframe to + match GDB's traceframe number, at the appropriate times. */ +extern void set_traceframe_number (int); + +/* Make the traceframe NUM be the current trace frame, all the way to + the target, and flushes all global state (register/frame caches, + etc.). */ +extern void set_current_traceframe (int num); + struct cleanup *make_cleanup_restore_current_traceframe (void); +struct cleanup *make_cleanup_restore_traceframe_number (void); void free_actions (struct breakpoint *); extern void validate_actionline (char **, struct breakpoint *);