From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9584 invoked by alias); 27 Mar 2012 18:53:57 -0000 Received: (qmail 9574 invoked by uid 22791); 27 Mar 2012 18:53:56 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,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; Tue, 27 Mar 2012 18:53:43 +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 q2RIrcDK031329; Tue, 27 Mar 2012 20:53:38 +0200 (CEST) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id q2RIrbWf024897; Tue, 27 Mar 2012 20:53:37 +0200 (CEST) Date: Tue, 27 Mar 2012 18:53:00 -0000 Message-Id: <201203271853.q2RIrbWf024897@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: jan.kratochvil@redhat.com CC: gdb-patches@sourceware.org In-reply-to: <20120326190414.GB11001@host2.jankratochvil.net> (message from Jan Kratochvil on Mon, 26 Mar 2012 21:04:14 +0200) Subject: Re: ping: [patch 2/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5 References: <20120309210117.GB30432@host2.jankratochvil.net> <20120326190414.GB11001@host2.jankratochvil.net> 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-03/txt/msg00917.txt.bz2 > Date: Mon, 26 Mar 2012 21:04:14 +0200 > From: Jan Kratochvil > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -2326,6 +2326,30 @@ i386_16_byte_align_p (struct type *type) > return 0; > } > > +/* Implementation for set_gdbarch_push_dummy_code. */ > + > +static CORE_ADDR > +i386_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 bplen; > + CORE_ADDR bppc = sp; > + > + gdbarch_breakpoint_from_pc (gdbarch, &bppc, &bplen); > + sp -= bplen; > + > + /* amd64_push_dummy_call does alignment on its own but i386_push_dummy_call > + does not. ABI requires stack alignment for executables using SSE. */ > + if (gdbarch_frame_align_p (gdbarch)) > + sp = gdbarch_frame_align (gdbarch, sp); > + > + *bp_addr = sp; > + *real_pc = funaddr; > + return sp; > +} You're almost certainly right worrying about stack alignment. However, the comment doesn't make a lot of sense. For one thing, amd64_push_dummy_call() doesn't explicitly align the stack. It just preserves alignment. Also, if i386_push_dummy_call() doesn't preserve alignment (and it sure looks like it doesn't), then aligning the stack here doesn't help. In any case, we don't need to to explicitly align the stack since that's already done bu call_function_by_hand(). So we only need to make sure the stack stays aligned, which can be easily done by always allocating 16 bytes of stack space. Calling gdbarch_breakpoint_from_pc() is also a bit overkill. The breakpoint instruction on i386 is pretty much fixed, we know it is just a single byte and we know it can be placed just about anywhere. So the simplified version below is perfectly adequate. We have some freedom on where to place the breakpoint in the 16-byte stack gap we create. I chose to put it up hight such that a small buffer overflow isn't likely to overwrite the breakpoint instruction. Index: i386-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/i386-tdep.c,v retrieving revision 1.346 diff -u -p -r1.346 i386-tdep.c --- i386-tdep.c 29 Feb 2012 14:59:41 -0000 1.346 +++ i386-tdep.c 27 Mar 2012 18:30:24 -0000 @@ -2327,6 +2327,21 @@ i386_16_byte_align_p (struct type *type) } static CORE_ADDR +i386_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, + CORE_ADDR funcaddr, + struct value **args, int nargs, + struct type *value_type, + CORE_ADDR *real_pc, CORE_ADDR *bp_addr, + struct regcache *regcache) +{ + *bp_addr = sp - 1; + *real_pc = funcaddr; + + /* Keep the stack aligned. */ + return sp - 16; +} + +static CORE_ADDR i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, @@ -7372,6 +7387,8 @@ i386_gdbarch_init (struct gdbarch_info i set_gdbarch_get_longjmp_target (gdbarch, i386_get_longjmp_target); /* Call dummy code. */ + set_gdbarch_call_dummy_location (gdbarch, ON_STACK); + set_gdbarch_push_dummy_code(gdbarch, i386_push_dummy_code); set_gdbarch_push_dummy_call (gdbarch, i386_push_dummy_call); set_gdbarch_frame_align (gdbarch, i386_frame_align);