From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16531 invoked by alias); 4 May 2012 21:19:44 -0000 Received: (qmail 16520 invoked by uid 22791); 4 May 2012 21:19:43 -0000 X-SWARE-Spam-Status: No, hits=-2.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 May 2012 21:19:18 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id q44LIkVM014502; Fri, 4 May 2012 23:18:46 +0200 (CEST) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id q44LIh3p018153; Fri, 4 May 2012 23:18:43 +0200 (CEST) Date: Fri, 04 May 2012 21:19:00 -0000 Message-Id: <201205042118.q44LIh3p018153@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: brobecker@adacore.com CC: macro@codesourcery.com, gdb-patches@sourceware.org In-reply-to: <20120504205818.GT15555@adacore.com> (message from Joel Brobecker on Fri, 4 May 2012 13:58:18 -0700) Subject: Re: [RFA 1/2] mips: Switch inferior function calls to ON_STACK method. References: <1336071802-13599-1-git-send-email-brobecker@adacore.com> <1336071802-13599-2-git-send-email-brobecker@adacore.com> <20120503214933.GJ15555@adacore.com> <20120504205818.GT15555@adacore.com> 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: 2012-05/txt/msg00137.txt.bz2 > Date: Fri, 4 May 2012 13:58:18 -0700 > From: Joel Brobecker > > > Understood, but I'd be happier if the comment you're removing or a > > similar stayed in place. If by trap you mean SIGTRAP, then I think this > > is not going to be the case. > > I think you refer to the comment from Andrew Cagney? I've put it back > as is. I really don't think that comment is helpful anymore. Almost ten years have gone by without anyone feeling the need to implement the command Andrew is alluding to here, > OK to commit, modulo the possible rename above? Seems you missed my comment about the the mips_frame_align() call. It's unecessary since the frame is already properly aligned by the calles of push_dummy_code(). > commit b78a75e1442a349531a017036a02f43c4df71427 > Author: Joel Brobecker > Date: Wed May 2 20:39:57 2012 -0400 > > mips: Switch inferior function calls to ON_STACK method. > > This patch switches the mips code to use the ON_STACK method > for function calls instead of AT_SYMBOL, which we want to remove. > > gdb/ChangeLog: > > * mips-tdep.c (mips_push_dummy_code): New function. > (mips_gdbarch_init): Set the gdbarch call_dummy_location to > ON_STACK and install mips_push_dummy_code as our gdbarch > push_dummy_code routine. > > diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c > index 9a3c7fb..5e9a6ed 100644 > --- a/gdb/mips-tdep.c > +++ b/gdb/mips-tdep.c > @@ -3009,6 +3009,37 @@ mips_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr) > return align_down (addr, 16); > } > > +/* Implement the push_dummy_code gdbarch method for mips targets. */ Perhaps change that comment into: /* Implement the "push_dummy_call" gdbarch method. */ such that it is consistent with the style of comments in rl78-tdep.c and rx-tdep.c and moxie-tdep.c? The "for mips targets" isn't really adding any information (and might end up accidentally being copied). > + > +static CORE_ADDR > +mips_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, > + CORE_ADDR funaddr, struct value **args, > + int nargs, struct type *value_type, > + CORE_ADDR *real_pc, CORE_ADDR *bp_addr, > + struct regcache *regcache) > +{ > + int bp_len; > + gdb_byte null_insn[4] = { 0 }; > + > + *bp_addr = mips_frame_align (gdbarch, sp); > + gdbarch_breakpoint_from_pc (gdbarch, bp_addr, &bp_len); > + > + /* The breakpoint layer automatically adjusts the address of > + breakpoints inserted in a branch delay slot. With enough > + bad luck, the 4 bytes located just before our breakpoint > + instruction could look like a branch instruction, and thus > + trigger the adjustement, and break the function call entirely. > + So, we reserve those 4 bytes and write a null instruction > + to prevent that from happening. */ > + write_memory (*bp_addr - bp_len, null_insn, sizeof (null_insn)); > + sp = mips_frame_align (gdbarch, *bp_addr - 2 * bp_len); > + > + /* Inferior resumes at the function entry point. */ > + *real_pc = funaddr; > + > + return sp; > +} > + > static CORE_ADDR > mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, > struct regcache *regcache, CORE_ADDR bp_addr, > @@ -6909,7 +6940,8 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > /* NOTE: cagney/2003-08-05: Eventually call dummy location will be > replaced by a command, and all targets will default to on stack > (regardless of the stack's execute status). */ > - set_gdbarch_call_dummy_location (gdbarch, AT_SYMBOL); > + set_gdbarch_call_dummy_location (gdbarch, ON_STACK); > + set_gdbarch_push_dummy_code (gdbarch, mips_push_dummy_code); > set_gdbarch_frame_align (gdbarch, mips_frame_align); > > set_gdbarch_convert_register_p (gdbarch, mips_convert_register_p); > > --fXStkuK2IQBfcDe+-- >