From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10399 invoked by alias); 7 Feb 2011 14:30:44 -0000 Received: (qmail 10384 invoked by uid 22791); 7 Feb 2011 14:30:40 -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, 07 Feb 2011 14:30:34 +0000 Received: (qmail 25841 invoked from network); 7 Feb 2011 14:30:32 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 7 Feb 2011 14:30:32 -0000 To: gdb-patches@sourceware.org Subject: [unavailable values part 1, 05/17] move traceframe memory reading fallback to read-only sections to GDB side From: Pedro Alves Date: Mon, 07 Feb 2011 14:30:00 -0000 MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201102071430.29773.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/msg00136.txt.bz2 This moves the fall-back to read from read-only sections to GDB side. This completely emulates QTro on GDB's end, making the target side implementation unnecessary. That is, it makes GDB itself fall back to reading read-only memory from the live target (forcing a temporary switch out of tfind mode), if a given memory address we want to read has not been collected. This makes the set of read-only sections always match 100% that used by the new read_value_memory function from patch 4, which was what make me want to fix this. Having GDB do the fallback is more correct than having the target do it using the readonly section list GDB gave the target at "tstart" time with the QTro packet, because in the presence of dynamically loaded modules, the read-only sections the target knows about (as given by GDB earlier with QTro) may not match the current read-only sections of the live target, at the time the user is inspecting a traceframe. This implementation also does _not_ assume a single transfer can _not_ cover half-trace + half-live memory, unlike gdbserver's. Note that the switching out and back of tfind mode is lazy in the patch below. The common code only switches "traceframe_number", and both the remote and the tfile targets make sure the target end's current traceframe matches common GDB's, at appropriate times. This avoids many unnecessary switching between tfind mode off and on when, for example, disassembling, where you have several reads from live memory in a row. We already use the same technique to avoid a switch of the current thread in common code immediately causing a switch of the remote current thread (remote.c:set_general_thread). With this patch, if the target supports the new qXfer:traceframe-info:read packet, GDB will never even attempt reading memory that has not been collected in the traceframe, so effectively, the QTro handling code in the remote end is never exercised. This allows deprecating the QTro packet, but at the same time, keep sending it to (older) stubs (that will still need it), and allows stubs to continue implementing the packet if they want their tracepoint support to work with earlier GDBs. -- Pedro Alves 2011-02-07 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 (proceed): 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 | 70 ++++++++++++++++++++++++----- gdb/tracepoint.h | 19 +++++++- 5 files changed, 247 insertions(+), 16 deletions(-) Index: src/gdb/target.c =================================================================== --- src.orig/gdb/target.c 2011-02-07 11:14:48.146706001 +0000 +++ src/gdb/target.c 2011-02-07 11:15:14.186706002 +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-07 11:14:54.026705999 +0000 +++ src/gdb/tracepoint.c 2011-02-07 11:15:14.186706002 +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); } @@ -2931,20 +2931,11 @@ 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_traceframe_number (int num) +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) @@ -2959,6 +2950,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 +2972,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 +2995,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 +3253,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 +3345,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 +3761,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 +3799,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 +3857,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 +3974,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 +4059,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 +4144,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-07 11:14:48.166706003 +0000 +++ src/gdb/remote.c 2011-02-07 11:15:14.196706003 +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-02 18:58:36.277899003 +0000 +++ src/gdb/infrun.c 2011-02-07 11:15:14.206706005 +0000 @@ -2001,7 +2001,7 @@ proceed (CORE_ADDR addr, enum target_sig 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-07 11:14:54.026705999 +0000 +++ src/gdb/tracepoint.h 2011-02-07 11:15:14.206706005 +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 *);