From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31222 invoked by alias); 20 Mar 2013 10:10:27 -0000 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 Received: (qmail 31190 invoked by uid 89); 20 Mar 2013 10:10:17 -0000 X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 20 Mar 2013 10:10:14 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1UIFyH-00063i-7w from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Wed, 20 Mar 2013 03:10:13 -0700 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Wed, 20 Mar 2013 03:10:12 -0700 Received: from qiyao.dyndns.org.dyndns.org (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Wed, 20 Mar 2013 03:10:12 -0700 From: Yao Qi To: Subject: [PATCH] Rely on beneath to do partial xfer from tfile target Date: Wed, 20 Mar 2013 11:40:00 -0000 Message-ID: <1363774156-31949-1-git-send-email-yao@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-03/txt/msg00731.txt.bz2 When I examine the target API usage in both tfile target and ctf target, I find tfile_xfer_partial does something unnecessary at the bottom half of it. It looks at executable for read-only pieces. However, isn't it better to delegate this to beneath target (exec target or remote target) on the stack? In target.c:memory_xfer_partial_1, we can see: do { res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL, readbuf, writebuf, memaddr, reg_len); if (res > 0) break; /* We want to continue past core files to executables, but not past a running target's memory. */ if (ops->to_has_all_memory (ops)) break; ops = ops->beneath; } while (ops != NULL); it goes through the target vectors on stack to try to fetch the requested memory, so in target tfile, GDB can fall back to beneath target to read memory isn't in trace frames. So, we only to handle the requested memory in trace frames. For the rest, just return zero, and fall back to beneath target to handle. We also need field 'to_has_all_memory' of tfile_ops return zero, so that GDB will fall back to the beneath target, as shown above. Considering that trace file is a snapshot, it makes sense to return zero in 'to_has_all_memory' in tfile target. In target.c:add_target, if to_has_all_memory is NULL, it will be set to 'return_zero', so we don't have to set 'to_has_all_memory' in tfile_ops. This patch is to revert some changes from this patch [PATCH] Do partial xfers from trace file http://sourceware.org/ml/gdb-patches/2010-04/msg00082.html the test case added by this patch still passes. I also add a test in gdb.trace/tfile.exp to do 'disassemble' to test GDB is able to read read-only part from executable. It was mentioned in the patch post, but not implemented as a test case. Regression tested on x86_64-linux. Is it OK? gdb: 2013-03-20 Yao Qi * tracepoint.c: Don't include "gdbcore.h". (tfile_xfer_partial): Return 0 to fall back to beneath target to do partial xfer. (tfile_has_all_memory): Remove. (init_tfile_ops): Don't install tfile_has_all_memory to field to_has_all_memory of tfile_ops. gdb/testsuite: 2013-03-20 Yao Qi * gdb.trace/tfile.exp: Test GDB reads correctly from executable in tfile target. --- gdb/testsuite/gdb.trace/tfile.exp | 4 +++ gdb/tracepoint.c | 46 ++---------------------------------- 2 files changed, 7 insertions(+), 43 deletions(-) diff --git a/gdb/testsuite/gdb.trace/tfile.exp b/gdb/testsuite/gdb.trace/tfile.exp index 2bdde37..82aec20 100644 --- a/gdb/testsuite/gdb.trace/tfile.exp +++ b/gdb/testsuite/gdb.trace/tfile.exp @@ -66,6 +66,10 @@ gdb_test "print testglob2" " = 271828" "print testglob2 on trace file" gdb_test "print constglob" " = 10000" "print constglob on trace file" +# Test GDB correctly reads read-only pieces from the executable. +gdb_test "disassemble main" \ + "Dump of assembler code for function main:.* $hex <\\+0\\>:.*End of assembler dump\." + gdb_test "tfind" "Target failed to find requested trace frame." \ "tfind does not find a second frame in trace file" diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index 00b0b81..a2fd66c 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -39,7 +39,6 @@ #include "observer.h" #include "user-regs.h" #include "valprint.h" -#include "gdbcore.h" #include "objfiles.h" #include "filenames.h" #include "gdbthread.h" @@ -5024,41 +5023,9 @@ tfile_xfer_partial (struct target_ops *ops, enum target_object object, } } - /* It's unduly pedantic to refuse to look at the executable for - read-only pieces; so do the equivalent of readonly regions aka - QTro packet. */ - /* FIXME account for relocation at some point. */ - if (exec_bfd) - { - asection *s; - bfd_size_type size; - bfd_vma vma; - - for (s = exec_bfd->sections; s; s = s->next) - { - if ((s->flags & SEC_LOAD) == 0 - || (s->flags & SEC_READONLY) == 0) - continue; - - vma = s->vma; - size = bfd_get_section_size (s); - if (vma <= offset && offset < (vma + size)) - { - ULONGEST amt; - - amt = (vma + size) - offset; - if (amt > len) - amt = len; - - amt = bfd_get_section_contents (exec_bfd, s, - readbuf, offset - vma, amt); - return amt; - } - } - } - - /* Indicate failure to find the requested memory block. */ - return -1; + /* If GDB can't find the requested memory in trace frames, fall back + to the next target down on the stack. */ + return 0; } /* Iterate through the blocks of a trace frame, looking for a 'V' @@ -5098,12 +5065,6 @@ tfile_get_trace_state_variable_value (int tsvnum, LONGEST *val) } static int -tfile_has_all_memory (struct target_ops *ops) -{ - return 1; -} - -static int tfile_has_memory (struct target_ops *ops) { return 1; @@ -5202,7 +5163,6 @@ init_tfile_ops (void) tfile_ops.to_get_trace_state_variable_value = tfile_get_trace_state_variable_value; tfile_ops.to_stratum = process_stratum; - tfile_ops.to_has_all_memory = tfile_has_all_memory; tfile_ops.to_has_memory = tfile_has_memory; tfile_ops.to_has_stack = tfile_has_stack; tfile_ops.to_has_registers = tfile_has_registers; -- 1.7.7.6