From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19344 invoked by alias); 8 May 2012 16:06:13 -0000 Received: (qmail 19268 invoked by uid 22791); 8 May 2012 16:06:08 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO,TW_EG X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 May 2012 16:05:49 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A7C561C6831; Tue, 8 May 2012 12:05:48 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ntwGn2h-sttL; Tue, 8 May 2012 12:05:48 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 50CFF1C66BB; Tue, 8 May 2012 12:05:48 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 4335E145616; Tue, 8 May 2012 09:05:42 -0700 (PDT) Date: Tue, 08 May 2012 16:06:00 -0000 From: Joel Brobecker To: "Maciej W. Rozycki" Cc: Mark Kettenis , gdb-patches@sourceware.org Subject: Re: [RFA 1/2] mips: Switch inferior function calls to ON_STACK method. Message-ID: <20120508160542.GB15555@adacore.com> 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> <201205042118.q44LIh3p018153@glazunov.sibelius.xs4all.nl> <201205051144.q45Bitv4006357@glazunov.sibelius.xs4all.nl> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Oiv9uiLrevHtW1RS" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) 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/msg00237.txt.bz2 --Oiv9uiLrevHtW1RS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 782 > After some thinking I realised that the reliance on signal delivery to > work properly to trap non-executable stack may actually be a problem for > bare-iron targets. IMO, I think we can start worrying about those when we actually encounter the problem; and I am assuming that this is not going to be specific to mips. Are we still good to go with this patch? Attached is the latest version. For any additional comments that you'd like to be added (in particular, with respect to what you just pointed out), I suggest add them as a followup patch. This way, this one is out of the way, and we can just focus on the comments themselves. I am also adding a second patch, which shows the changes I made in this iteration. Tested on mips-irix, no regression. Thanks, -- Joel --Oiv9uiLrevHtW1RS Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-mips-Switch-inferior-function-calls-to-ON_STACK-meth.patch" Content-length: 2878 >From ddb25412f122ca8180238f188536b3027182cb31 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Wed, 2 May 2012 20:39:57 -0400 Subject: [PATCH] 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. --- gdb/mips-tdep.c | 37 +++++++++++++++++++++++++++++++++---- 1 files changed, 33 insertions(+), 4 deletions(-) diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c index 9a3c7fb..68ac858 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_call" gdbarch method. */ + +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; + static gdb_byte nop_insn[] = { 0, 0, 0, 0 }; + + *bp_addr = 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 nop instruction + to prevent that from happening. */ + write_memory (*bp_addr - bp_len, nop_insn, sizeof (nop_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, @@ -6906,10 +6937,8 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) /* MIPS version of CALL_DUMMY. */ - /* 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); -- 1.7.0.4 --Oiv9uiLrevHtW1RS Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-mips-tdeps-ON_STACK-push_dummy_call-adjustements.patch" Content-length: 2395 >From 758b0c5b1dfab824516badb3f8b238b40905732f Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Mon, 7 May 2012 18:54:27 -0400 Subject: [PATCH] mips-tdeps ON_STACK push_dummy_call adjustements. --- gdb/mips-tdep.c | 13 +++++-------- 1 files changed, 5 insertions(+), 8 deletions(-) diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c index 5e9a6ed..68ac858 100644 --- a/gdb/mips-tdep.c +++ b/gdb/mips-tdep.c @@ -3009,7 +3009,7 @@ mips_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr) return align_down (addr, 16); } -/* Implement the push_dummy_code gdbarch method for mips targets. */ +/* Implement the "push_dummy_call" gdbarch method. */ static CORE_ADDR mips_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, @@ -3019,9 +3019,9 @@ mips_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, struct regcache *regcache) { int bp_len; - gdb_byte null_insn[4] = { 0 }; + static gdb_byte nop_insn[] = { 0, 0, 0, 0 }; - *bp_addr = mips_frame_align (gdbarch, sp); + *bp_addr = sp; gdbarch_breakpoint_from_pc (gdbarch, bp_addr, &bp_len); /* The breakpoint layer automatically adjusts the address of @@ -3029,9 +3029,9 @@ mips_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, 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 + So, we reserve those 4 bytes and write a nop instruction to prevent that from happening. */ - write_memory (*bp_addr - bp_len, null_insn, sizeof (null_insn)); + write_memory (*bp_addr - bp_len, nop_insn, sizeof (nop_insn)); sp = mips_frame_align (gdbarch, *bp_addr - 2 * bp_len); /* Inferior resumes at the function entry point. */ @@ -6937,9 +6937,6 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) /* MIPS version of CALL_DUMMY. */ - /* 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, ON_STACK); set_gdbarch_push_dummy_code (gdbarch, mips_push_dummy_code); set_gdbarch_frame_align (gdbarch, mips_frame_align); -- 1.7.0.4 --Oiv9uiLrevHtW1RS--