From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10751 invoked by alias); 12 May 2005 13:11:39 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 10307 invoked from network); 12 May 2005 13:11:04 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sourceware.org with SMTP; 12 May 2005 13:11:04 -0000 Received: from drow by nevyn.them.org with local (Exim 4.50) id 1DWDSx-00081E-Uc; Thu, 12 May 2005 09:11:04 -0400 Date: Thu, 12 May 2005 13:13:00 -0000 From: Daniel Jacobowitz To: Orjan Friberg Cc: gdb-patches@sources.redhat.com Subject: Re: [gdbserver/rfa] CRIS/CRISv32 gdbserver support (part 2) Message-ID: <20050512131103.GB30555@nevyn.them.org> Mail-Followup-To: Orjan Friberg , gdb-patches@sources.redhat.com References: <41E7969F.3010009@axis.com> <20050130045359.GC25185@nevyn.them.org> <41FF9274.7030700@axis.com> <20050224204906.GC11751@nevyn.them.org> <42835324.70905@axis.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <42835324.70905@axis.com> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00276.txt.bz2 On Thu, May 12, 2005 at 02:59:16PM +0200, Orjan Friberg wrote: > Daniel Jacobowitz wrote: > > > >Let's go ahead. The patch is mostly OK, but I have a last question or > >two... > > Thanks for your comments, and apologies for the long delay. (There are a > couple of questions at the end also.) I didn't see any questions? > Another thing: when removing cris_reinsert_addr, I got the same errors as > described in http://sourceware.org/ml/gdb/2005-01/msg00071.html (i.e. > "thread getmsg err: no event message for getmsg" and gdbserver getting a > SIGSEGV). At the moment I have no idea what's going on, so as a temporary > solution I reinstated cris_reinsert_addr but with a FIXME. Odd. Does your hardware have real single step support, or implement PTRACE_SINGLESTEP by managing breakpoints? Other than that, I'm not sure what could be wrong. I've got more nits for you though. > static const unsigned long cris_breakpoint = 0xe938; > #define cris_breakpoint_len 2 > > static int > cris_breakpoint_at (CORE_ADDR where) > { > unsigned long insn; > > (*the_target->read_memory) (where, (char *) &insn, cris_breakpoint_len); > if (insn == cris_breakpoint) > return 1; > > /* If necessary, recognize more trap instructions here. GDB only uses the > one. */ > return 0; > } This is presumably going to work for you, but it's seriously untidy - there's a hidden endianness dependency. Can you see it? Hint: the types of cris_breakpoint and insn are wrong. Same thing applies to linux-crisv32-low.c. I just realized that these files will require an entry in config/djgpp/fnchange.lst; their names are too similar. > static void > cris_write_data_breakpoint (int bp, unsigned long start, unsigned long end) > { > switch (bp) > { > case 0: > supply_register_by_name("s3", &start); > supply_register_by_name("s4", &end); > break; You were doing great on GNU formatting right up until here, and then you lost it. Everything from here down in this file needs to be checked. For instance: > static int > cris_insert_watchpoint(char type, CORE_ADDR addr, int len) Space. > if (type < '2' || type > '4') { > /* Unsupported. */ > return 1; > } Unnecessary braces; even if they were necessary they'd have to be on their own lines and indented. -- Daniel Jacobowitz CodeSourcery, LLC