From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6521 invoked by alias); 9 Oct 2008 17:11:44 -0000 Received: (qmail 6513 invoked by uid 22791); 9 Oct 2008 17:11:42 -0000 X-Spam-Check-By: sourceware.org Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 09 Oct 2008 17:10:47 +0000 Received: from localhost (hagrid.ecoscentric.com [127.0.0.1]) by mail.ecoscentric.com (Postfix) with ESMTP id 222843B40055; Thu, 9 Oct 2008 18:10:45 +0100 (BST) Received: from mail.ecoscentric.com ([127.0.0.1]) by localhost (hagrid.ecoscentric.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OAeJgu38EaZi; Thu, 9 Oct 2008 18:10:43 +0100 (BST) Received: from delenn.bartv.net (hagrid.vpn.ecoscentric.com [192.168.145.1]) by mail.ecoscentric.com (Postfix) with ESMTP id A694A3B4004A; Thu, 9 Oct 2008 18:10:42 +0100 (BST) Date: Thu, 09 Oct 2008 17:11:00 -0000 Message-Id: From: Bart Veer To: Daniel Jacobowitz CC: gdb-patches@sourceware.org In-reply-to: <20081001210933.GA8477@caradoc.them.org> (message from Daniel Jacobowitz on Wed, 1 Oct 2008 17:09:33 -0400) Subject: Re: add file I/O support when debugging an embedded target via jtag References: <48BAAC44.4000002@codesourcery.com> <20080925222009.GA8202@caradoc.them.org> <20081001210933.GA8477@caradoc.them.org> 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: 2008-10/txt/msg00292.txt.bz2 >>>>> "Daniel" == Daniel Jacobowitz writes: Daniel> On Wed, Oct 01, 2008 at 07:38:22PM +0100, Bart Veer wrote: >> Adding a new stratum certainly appears to be controversial, >> although I am not certain why. Daniel> Because this is not about what in the current sources will Daniel> break. It's about the purpose of the target stack, which Daniel> does not include synthesizing operations like this. I find this somewhat confusing. My understanding of the target stack is that it allows target-related operations to be encapsulated in independent modules. The various target_ops structures are combined into a single current target vector, with a simple priority scheme in the form of strata. According to my earlier investigation the individual modules do not care whether there are five strata or 500, as long as the existing priority ordering is preserved. I do not see anything obvious in the source code or the gdb internals documentation to contradict this. Further, I see code in bsd-uthread.c, aix-thread.c and linux-thread-db.c which appears to work in much the same way as my patch. For example bsd_uthread_wait() chains to another wait() lower down the stack. The current file I/O support in remote.c is handled entirely at the target stack level, completely transparent to higher-level code like wait_for_inferior(). It seems strange that the h/w debug file I/O now needs to be implemented higher up. Daniel>> I don't see why it has to be in the target vector at all. >> The h/w debug file I/O code needs to take some action for every >> load, resume and wait operation. For the wait, >> deprecated_target_wait_hook provides an alternative but I >> assume that hook is going to disappear at some point. There are >> no equivalent hooks for load and resume. Daniel> Those operations are where your patch acts, but I don't Daniel> think they're what you actually want. What you do after Daniel> load doesn't have anything to do with "load"; I assume Daniel> that at load time you're setting the breakpoint, but GDB Daniel> already has several hooks to set internal breakpoints. And Daniel> it already has support for hitting an internal breakpoint, Daniel> hiding it from the user, and taking some action - this is Daniel> how SVR4 shared library support works. That is not exactly how my patch works. The breakpoint is permanently embedded in the target-side executable and never needs to be set by gdb. That avoids using up one of the hardware breakpoint when debugging an executable held in flash. However there is a target-side flag in the .data section which determines whether or not the application is running under gdb control. If instead the application is running stand-alone then any file I/O operations will fail with ENOSYS. The load operation must be detected because it will invalidate the flag, and next subsequent resume operation must be intercepted so that the flag can be set again. Anyway, I have been trying to understand some of the higher-level gdb code, to see how to get things working there. It looks like it would take something like the following: 1) a new post-load notifier, invoked from load_command() after the target_load() call. 2) a new resume notifier, invoked before the target gets resumed. I am not totally sure where this should go, but proceed() is a likely candidate. 3) extensions to breakpoint.c, along the lines of create_solib_event_breakpoint() etc. This is where things get much more complicated. hwdebug-fileio.o is part of REMOTE_OBS alongside remote.o, remote-fileio.o, etc., and not part of the main sources. Hence breakpoint.c cannot have any direct dependencies on the file I/O support, and something more generic is needed. Something along the lines of: a) two new breakpoint types, bp_callback and bp_callback_no_insert. should_be_inserted() can distinguish between these two types. b) a new field in the breakpoint structure, struct bpstat_what (*callback_fn)(something_or_other) c) a new function to register a callback breakpoint create_callback_breakpoint (bptype, CORE_ADDR, callback_fn); plus of course a matching remove function. d) when handle_inferior_event() calls bpstat_what(), that can detect a callback breakpoint, invoke the callback function, and return its bpstat_what value. handle_inferior_event() can then be made to keep_going(). e) the h/w debug file I/O code can then register a no_insert callback breakpoint at the appropriate address, and the callback function would be responsible for performing the file I/O operation. That seems to be roughly how the SVR4 shared library support works. No doubt there would be various details to sort out. I am also rather concerned about handle_inferior_event(). There are almost 1000 lines of code in that function before the call to bpstat_what(), and I have very little understanding of what most of that code does or how it might interfere with what I am trying to achieve. For example I don't know whether gdb is going to try to remove all breakpoints and reinsert them later, which would mess up performance for no good reason. I do see some handling of thread events which are of no interest if the target is going to be resumed immediately after the file I/O operation has completed. Now, my current patch changes one line apiece in target.h, remote.c, and remote-fileio.h, plus a handful of simple changes in remote-fileio.c. Everything else is done in a self-contained new module, courtesy of the modularity within the current target vector implementation. Doing things above the target vector level would seem to involve changes to the gdb core that are much much bigger, and would add yet more complexity to existing code. That looks like a bad idea to me. Bart -- Bart Veer eCos Configuration Architect eCosCentric Limited The eCos experts http://www.ecoscentric.com/ Barnwell House, Barnwell Drive, Cambridge, UK. Tel: +44 1223 245571 Registered in England and Wales: Reg No 4422071.