From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5437 invoked by alias); 26 Jun 2006 13:38:48 -0000 Received: (qmail 5423 invoked by uid 22791); 26 Jun 2006 13:38:46 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Mon, 26 Jun 2006 13:38:39 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1FurIT-0007c1-OS for gdb-patches@sourceware.org; Mon, 26 Jun 2006 09:38:37 -0400 Date: Mon, 26 Jun 2006 13:38:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: Re: [rfc] Correct semantics of target_read_partial, add target_read_whole Message-ID: <20060626133837.GA28927@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org References: <20060622032355.GA27566@nevyn.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.11+cvs20060403 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-06/txt/msg00374.txt.bz2 On Sat, Jun 24, 2006 at 02:06:07PM +0400, Vladimir Prus wrote: > did you notice my post on gdb-devel about having two different methods of > reading method. The post subject was: "target memory read/write methods". As I told Vladimir on IRC, I'd been putting this off because it's such a complicated question :-) > To quote: > > One way to read a memory is: > > - target_read_memory, which calls > - xfer_using_stratum, which calls > - target_xfer_partial (iterating over 'stratums'), which > - goes to function pointer in target_ops > > This is the predominant method. And the right one. > > Another way to read memory is: > > - get_target_memory{unsigned}, which calls: > - target_read, which calls: > - target_xfer_partial I think this is just wrong, given the way GDB has changed since. > Your patch add target_read_whole, that calls 'target_read', which until now > is used only by get_target_memory{unsigned}, which is used in 3 places in > entire gdb. In addition 'xfer_using_stratum' already has some code to > repeatedly call target_xfer_partial. I also might not understand what > stratums are but should not they be used in all cases? Otherwise, behaviour > of target_read_while for 'object = TARGET_OBJECT_MEMORY' will be different > from the behaviour of 'target_read_memory'. You have to understand what the strata (plural of stratum :-) are to understand what's going on here. Roughly, each stratum is a view of the target. The actual details are murkier than this and in need of a lot of changes, but it's low priority and fairly fragile, so it gets left alone. When debugging a threaded native program, we might have: thread_stratum -> process_stratum -> file_stratum When debugging a core file we might have: core_stratum -> file_stratum That core file case is the most interesting one for xfer_using_stratum. A core file often has some of memory, but not all of it. For instance, it might have all the data sections but none of the code sections; we have to load those from the executable instead. This means that for memory, we may need to fulfill a single memory read from more than one stratum. For other objects, this is generally not true. We want to fill the request from the first stratum that knows the object we're requesting. We don't support sharing objects across strata for anything besides memory. This fits together with another piece of the puzzle: > Also, how you target_read_whole work when > > target_xfer_partial_p () > > return false? The 'target_read' function does not check it and > 'target_xfer_partial' will just assert. > > Should 'target_read_whole' check for target_xfer_partial_p and fail is its > false? That's not actually true. static int target_xfer_partial_p (void) { return (target_stack != NULL && target_stack->to_xfer_partial != default_xfer_partial); } versus gdb_assert (ops->to_xfer_partial != NULL); A target which does not initialize to_xfer_partial will have it set to the default in add_target. The default pushes requests down the target stack until it finds a non-default implementation and calls that. Many implementations seem to stop there; that's probably a bug waiting to happen, I don't know if the implementations should pass unknown objects further down or if default_xfer_partial should. But since we're usually interested in the process layer's to_xfer_partial it doesn't cause a problem in practice. > I think that either: > 1. There are way too many code paths for reading > 2. There are way too few comments. Both true. Feel free to propose fixes for either :-) This is one of several areas in which GDB has too many interfaces for doing the same thing. -- Daniel Jacobowitz CodeSourcery