From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30085 invoked by alias); 21 Aug 2013 15:32:29 -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 30076 invoked by uid 89); 21 Aug 2013 15:32:29 -0000 X-Spam-SWARE-Status: No, score=-8.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 21 Aug 2013 15:32:28 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7LFWNCr014369 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 21 Aug 2013 11:32:24 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r7LFWL6c029499; Wed, 21 Aug 2013 11:32:22 -0400 Message-ID: <5214DD85.5060605@redhat.com> Date: Wed, 21 Aug 2013 15:32:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Mark Kettenis CC: yao@codesourcery.com, tromey@redhat.com, gdb-patches@sourceware.org Subject: Re: [PATCH] PR gdb/15871: Unavailable entry value is not shown correctly References: <1376379586-24150-1-git-send-email-yao@codesourcery.com> <1376379586-24150-3-git-send-email-yao@codesourcery.com> <87siy4gsn4.fsf@fleche.redhat.com> <521458A8.7060304@codesourcery.com> <5214D03C.9040703@redhat.com> <201308211447.r7LEld3t025510@glazunov.sibelius.xs4all.nl> In-Reply-To: <201308211447.r7LEld3t025510@glazunov.sibelius.xs4all.nl> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00589.txt.bz2 On 08/21/2013 03:47 PM, Mark Kettenis wrote: >> Date: Wed, 21 Aug 2013 15:35:40 +0100 >> From: Pedro Alves >> >> On 08/13/2013 08:39 AM, Yao Qi wrote: >>> However, when I run this test, I find j@entry is something like, >>> j@entry= >>> instead of unavailable. >>> >>> I don't emit a fail for it because I am not very sure it is expected >>> to be "unavailable". I am fine to kfail it. >>> >>> I looked into a little, and looks reading entry value doesn't use >>> value availability-aware API. It is not an easy fix to me. >> >> I looked into this. Here's a patch. Let me know what you think. > > I think you should simply define the return codes as negative numbers. I think that's really a matter of taste. But maybe my taste is broken... It's quite common to return "-errno" as error indication. The Linux kernel uses that convention for example. Here's the updated patch. ---------- Subject: [PATCH] PR gdb/15871: Unavailable entry value is not shown correctly In entry-values.exp, we have a test where the entry value of 'j' is unavailable, so it is expected that printing j@entry yields "". However, the actual output is: (gdb) frame #0 0x0000000000400540 in foo (i=0, i@entry=2, j=2, j@entry=) The error is thrown here: #0 throw_it (reason=RETURN_ERROR, error=MEMORY_ERROR, fmt=0x8cd550 "Cannot access memory at address %s", ap=0x7fffffffc8e8) at ../../src/gdb/exceptions.c:373 #1 0x00000000005e2f9c in throw_error (error=MEMORY_ERROR, fmt=0x8cd550 "Cannot access memory at address %s") at ../../src/gdb/exceptions.c:422 #2 0x0000000000673a5f in memory_error (status=5, memaddr=6293992) at ../../src/gdb/corefile.c:204 #3 0x0000000000673aea in read_memory (memaddr=6293992, myaddr=0x7fffffffca60 "\200\316\377\377\377\177", len=4) at ../../src/gdb/corefile.c:223 #4 0x00000000006784d1 in dwarf_expr_read_mem (baton=0x7fffffffcd50, buf=0x7fffffffca60 "\200\316\377\377\377\177", addr=6293992, len=4) at ../../src/gdb/dwarf2loc.c:334 #5 0x000000000067645e in execute_stack_op (ctx=0x1409480, op_ptr=0x7fffffffce87 "\237<\005@", op_end=0x7fffffffce88 "<\005@") at ../../src/gdb/dwarf2expr.c:1045 #6 0x0000000000674e29 in dwarf_expr_eval (ctx=0x1409480, addr=0x7fffffffce80 "\003\350\t`", len=8) at ../../src/gdb/dwarf2expr.c:364 #7 0x000000000067c5b2 in dwarf2_evaluate_loc_desc_full (type=0x10876d0, frame=0xd8ecc0, data=0x7fffffffce80 "\003\350\t`", size=8, per_cu=0xf24c40, byte_offset=0) at ../../src/gdb/dwarf2loc.c:2236 #8 0x000000000067cc65 in dwarf2_evaluate_loc_desc (type=0x10876d0, frame=0xd8ecc0, data=0x7fffffffce80 "\003\350\t`", size=8, per_cu=0xf24c40) at ../../src/gdb/dwarf2loc.c:2407 #9 0x000000000067a5d4 in dwarf_entry_parameter_to_value (parameter=0x13a7960, deref_size=18446744073709551615, type=0x10876d0, caller_frame=0xd8ecc0, per_cu=0xf24c40) at ../../src/gdb/dwarf2loc.c:1160 #10 0x000000000067a962 in value_of_dwarf_reg_entry (type=0x10876d0, frame=0xd8de70, kind=CALL_SITE_PARAMETER_DWARF_REG, kind_u=...) at ../../src/gdb/dwarf2loc.c:1310 #11 0x000000000067aaca in value_of_dwarf_block_entry (type=0x10876d0, frame=0xd8de70, block=0xf1c2d4 "Q", block_len=1) at ../../src/gdb/dwarf2loc.c:1363 #12 0x000000000067e7c9 in locexpr_read_variable_at_entry (symbol=0x13a7540, frame=0xd8de70) at ../../src/gdb/dwarf2loc.c:3326 #13 0x00000000005daab6 in read_frame_arg (sym=0x13a7540, frame=0xd8de70, argp=0x7fffffffd0e0, entryargp=0x7fffffffd100) at ../../src/gdb/stack.c:362 #14 0x00000000005db384 in print_frame_args (func=0x13a7470, frame=0xd8de70, num=-1, stream=0xea3890) at ../../src/gdb/stack.c:669 #15 0x00000000005dc338 in print_frame (frame=0xd8de70, print_level=1, print_what=SRC_AND_LOC, print_args=1, sal=...) at ../../src/gdb/stack.c:1199 #16 0x00000000005db8ee in print_frame_info (frame=0xd8de70, print_level=1, print_what=SRC_AND_LOC, print_args=1) at ../../src/gdb/stack.c:851 #17 0x00000000005da2bb in print_stack_frame (frame=0xd8de70, print_level=1, print_what=SRC_AND_LOC) at ../../src/gdb/stack.c:169 #18 0x00000000005de236 in frame_command (level_exp=0x0, from_tty=1) at ../../src/gdb/stack.c:2265 dwarf2_evaluate_loc_desc_full (frame #7) knows to handle NOT_AVAILABLE_ERROR errors, but read_memory always throws a generic error. Presently, only the value machinery knows to handle unavailable memory. We need to push the awareness down to the target_xfer layer, making it return a finer grained error indication. We can only return a generic -1 nowadays, which leaves the upper layers with no clue on why the xfer failed. Use target_xfer_partial directly, rather than propagating the error through target_read_memory so as to get a better address to display in the error message. (target_read_memory & friends build on top of target_read (thus the target_xfer machinery), but turn all errors to EIO, an errno value. I think this is a mistake, and we'd better convert all these to return a target_xfer_error too, but that can be done separately. I looked around a bit over memory_error calls, and the need to handle random errno values, other than the EIOs gdb itself hardcodes, probably comes (only) from deprecated_xfer_memory, which uses errno for error indication, but I didn't look exhaustively. We should really get rid of that...) Tested on x86_64 Fedora 17, native and gdbserver. gdb/ 2013-08-21 Pedro Alves PR gdb/15871 * corefile.c (target_xfer_memory_error): New function. (memory_error): Defer EIO to target_memory_error. (read_memory): Use target_xfer_partial, and handle finer-grained target xfer errors. * target.c (memory_xfer_partial_1): If memory is known to be unavailable, return TARGET_XFER_E_UNAVAILABLE instead of -1. (target_xfer_partial): Make extern. * target.h (enum target_xfer_error): New. (target_xfer_partial): Declare. (struct target_ops) : Adjust describing comment. gdb/testsuite/ 2013-08-21 Pedro Alves PR gdb/15871 * gdb.trace/entry-values.exp: Remove kfail. --- gdb/corefile.c | 50 ++++++++++++++++++++++++++------ gdb/target.c | 10 ++----- gdb/target.h | 25 +++++++++++++++- gdb/testsuite/gdb.trace/entry-values.exp | 1 - 4 files changed, 67 insertions(+), 19 deletions(-) diff --git a/gdb/corefile.c b/gdb/corefile.c index a86f4b3..c86c172 100644 --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -193,17 +193,38 @@ Use the \"file\" or \"exec-file\" command.")); } +/* Report a target xfer memory error by throwing a suitable + exception. */ + +static void +target_xfer_memory_error (enum target_xfer_error err, CORE_ADDR memaddr) +{ + switch (err) + { + case TARGET_XFER_E_IO: + /* Actually, address between memaddr and memaddr + len was out of + bounds. */ + throw_error (MEMORY_ERROR, + _("Cannot access memory at address %s"), + paddress (target_gdbarch (), memaddr)); + case TARGET_XFER_E_UNAVAILABLE: + throw_error (NOT_AVAILABLE_ERROR, + _("Memory at address %s unavailable."), + paddress (target_gdbarch (), memaddr)); + default: + internal_error (__FILE__, __LINE__, + "unhandled target xfer memory error: %s", + plongest (err)); + } +} + /* Report a memory error by throwing a MEMORY_ERROR error. */ void memory_error (int status, CORE_ADDR memaddr) { if (status == EIO) - /* Actually, address between memaddr and memaddr + len was out of - bounds. */ - throw_error (MEMORY_ERROR, - _("Cannot access memory at address %s"), - paddress (target_gdbarch (), memaddr)); + target_xfer_memory_error (TARGET_XFER_E_IO, memaddr); else throw_error (MEMORY_ERROR, _("Error accessing memory address %s: %s."), @@ -216,11 +237,22 @@ memory_error (int status, CORE_ADDR memaddr) void read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len) { - int status; + LONGEST xfered = 0; - status = target_read_memory (memaddr, myaddr, len); - if (status != 0) - memory_error (status, memaddr); + while (xfered < len) + { + LONGEST xfer = target_xfer_partial (current_target.beneath, + TARGET_OBJECT_MEMORY, NULL, + myaddr + xfered, NULL, + memaddr + xfered, len - xfered); + + if (xfer == 0) + target_xfer_memory_error (TARGET_XFER_E_IO, memaddr + xfered); + if (xfer < 0) + target_xfer_memory_error (xfer, memaddr + xfered); + xfered += xfer; + QUIT; + } } /* Same as target_read_stack, but report an error if can't read. */ diff --git a/gdb/target.c b/gdb/target.c index 377724d..f17e53e 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -81,12 +81,6 @@ static LONGEST current_xfer_partial (struct target_ops *ops, const gdb_byte *writebuf, ULONGEST offset, LONGEST len); -static LONGEST target_xfer_partial (struct target_ops *ops, - enum target_object object, - const char *annex, - void *readbuf, const void *writebuf, - ULONGEST offset, LONGEST len); - static struct gdbarch *default_thread_architecture (struct target_ops *ops, ptid_t ptid); @@ -1523,7 +1517,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, /* No use trying further, we know some memory starting at MEMADDR isn't available. */ - return -1; + return TARGET_XFER_E_UNAVAILABLE; } /* Don't try to read more than how much is available, in @@ -1700,7 +1694,7 @@ make_show_memory_breakpoints_cleanup (int show) /* For docs see target.h, to_xfer_partial. */ -static LONGEST +LONGEST target_xfer_partial (struct target_ops *ops, enum target_object object, const char *annex, void *readbuf, const void *writebuf, diff --git a/gdb/target.h b/gdb/target.h index d538e02..64a8b0f 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -199,6 +199,20 @@ enum target_object /* Possible future objects: TARGET_OBJECT_FILE, ... */ }; +/* Possible error codes returned by target_xfer_partial, etc. */ + +enum target_xfer_error +{ + /* Generic I/O error. Note that it's important that this is '-1', + as we still have target_xfer-related code returning hardcoded + '-1' on error. */ + TARGET_XFER_E_IO = -1, + + /* Transfer failed because the piece of the object requested is + unavailable. */ + TARGET_XFER_E_UNAVAILABLE = -2 +}; + /* Enumeration of the kinds of traceframe searches that a target may be able to perform. */ @@ -293,6 +307,14 @@ extern char *target_read_stralloc (struct target_ops *ops, enum target_object object, const char *annex); +/* See target_ops->to_xfer_partial. */ + +extern LONGEST target_xfer_partial (struct target_ops *ops, + enum target_object object, + const char *annex, + void *readbuf, const void *writebuf, + ULONGEST offset, LONGEST len); + /* Wrappers to target read/write that perform memory transfers. They throw an error if the memory transfer fails. @@ -475,7 +497,8 @@ struct target_ops data-specific information to the target. Return the number of bytes actually transfered, zero when no - further transfer is possible, and -1 when the transfer is not + further transfer is possible, and a negative error code (really + an 'enum target_xfer_error' value) when the transfer is not supported. Return of a positive value smaller than LEN does not indicate the end of the object, only the end of the transfer; higher level code should continue transferring if diff --git a/gdb/testsuite/gdb.trace/entry-values.exp b/gdb/testsuite/gdb.trace/entry-values.exp index bfa4f5c..20d06ce 100644 --- a/gdb/testsuite/gdb.trace/entry-values.exp +++ b/gdb/testsuite/gdb.trace/entry-values.exp @@ -265,7 +265,6 @@ gdb_test_no_output "tstop" gdb_test "tfind" "Found trace frame 0, .*" "tfind start" # Since 'global2' is not collected, j@entry is expected to be 'unavailable'. -setup_kfail "gdb/15871" *-*-* gdb_test "bt 1" "#0 .* foo \\(i=\[-\]?$decimal, i@entry=2, j=\[-\]?$decimal, j@entry=\\).*" gdb_test "tfind" "Target failed to find requested trace frame\..*" -- 1.7.11.7